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

fix!(biome): allow passing options to biome check #4799

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

redbmk
Copy link
Contributor

@redbmk redbmk commented Jun 25, 2024

The only option available to biome's lsp-proxy command used for linting is --config-path. However, we are using ALE to find and set the project root, and have a way to manually override, so that is no longer necessary.

The LSP proxy also used the g:ale_biome_options config, which is shared with the fixer's check command, but lsp-proxy will throw an error if unknown options are included, making it so that option is only useful to set the project root.

BREAKING CHANGE: We are no longer passing options to the biome LSP proxy, but we can still set the project root with
g:ale_biome_lsp_project_root.


I added a test that shows you can pass arbitrary options along to biome check and updated the documentation.

I had thought about continuing to pass g:ale_biome_options to the LSP for backwards compatibility and t hen creating a new g:ale_biome_fixer_options that get added additionally to the fixer, but that seemed a lot more complex. Considering the only shared option is --config-path, which is also the only LSP option, people either were only using that option or their LSP would have been broken.

The downside to this approach is that if someone is relying on --config-path they may need to update to use ale_biome_lsp_project_root instead, so maybe a deprecation path is better. @w0rp @hsanson, what do you think?

The only option available to biome's `lsp-proxy` command used for
linting is `--config-path`. However, we are using ALE to find and set
the project root, and have a way to manually override, so that is no
longer necessary.

The LSP proxy also used the `g:ale_biome_options` config, which is
shared with the fixer's `check` command, but `lsp-proxy` will throw an
error if unknown options are included, making it so that option is only
useful to set the project root.

BREAKING CHANGE: We are no longer passing options to the biome LSP
proxy, but we can still set the project root with
`g:ale_biome_lsp_project_root`.
@hsanson
Copy link
Contributor

hsanson commented Jun 27, 2024

No strong opinions here. As long as there are alternatives users can easily implement to get same functionality as before I see no reason to stop this PR.

Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

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

Thanks for the update.

@hsanson hsanson merged commit a0ad5f9 into dense-analysis:master Jun 27, 2024
7 checks passed
@redbmk redbmk deleted the biome-check-options branch July 8, 2024 06:06
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.

2 participants