Kampfkarren / selene

A blazing-fast modern Lua linter written in Rust
https://kampfkarren.github.io/selene/
Mozilla Public License 2.0
600 stars 76 forks source link

✨feat: `.selene.toml` and `selene.toml` are now detected by order of priority #591

Open Zeioth opened 6 months ago

Zeioth commented 6 months ago

This PR:

The feature has been tested on neovim + none-ls with:

Example of bad syntax screenshot_2024-03-10_04-17-20_778230898

Example of file detected OK screenshot_2024-03-10_04-18-01_912823820

Example of no config file screenshot_2024-03-10_04-19-59_927521326

Zeioth commented 6 months ago

Also it closes #469

Zeioth commented 6 months ago

@chriscerie I'm gonna try but this is my first contribution to a rust project and I'm not a expert in the language so I feeI might cause more noise than it's worth trying to apply the feedback.

If you want to co-author please go ahead.

Zeioth commented 6 months ago

Need changelog + docs.

Changelog: https://github.com/Zeioth/selene/blob/dot-selene-toml/CHANGELOG.md Docs: https://github.com/Zeioth/selene/commit/e5fd4f0fab47fbf515c7d60aaca30eeb7c50bb95

More info

In the docs, I've just replaced every mention to selene.toml by .selene.toml since now .selene.toml has priority over selene.toml.

Only in src/usage/configuration we mention that we also accept selene.toml to avoid duplication.

Zeioth commented 6 months ago

The PR should be ready merge it. Please tell me if you would like some other changes.

Hopefully we can ship it relatively soon.

chriscerie commented 6 months ago

Thanks for working on this! Unfortunately I've closed the parent issue for now: https://github.com/Kampfkarren/selene/issues/469#issuecomment-2016543458

This pr will still be very helpful if the issue gets reopened in the future. If that happens, this pr will also need to update the vscode extension with the new config resolution.

Zeioth commented 6 months ago

chriscerie since there are no bugs, the code has already being refactored as specified, and all the new reviews are stylistic preferences, or would affect parts outside the PR, I'm gonna leave the code as it is. Feel free to co-author if you see something you would prefer to do in a different way.

The code is safe to merge.

chriscerie commented 6 months ago

I appreciate your contribution, but this pr can't be merged until these bugs are fixed (see above comments for root causes). I'm happy to step in and co-author to get this merged but as there are other selene features with higher priority, I make no promises on the timeline.

Config with invalid utf-8 character

Currently image New image

Missing config

Currently image New image

Zeioth commented 6 months ago

bad UTF-8 error is controlled now. screenshot_2024-03-27_14-18-16_067338925

Zeioth commented 6 months ago

Missing file is also parsed correctly (this is my build, not the package manager one). screenshot_2024-03-27_14-34-02_956661142

Zeioth commented 6 months ago

Relevant changes to vscode extension https://github.com/Kampfkarren/selene/pull/591/commits/47b06421a96fca5d0f3646112ded08b01d4b8714

@chriscerie It should be ready but I have no clue how to test this on vscode. Could you test it or tell me how you test it so I can try to reproduce?