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

Overhaul extension binary distribution #767

Closed
wants to merge 3 commits into from
Closed

Conversation

JohnnyMorganz
Copy link
Owner

@JohnnyMorganz JohnnyMorganz commented Sep 16, 2023

This PR overhauls the way the VSCode extension works in finding and using a stylua binary. It gets rid of the overhead of downloading from GitHub and updating the binary from time to time via a notifcation.

The stylua binary lookup works in the following order:

  1. Use stylua.styluaPath if specified
  2. Lookup in the PATH environment variable for a binary if stylua.useBinaryFromPath is enabled (default: true)
  3. Fall back to a bundled version of stylua

StyLua is commonly managed by other tools such as Aftman/Foreman, or a system installation. Step (2) means that we will use the same version that a user has on the command line to run stylua. This means the version will always remain in sync. Closes #527.

The extension version will now reflect the version of stylua bundled. Due to existing limitations, we will change the major version to use 1 instead of 0 (as the vscode extension has already uploaded 1.X.X releases). So 0.18.2 will map to extension version 1.18.2. Currently, there is confusion about what the extension versioned represented, and this alleviates that problem. Also, users can now downgrade/upgrade the extension version to determine the version of stylua they want.

The extension will be released alongside new stylua releases. This has the added benefit that the extension will always be updated. Currently the process was manual, and often times I would forget to update the extension.
The extension README and CHANGELOG will now also follow the equivalent main stylua README/CHANGELOG.

We get rid of the following settings, as they have no use anymore:

  • stylua.releaseVersion
  • stylua.targetReleaseVersion
  • stylua.disableVersionCheck

Potential drawbacks:

  • For users of Aftman, if some projects are managed by aftman and others not, then the extension will fail to format due to an aftman error in those unmanaged projects ("attempted to run an Aftman-managed tool but tool not specified in a manifest"). Users can resolve this by adding the tool to their global aftman config.
  • When not using a toolchain manager, it is no longer possible to manage the bundled version via a workspace-level .vscode/settings.json with a stylua.targetReleaseVersion set. The bundled version is globally consistent. To resolve this, a toolchain manager will need to be used. This problem is consistent with other tools, such as prettier or black.
  • In doing this, the bundled extension will always have to be built with all features, otherwise we would have to bundle multiple binaries at once for the different feature sets. The idea is the bundled version is a fallback option that isn't meant to be used in most cases - appropriate feature selection should be done by the binary found on the PATH.

TODO: this overhaul is currently blocked on LPGhatguy/aftman#54
TODO: update readme and changelog about this

@codecov
Copy link

codecov bot commented Sep 16, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (1fcdc6e) 97.58% compared to head (1d62da3) 97.58%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #767   +/-   ##
=======================================
  Coverage   97.58%   97.58%           
=======================================
  Files          16       16           
  Lines        6135     6135           
=======================================
  Hits         5987     5987           
  Misses        148      148           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JohnnyMorganz JohnnyMorganz changed the title Overhaul extension binary lookup Overhaul extension binary distribution Sep 16, 2023
@JohnnyMorganz
Copy link
Owner Author

I like the way that the biome vscode extension changed its structure actually. They used to bundle the binary but now follow more closely to what our existing setup is: https://github.com/biomejs/biome-vscode.

Best seen in this video: https://biomejs.dev/blog/biome-wins-prettier-challenge/#vscode-extension-goodies

In particular, having relevant info about what version is being used in the status is helpful, as well as quick-switching versions. And, switching to using a binary available on the PATH by default.

Maybe we should rethink our approach

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.

[VSCode Ext] pick stylua version/path from system environment PATH
1 participant