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

Provide ways to enforce StyLua versions in codebase #398

Open
JohnnyMorganz opened this issue Mar 7, 2022 · 7 comments
Open

Provide ways to enforce StyLua versions in codebase #398

JohnnyMorganz opened this issue Mar 7, 2022 · 7 comments
Labels
enhancement New feature or request
Milestone

Comments

@JohnnyMorganz
Copy link
Owner

A common problem that occurs (especially with lots of external contributors to a project) is when the version of the stylua binary installed and the version of the binary used to initially format a project differ. This can lead to different formatting outputs, and confusing CI failures (no diff locally, but get an error on CI).

There are tools out there to help manage binary versions for projects, such as foreman, asdf or bvm, but it may also be worthwhile to try and provide some support in the binary itself to help warn people of these issues.

One solution is to allow an optional version field in stylua.toml, which can be pinned to a semver version of the binary. Then when StyLua is run and the versions differ, we can emit a warning.

We can also take this a step further in the VSCode Extension and GitHub Action, where we read the version field in the config file (if present) and use that specific version for the project.

The caveat to this is that users may expect a "version" field in the configuration to imply that the version is automatically managed for them, and changing this field would automatically update the binary. This is not the case, the field just acts as a hint to the binary to warn on version mismatches.

@JohnnyMorganz JohnnyMorganz added the enhancement New feature or request label Mar 7, 2022
@LastTalon
Copy link
Contributor

We had talked about using foreman in the extension previously. I think that may be a good way to handle this overall. Prettier has this exact same issue, but their ecosystem typically solves it for them since the prettier version is provided in package.json.

@JohnnyMorganz
Copy link
Owner Author

We had talked about using foreman in the extension previously. I think that may be a good way to handle this overall.

A large majority of users of stylua do not use foreman (if they did, this issue wouldn't even be a problem as foreman would be handling the version for the project).

This issue isn't just scoped to the version the vscode extension should be using, but the binary itself installed from say a package manager or through github releases, which is the main problem.

@LastTalon
Copy link
Contributor

I still think it may be good to tackle the extension version foreman usage at some point. I use foreman for managing StyLua and it would be great if the extension automatically used the foreman version.

I think its a bit out of scope for a formatter to do version management. Pinning a version is necessary for a formatter, but I think should be done by a tool that specializes in it. Prettier, for instance, mentions this problem, saying that its important to lock to a version of prettier, to install that version for a project, ensuring you're using the right one (rather than npx installing a temporary latest prettier), and noting how versioning prettier is difficult since each released version inherently formats differently.

To me this says that we should be encouraging people to use StyLua through a tool like foreman that can manage the version correctly and per project. This is the exact reason I use foreman for it in my projects. This ensures all devs agree and CI doesn't break. Adding a version to the StyLua config, I think, is confusing, since it only warns you, and involves double configuration (configuring for StyLua, as well as a tool that actually manages it).

@JohnnyMorganz
Copy link
Owner Author

JohnnyMorganz commented Mar 8, 2022

I think its a bit out of scope for a formatter to do version management.

The suggestion wasn't to manage the version itself, merely provide a warning if it notices the versions are different. I agree though, having a version tag in the configuration is confusing, and managing versions in more than one place is probably also annoying.

@LastTalon
Copy link
Contributor

I agree though, having a version tag in the configuration is confusing, and managing versions in more than one place is probably also annoying.

Actually, I think I made a mistake when I said it was confusing. Its not just that its confusing or annoying, its that it introduces errors. Its very easy to configure the version in one place and not another.

@JohnnyMorganz JohnnyMorganz added this to the 2.0 milestone Dec 7, 2022
@JohnnyMorganz
Copy link
Owner Author

Something I saw recently was that black has a --required-version command line flag to provide similar behaviour

@LastTalon
Copy link
Contributor

That can help us prevent using the incorrect version at least. This would be better if it can be included in configuration files, too. And I still think it's really important to be able to configure this all in one place as that's easy to overlook and introduce error if I have to, e.g. change the StyLua version in my stylua.toml, aftman.toml, and some-action.yaml.

One thing I've been thinking about in the meantime is the way the rust ecosystem handles all of this by having editions. If the most current StyLua could always know how to format the way all prior versions did we could add that "edition" as configuration to stylua.toml and then you can always update StyLua to the latest version without worrying.

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

No branches or pull requests

2 participants