Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make actionEvent of extension more specific #212

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

FieteO
Copy link
Contributor

@FieteO FieteO commented Mar 24, 2023

There is a warning in the package.json about the activationEvent: Using '*' activation is usually a bad idea as it impacts performance.

As explained in the vscode docs, the * corresponds to 'always activate'.
This PR makes this actionEvent more specific. Ideally, the extension should only be activated for json and yaml files using onLanguage:

"activationEvents": [
    "onLanguage:yaml",
    "onLanguage:json"
]

However the deleteYamlNode test case is failing then. Maybe it would make sense that someone with more experience of this project (@ak1394 ?) could take a look in a second step to improve this further

@ak1394
Copy link
Collaborator

ak1394 commented Mar 24, 2023

Thanks for the PR! Unfortunately we can't quite get rid of activation on '*' at the moment. This extension contains two large chunks of functionality, one which is mostly self-contained and the other that requires user to configure credentials to access our cloud-based "42Crunch Platform".

At the moment we conditionally enable some of that functionality at the launch of the VS Code, if user has indeed configured platform credentials, therefore we have to activate as soon as possible.

We have plans to migrate away from using '*', but we are not quite ready to do it.

@FieteO
Copy link
Contributor Author

FieteO commented Mar 25, 2023

Ah I see. Thanks for elaborating @ak1394 !

@ak1394
Copy link
Collaborator

ak1394 commented Mar 25, 2023

Thanks for taking an interest in it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants