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

Activate the extension automatically #344

Conversation

vogelsgesang
Copy link
Collaborator

@vogelsgesang vogelsgesang commented Feb 26, 2024

So far, the bazelWorkspace view, listing the available build targets,
was only shown after running the first Bazel command, e.g., through
the command pallete. This behavior was probably unintended and is rather
cumbersome for people which want to inspect the available Bazel targets.

The chain of events which lead to this behavior are:

  • The bazelWorkspace view is only shown when the
    vscodeBazelHaveBazelWorkspace context variable is set.
  • This context variable is unset by default, and hence the view is
    hidden initially.
  • As soon as the first Bazel command is run from the command pallete,
    the extension was activated.
  • As part of activating, the extension detected that there is in fact
    a Bazel workspace and hence it set the vscodeBazelHaveBazelWorkspace
    context variable, unhiding the bazelWorkspace view.

This commit fixes the issue by activating the extension as soon as one
of Bazel's build files is found.

Furthermore, all pre-existing activation events were redundant since
VS Code 1.74 which started to automatically infer activation events
based on the contributions provided by an extension

BEGIN_COMMIT_OVERRIDE
fix: Activate the extension automatically (#344)
END_COMMIT_OVERRIDE

@cameron-martin
Copy link
Collaborator

It might be better using the workspaceContains activation event here, looking for the MODULE.bazel or WORKSPACE file. That way, the activation cost is only paid if the user is opening a bazel workspace.

@vogelsgesang vogelsgesang force-pushed the avogelsgesang-activate-automatically branch from 222afcf to ee51c51 Compare February 27, 2024 11:58
So far, the `bazelWorkspace` view, listing the available build targets,
was only shown after running the first Bazel command, e.g., through
the command pallete. This behavior was probably unintended and is rather
cumbersome for people which want to inspect the available Bazel targets.

The chain of events which lead to this behavior are:
* The `bazelWorkspace` view is only shown when the
  `vscodeBazelHaveBazelWorkspace` context variable is set.
* This context variable is unset by default, and hence the view is
  hidden initially.
* As soon as the first Bazel command is run from the command pallete,
  the extension was activated.
* As part of activating, the extension detected that there is in fact
  a Bazel workspace and hence it set the `vscodeBazelHaveBazelWorkspace`
  context variable, unhiding the `bazelWorkspace` view.

This commit fixes the issue by activating the extension as soon as one
of Bazel's build files is found.
@vogelsgesang vogelsgesang force-pushed the avogelsgesang-activate-automatically branch from ee51c51 to 18b3af3 Compare February 27, 2024 11:58
@vogelsgesang
Copy link
Collaborator Author

vogelsgesang commented Feb 27, 2024

Based on comments like #72 (comment)

[the] directory might not have a BUILD file (it could be some random place within the overall bazel workspace)

I was initially unsure, whether such a workspaceContains trigger would be sufficient in all cases.

But reading that thread again, I now think that the comment was about opening a folder somewhere in the folder hierarchy between two BUILD files. I.e., there would still also be a BUILD file in the subdirectory. As such, I think using workspaceContains should be fine?

I updated the pull request to use workspaceContains now. This certainly solves my use case, and we can always switch to onStartupFinished if anyone runs into issues in the future

@@ -17,16 +17,11 @@
"Programming Languages"
],
"activationEvents": [
"onLanguage:starlark",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep onLanguage:starlark, on the off chance case someone opens a .bzl file in a workspace without any of the other workspaceContains triggers. Doesn't hurt.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to VS Code's documentation, the onLanguage:starlark activation event is unnecessary:

Beginning with VS Code 1.74.0, languages contributed by your extension do not require a corresponding onLanguage activation event declaration for your extension to be activated.

Note that this extension only support VSCode >= 1.85.0

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the description of this PR to also include the reasoning on why I removed all pre-existing activation events

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks, sounds good!

@jfirebaugh jfirebaugh merged commit 085262e into bazelbuild:master Feb 27, 2024
4 checks passed
@vogelsgesang vogelsgesang deleted the avogelsgesang-activate-automatically branch March 26, 2024 02:08
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

3 participants