selene icon indicating copy to clipboard operation
selene copied to clipboard

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

Open LinuxOnTheDesktop opened this issue 1 year ago • 5 comments

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 avatar Jun 30 '24 00:06 LinuxOnTheDesktop

Cf. #90:

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

LinuxOnTheDesktop avatar Jun 30 '24 02:06 LinuxOnTheDesktop

  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?

chriscerie avatar Jun 30 '24 03:06 chriscerie

    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).

LinuxOnTheDesktop avatar Jun 30 '24 23:06 LinuxOnTheDesktop

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.

chriscerie avatar Jul 01 '24 00:07 chriscerie

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.

LinuxOnTheDesktop avatar Jul 02 '24 01:07 LinuxOnTheDesktop