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

Add option to turn off duplicate lints serviced by luau-lsp #580

Open
chriscerie opened this issue Jan 14, 2024 · 2 comments
Open

Add option to turn off duplicate lints serviced by luau-lsp #580

chriscerie opened this issue Jan 14, 2024 · 2 comments
Labels
A-bin Area: selene, the program A-config Area: Configuring selene A-luau Area: Luau C-enhancement Category: Feature request or improvement

Comments

@chriscerie
Copy link
Collaborator

We've heard from several selene + luau-lsp users that duplicate lints of the same thing coming from both tools were annoying enough to turn one off entirely. I propose we add an option to turn off lints that already exist in luau-lsp. This would exist as a selene.toml config so both the cli and extension can consume it.

Note that lints like argument count mismatching are duplicate, but we'd need to handle cases where they deviate or agree depending on the specific case (e.g., passing two args to Instance.new() is allowed by luau-lsp but not selene. In this case we'd keep the warning, but not for other arg count cases where selene and luau-lsp agree).

Alternatives

  • Instead of an opt out of duplicate lints, we automatically turn off duplicate lints when luau std is used. Then we can provide an option to opt back in for users who don't use luau-lsp. This would be a breaking change, however.
    • It should be possible for selene's vscode extension to detect if luau-lsp is also installed to prevent false positives. However, we can't reliably detect it in CI environments.
@chriscerie chriscerie added C-enhancement Category: Feature request or improvement A-config Area: Configuring selene A-bin Area: selene, the program A-luau Area: Luau labels Jan 14, 2024
@LastTalon
Copy link
Contributor

I think it makes sense to not make assumptions about what other tools the user is using; keeping the lints enabled by default and opt-out sounds correct to me.

I think this is something I would definitely use, but I would also want a particular workflow easily supported still. My team uses selene as part of our CI workflow and while it would be useful to have the lints de-duplicated in editor, we don't run Luau lints in our CI workflow, so I would want the ability to keep those lints in CI even if they aren't shown in editor.

@chriscerie
Copy link
Collaborator Author

while it would be useful to have the lints de-duplicated in editor, we don't run Luau lints in our CI workflow

Thanks, this is helpful! Supporting cli and extension toggles independently is definitely possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-bin Area: selene, the program A-config Area: Configuring selene A-luau Area: Luau C-enhancement Category: Feature request or improvement
Projects
None yet
Development

No branches or pull requests

2 participants