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

Docs: more help with selene.toml syntax would be welcome #609

Open LinuxOnTheDesktop opened 3 months ago

LinuxOnTheDesktop commented 3 months ago

The documentation on selene.toml syntax (here and, as it were, 'under' here) seems sparse. A greater number of examples (somewhere) would help.

Let me illustrate the problem. I created a selene.toml file the contents of which are as follows.

[config]
unused_variable = { ignore_pattern = [ "^_", "^conky_"] }
undefined_variable = { ignore_pattern = "^conky_" }

[EDITED.]

It took me a while to formulate that syntax, and that syntax fails thusly:

ERROR: [undefined_variable] Configuration was incorrectly formatted: invalid type: map, expected unit

LinuxOnTheDesktop commented 3 months ago

Cf. #90:

the CLI complains about it being wrong, what's the correct format? The docs have no example...

chriscerie commented 3 months ago
  1. ignore_pattern in unused_variable takes a regex string as documented along with the default value "^_" as an example.
  2. undefined_variable doesn't have the ignore_pattern config as documented.

The bug here is the error message is not helpful, but as far as docs go what kind of further examples would have been helpful?

LinuxOnTheDesktop commented 3 months ago

    Thanks.

    The error message is indeed unhelpful.

    Re your point 2: the state of affairs at issue is documented only in the arguably attenuated sense that the relevant webpage does not mention ignore_pattern.

    Helpful examples would be ones that show:

I) one needs braces; II) string values are to be quoted (if indeed they are); III) one can use arrays.

I presume that documentation on I-III is available via the TOML link. Yet, having the top-level configuration page show a few choice examples would work as a 'quick start guide'. It might be an idea to use as one of the examples the aforementioned unused_variable = { ignore_pattern = [ "^_", "^conky_"] }. Here is why. First, that's the sort of thing lots of people will want. Second: that example might help users avoid a 'footgun' to which I fell victim, namely, overwriting the ^_ pattern.

     Here are two further points. Each concerns the top-level configuration page. (i) It seems to be a bit confusing to count 'allow' as a level of severity. (ii) Formatting of the style lint_1 = "severity" would be better replaced by the style lint_1 = "<severity>". The existing style encourages the reader to interpret severity as a literal string (even though the subsequent text tells them not to; it is best not to lead them in the wrong direction in the first place).

    I might seem to pick nits. But everything I have suggested would have helped me, for one, to get going more quickly (and I had the additional burden of having to get up to speed on regex, so I did want the other stuff made as accessible as possible).

chriscerie commented 3 months ago

I) one needs braces;

Existing example in https://kampfkarren.github.io/selene/usage/configuration.html#configuring-specific-lints.

II) string values are to be quoted (if indeed they are); III) one can use arrays.

I'd be fine adding a non-boolean example to https://kampfkarren.github.io/selene/usage/configuration.html#configuring-specific-lints.

It might be an idea to use as one of the examples the aforementioned unused_variable = { ignore_pattern = [ "^_", "^conky_"] }. Here is why. First, that's the sort of thing lots of people will want. Second: that example might help users avoid a 'footgun' to which I fell victim, namely, overwriting the ^_ pattern.

I'm confused here. ignore_pattern accepts string as documented, not a list.

(i) It seems to be a bit confusing to count 'allow' as a level of severity.

This is borrowed from clippy and it won't change.

Formatting of the style lint_1 = "severity" would be better replaced by the style lint_1 = "<severity>"

This is a valid suggestion.

LinuxOnTheDesktop commented 3 months ago

ignore_pattern accepts string as documented, not a list.

I think that the syntax that I was after was unused_variable = { ignore_pattern = "^_|^conky_" }. The wider point stands.

As this post of mine shows, I'm not the ideal person to take up the job of improving the documentation.