make buildifier warning less aggressive

[jab]

This extension's warning about no buildifier being found is overly eager, and presumes that the environment should have a single, global buildifier executable shared by multiple projects.

But developers often prefer to have project-local versions of their tool dependencies (and no global version) for better reproducibility, which is even facilitated for buildifier by https://github.com/keith/buildifier-prebuilt. Note that all projects created via aspect init use this setup (see https://github.com/aspect-build/aspect-workflows-template).

Such users can add something like the following to ensure that a buildifier binary is available when their projects are opened and that the Bazel extension is configured to use it:
.vscode/tasks.json:

    {
      // Build `buildifier` on folderOpen to ensure that a buildifier binary exists which the
      // VSCode Bazel plugin can use.
      "label": "Build buildifier",
      "command": "bazel build @buildifier_prebuilt//:buildifier",
      "presentation": {
        "reveal": "never",
        "panel": "new"
      },
      "runOptions": {
        "runOn": "folderOpen",
      },
      "type": "shell",
    }

.vscode/settings.json:

{
    "bazel.buildifierExecutable": "bazel-bin/external/buildifier_prebuilt~/buildifier/buildifier",
}

But even with this configuration, this extension still nags about no buildifier being found, because the check for it completes a split second before it's finished building, and it's never checked for again.

If the initial check fails, how about doing a retry or two with a sensible backoff?

cc @alexeagle

[jab]

Aha, looks like the config above can be changed to just setting bazel.buildifierExecutable to @buildifier_prebuilt//:buildifier and then this extension will use bazel to build that target and use it, nice.

[alexeagle]

Ooh, did you find that documented? If not I would like to re-open this to track the missing docs

[cameron-martin]

Looks like this was added in #350, but there doesn't seem to be docs for this. I'll re-open this to track docs support. A PR is very much welcome :)

Edit: The description of the config setting does mention this, but maybe that's not sufficient?

[jab]

Thanks for reopening this in support of making the user experience better here.

The fine print that is already in the Settings UI is indeed how I eventually discovered this:

It would be more discoverable if the bold "Buildifier Executable" title text were changed to "Buildifier Executable or Target".

And better yet, instead of linking to https://github.com/bazelbuild/buildtools/releases last in that description, and steering users to downloading and installing it globally, link to some instructions that document both options - (1) setting up a local bazel target, with a code snippet users could copy/paste, and (2) downloading and installing it globally. And similarly, the "no buildifier found warning" could steer users toward these docs rather than only offering the "download" button.

Better still would be if the logic were made smarter, something like trying to detect a local bazel target before falling back to the current behavior (looking for a "buildifier" on the PATH). If possible, that seems like the best way to make the "no buildifier found" experience occur less often.

[vogelsgesang]

my 2 cts: We should make the buildifier integration less aggressive in multiple ways

• only try to detect the absence / presence of buildifier when opening the first Starlark. Many users of this extension might only be Bazel users but not Bazel authors, i.e. people who only ever run bazel build, but never actually edit a Starlark file. We shouldn't pester them about installing buildifier
• there should be a way to disable buildifier altogether
• the warning should provide options "Never ask again" and "Configure Buildifier Path" instead of pointing to the download page
• we should improve the documentation (as mentioned already)