codespell-project / codespell

check code for common misspellings
GNU General Public License v2.0
1.84k stars 470 forks source link

Config file undocumented? #3208

Open buhtz opened 9 months ago

buhtz commented 9 months ago

I might have missed it but it seems to me that the config file is nearly undocumented.

I can see all (?) possible config options via codespell --help. But these are for the command line. It is not clear if this options do have an equivalent in the config file and what values they should have.

For example it seems that I can do count = True in the config file. But from reading --help it was not clear that True is the correct value. Or --check-hidden is totally unclear. It works on command line but when I do check-hidden = True I got codespell: error: unrecognized arguments: True. So I don't know how to configure check-hidden via config file.

DimitriPapadopoulos commented 9 months ago

The config file is explained using examples, the INI example:

[codespell]
skip = *.po,*.ts,./src/3rdParty,./src/Test
count =
quiet-level = 3

and the TOML example:

[tool.codespell]
skip = '*.po,*.ts,./src/3rdParty,./src/Test'
count = ''
quiet-level = 3
buhtz commented 9 months ago

"Explained" to not mean it is fully documented. And as a described I was not able to "map" the commandline option (check hidden) to its config file equivalent myself. Sorry but I need docu about it.

buhtz commented 9 months ago

I checked your code and realized that you "convert" the config file values back into command line arguments and then parse them via argparse.

    parser.add_argument(
        "-H",
        "--check-hidden",
        action="store_true",
        default=False,
        help='check hidden files and directories (those starting with ".") as well.',
    )

Doing in config fiel check-hidden=True do result in coveralls --check-hidden True. This works only when you use one store_true argument. But when you use a second argument like this it causes the error I reported. Please try:

codespell --check-hidden True --count True
DimitriPapadopoulos commented 9 months ago

I agree the documentation could be improved, I can recall myself having a hard time initially trying to use or modify codespell. Merge requests to improve documentation would be welcome.

By the way, I think you have understood that command line option --check-hidden maps to check-hidden = in INI config files and check-hidden = '' in TOML config files, haven't you? If so, the remaining issue is how to improve documentation so that newcomers can easily find documentation on the config file formats, isn't it?

buhtz commented 9 months ago

the remaining issue is how to improve documentation so that newcomers can easily find documentation on the config file formats, isn't it?

Correct. Maybe add a section in the help-page explaining the general rules about how to map command line options into config file entries. Especially for the "store_true" values (count, check-hidden) there should be an extra comment to just leave them empty to say "True".

nogweii commented 6 months ago

Alternatively, boolean arguments could accept a value that gets parsed as enabling it in the INI file and documenting that. I think that would make it easier to grok.

DimitriPapadopoulos commented 6 months ago

Good idea, however that's not what INI files usually look like.

DimitriPapadopoulos commented 6 months ago

Actually configparser supports a wide range of values for booleans:

Config parsers do not guess datatypes of values in configuration files, always storing them internally as strings. This means that if you need other datatypes, you should convert on your own: [...] Since this task is so common, config parsers provide a range of handy getter methods to handle integers, floats and booleans. The last one is the most interesting because simply passing the value to bool() would do no good since bool('False') is still True. This is why config parsers also provide getboolean(). This method is case-insensitive and recognizes Boolean values from 'yes'/'no', 'on'/'off', 'true'/'false' and '1'/'0' [1].

It looks like worth looking into.