black icon indicating copy to clipboard operation
black copied to clipboard

feat(config): Support `--config` in pyproject.toml

Open Shivansh-007 opened this issue 2 years ago • 18 comments

Description

You can now specify config key in pyproject.toml which does the same job as the CLI option --config. The path of config should be relative to the CWD it is running from. If the file is not found or is invalid i.e. it is a folder rather a TOML file it would raise click.exceptions.FileError, and if the file can't be parsed it would raise tomli.TOMLDecodeError. All black config keys in the custom config should be registered under [black] and they would overwrite the config specified in pyproject but not the one specified by --config flag while running black with the CLI.

I also did a small update to .gitignore by grouping the "test" and adding htmlcov to it. Which is generated by coverage html and .pytest_cache/ produced when you run pytest ..

Closes #1826

Checklist - did you ...

  • [x] Add a CHANGELOG entry if necessary?
  • [X] Add / update tests if necessary?
  • [x] Add new / update outdated documentation?

Shivansh-007 avatar Oct 07 '21 08:10 Shivansh-007

Thanks for getting this started as well!

Couple of discussion points:

  • I think a path relative to the original TOML's location should be better, right? That way the run wouldn't depend on where you executed Black, as it is currently.
  • How about just overriding the configuration instead of trying to merge it?

felix-hilden avatar Oct 07 '21 09:10 felix-hilden

I haven't taken a deep look at the code nor played with the feature locally but I do quickly want to mention we should probably somehow indicate we using a configuration file via indirection in verbose mode. Honestly I'm not sure what's up with the weird merging / overriding behaviour here, but I'd suggest to keep it simple for now and just treat a config value in config file (regardless if was explicitly given or implicitly discovered) as a HTTP 302 redirect.

ichard26 avatar Oct 07 '21 22:10 ichard26

Question: Should the config be overwritten if it is passed through the CLI option --config? The config = "black.toml would be found in that file (which is passed through the CLI).

Shivansh-007 avatar Oct 08 '21 00:10 Shivansh-007

I'm confused by what you mean, can you further explain what scenario we're talking about here? Do you mean whether --config at the command line should override just the config value in the implicitly discovered configuration file or instead override the use of that file? In that case I'd suggest to ignore any implicitly discovered configuration files as that's what passing --config at the command line does today.

ichard26 avatar Oct 08 '21 00:10 ichard26

There are two ways configs are loaded, (1) they are passed by the user through --config flag and (2) the pyproject.toml is found in the src. So currently when we find the config key we check whether if it is found by the config found in pyproject.toml or it is found in the one passed by the user via --config option. I am not sure why I did this originally, so I was asking should I keep this check or just overwrite it in any case. I would go with the latter.

Shivansh-007 avatar Oct 08 '21 00:10 Shivansh-007

So currently when we find the config key we check whether if it is found by the config found in pyproject.toml or it is found in the one passed by the user via --config option

Ah this is relevant because you handled those two cases differently. I'd say treat them the same as a HTTP 302 redirect but for configuration for simplicity. If the config key was found in pyproject.toml, ignore the rest and switch to the file specified. Same thing with the --config option, if present, just start the configuration loading process at its filepath. I feel like adding another merging rule on top of the "CLI options override options defined in a configuration file" merge rule will just overcomplicate things. Afterall --config is intended as an explicit way to force Black to use a specific configuration file (instead of searching for one) and nothing more. If the configuration file specified by --config contains a config key, then switch configuration files, kinda like a game of hot potato :p

Also you know we're on Python Discord right? It might be easier to discuss in the #black-formatter channel ^^

ichard26 avatar Oct 08 '21 00:10 ichard26

Yeah, I am on discord also (Jason Terror#XXXX) but currently, off from it until my exams ended, I have a break between the next ones till the 17th so opened these PRs - didn't want to keep them hanging, school ones just finished. And discord as you know, can become a big distraction LOL

Shivansh-007 avatar Oct 08 '21 00:10 Shivansh-007

By chaining do you mean you can specify config in the custom black config file you got from pyproject.toml, and so on (allowing to specify config in that config file...)?

Shivansh-007 avatar Oct 09 '21 00:10 Shivansh-007

This has a merge conflict that I can't resolve in the UI.

JelleZijlstra avatar Jan 27 '22 02:01 JelleZijlstra

diff-shades reports zero changes comparing this PR (eb627aa09d3684f75d75edd3865d78e7d389a4d8) to main (0abe85eebb94e7640aa5d443aefe5b9bed507bfc).


What is this? | Workflow run | diff-shades documentation

github-actions[bot] avatar Jan 27 '22 02:01 github-actions[bot]

This has a merge conflict without an obvious fix, could you take a look?

JelleZijlstra avatar Feb 21 '22 01:02 JelleZijlstra

@ichard26 are you happy with this PR now?

JelleZijlstra avatar Mar 24 '22 02:03 JelleZijlstra

This PR appears to have stalled? It would be great if it could progress since there are a couple of related issues (https://github.com/psf/black/issues/1826, https://github.com/psf/black/issues/2863) it will resolve and make black much easier to configure in a monorepo.

danielmitchell avatar Jun 14 '22 05:06 danielmitchell

Hey all, I'll try to review this soon (especially since IIRC I had some outstanding comments I never shared the last time I looked at this), but I can't make promises.

ichard26 avatar Jun 14 '22 17:06 ichard26

@ichard26 any chance to push it forward?

jaklan avatar Jul 29 '22 09:07 jaklan

I was looking for the usecase described in https://github.com/psf/black/issues/1826 and came across this PR Would be great if this can be released!

If I can help with something to push this forward, I'm happy to help

AbdealiLoKo avatar Nov 23 '22 09:11 AbdealiLoKo

@ichard26 Just a reminder to take a look please.

alfawal avatar Jul 29 '23 00:07 alfawal

See also https://github.com/psf/black/issues/1826#issuecomment-1922891622

hauntsaninja avatar Feb 02 '24 06:02 hauntsaninja