Fix #2055: Add support for PEP 518

[Freed-Wu]

Fix #2055

[Freed-Wu]

I import toml as a new dependence. In fact, it looks like it can be an optional dependence, because we can:

# Read toml before other config files.
if os.path.isfile(os.path.realpath(tomlfile)):
    import toml
    data = toml.load(tomlfile).get('tool', {})
    config.read_dict(data)

I am not sure if it is a good idea.

[DimitriPapadopoulos]

To avoid the current error in tests, you need to add toml to dependencies in setup.py, or make it optional as you suggested.

[Freed-Wu]

Added lines #L411 - L413 were not covered by tests

Need change test file to cover the if-clause.

Any one know how to add cover test file? Thanks.

[Freed-Wu]

Have rebased.

[cclauss]

Tests are going to be vital.

[Freed-Wu]

How to fix coverage test?

[larsoner]

Sorry for the slow response @Freed-Wu , I just enabled GH actions tests to actually run. Hopefully that helps :)

[larsoner]

Okay I added a test. I also bumped the min Python version to 3.7 since 3.6 has been EOL for 9 months.

@cclauss has approved already so I'll mark for merge-when-green, thanks in advance @Freed-Wu !

[larsoner]

... I started to add a print('WARNING: when pyproject.toml is found but tomli is not installed, in case there are people using codespell who do have such a toml file (but no tomli in their codespell CI run) -- which I think will be fairly common/likely.

Then I realized that just checking for pyproject.toml in the cwd is a bit fragile. So I changed it so that users need to specify --toml pyproject.toml or whatever filename they want to load from. This allows for absolute paths to other directories for example if they want. It's more explicit which seems safer. And now we can just raise ImportError if tomli is not found, because if a user passes --toml they should get an error if tomli is not present (they are unambiguously trying to do something not allowed).

[peternewman]

@larsoner while I can see the argument for not requiring it, it does seem like one of these things where you'd hope it will just work, and it's a bit of a shame if you have to explicitly specify it.

Can we not hijack whatever searching configparser does for setup.cfg and .codespellrc so it behaves the same as them (and maybe throws a warning if the file is present but the module unavailable or something?

[peternewman]

@larsoner can we have a comment explaining what this is trying to achieve. It looks like the old fail-fast would have done the same thing?

[larsoner]

fail-fast cancels jobs within an action if one of them fails

this cancels jobs if another commit is pushed

[peternewman]

Makes sense, although do we want fail-fast too?

[larsoner]

I think that our tests are fast enough already and as we add others (different OSes etc) they can potentially show different problems. But we reenable it if you want

[peternewman]

Presumably it won't start testing the next commit until the last one is done?

[larsoner]

With cancel-in-progress: true if you push a commit it kills runs from a previous commit. But all jobs from a given commit are allowed to run to completion (because of fail-fast: false) so you can see potentially different errors for your latest commit

To me it's the most reasonable behavior, as it's how Travis behaved from the start years ago IIRC and most CI providers followed suit (or at least provided an option for it) and has seemed like a good compromise over the years

[peternewman]

...via the --toml argument

[larsoner]

while I can see the argument for not requiring it, it does seem like one of these things where you'd hope it will just work, and it's a bit of a shame if you have to explicitly specify it.

I'd agree if we could be guaranteed that someone wants to use pyproject.toml if and only if it's located in the cwd when they run codespell. But I don't think we have any guarantee that's the case (I imagine it's actually more often the exception than the rule).

[cclauss]

I am a bit lost in this conversation.

If the current working directory contains a pyproject.toml and that file contains [codespell] configuration, can I just run codespell, or am I required to run codespell --toml? If the latter, will I be required to also add --toml to codespell commands run by Poetry, Flit, pre-commit, tox, GitHub Actions, etc.?

[larsoner]

If the current working directory contains a pyproject.toml and that file contains [codespell] configuration, can I just run codespell, or am I required to run codespell --toml? If the latter, will I be required to also add --toml to codespell commands run by Poetry, Flit, pre-commit, tox, etc.?

Currently you'll be required to add the --toml arg. But if poetry etc. guarantee you're in the dir containing pyproject.toml when running codespell this is a good reason to autodetect and use it. I can make a quick PR for it

[cclauss]

My sense is that Poetry, Flit, Hatch, and pdm require a pyproject.toml file.