Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature/BLACK_CONFIG: Black configuration file configurable #11

Merged
merged 2 commits into from Aug 6, 2019

Conversation

098799
Copy link

@098799 098799 commented Aug 5, 2019

Added flake8-black-config parameter to be set in flake8
configuration file, which points to the .toml file that should be
used instead of the default black pyptoject.toml.

@098799 098799 force-pushed the feature/BLACK_CONFIG branch 4 times, most recently from 49ab7da to 977ccdc Compare August 5, 2019 14:49
flake8_black.py Outdated Show resolved Hide resolved
@098799 098799 marked this pull request as ready for review August 5, 2019 14:52
flake8_black.py Outdated
type="string",
parse_from_config=True,
help=(
"A path to the global configuration file for ``black``. This file will"
Copy link
Owner

@peterjc peterjc Aug 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about something shorter:

help="Path to black configuration file (by default looks for pyproject.toml)"

Copy link
Author

@098799 098799 Aug 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh it sounds a bit confusing to me, but I'd be fine with: Path to black configuration file (overrides the default pyproject.toml)

@peterjc
Copy link
Owner

peterjc commented Aug 5, 2019

If the black configuration given in the .flake8 (etc) file is a relative path, as a user I would expect this to be interpreted relative to the location of the configuration file (not relative to the current directory which is my guess for what happens with the current code). Does that seem like a good idea?

Or we could just insist on an absolute path?

@peterjc
Copy link
Owner

peterjc commented Aug 5, 2019

This pull request would close issue #9.

@098799
Copy link
Author

098799 commented Aug 5, 2019

If the black configuration given in the .flake8 (etc) file is a relative path, as a user I would expect this to be interpreted relative to the location of the configuration file (not relative to the current directory which is my guess for what happens with the current code). Does that seem like a good idea?

Or we could just insist on an absolute path?

Definitely sounds like an improvement.

@098799 098799 force-pushed the feature/BLACK_CONFIG branch 2 times, most recently from bbd89b8 to 124a723 Compare August 5, 2019 15:45
@098799
Copy link
Author

098799 commented Aug 5, 2019

I think I covered all the comments, please recheck when you have the time.

@peterjc
Copy link
Owner

peterjc commented Aug 5, 2019

Feel free to bump the version number to v0.1.1 and add a line to the release history in README.rst? Something like:

Option to use a (global) black configuration file, contribution from Tomasz Grining.

Otherwise I'll probably merge this and do that tomorrow. Thanks!

Tomasz Grining added 2 commits August 6, 2019 09:25
Added `flake8-black-config` parameter to be set in flake8
configuration file, which points to the `.toml` file that should be
used instead of the default black `pyptoject.toml`.
@098799
Copy link
Author

098799 commented Aug 6, 2019

Ok, squashed commits and added the version bump.

@peterjc
Copy link
Owner

peterjc commented Aug 6, 2019

Looks good. Time to try and break it in local testing 😁

@peterjc
Copy link
Owner

peterjc commented Aug 6, 2019

Looks like something isn't quite right in the new configuration option,

$ black -h

This suggests a trivial test to add to tests/run_tests.sh,

flake8 -h 2>&1  | grep "black-config"

@peterjc
Copy link
Owner

peterjc commented Aug 6, 2019

Easy fix - string vs tuple for the help argument

@peterjc peterjc merged commit 77ddf99 into peterjc:master Aug 6, 2019
@peterjc
Copy link
Owner

peterjc commented Aug 6, 2019

Merged, thank you 🎉

@098799
Copy link
Author

098799 commented Aug 6, 2019

Ugh, sorry about it, flake8-strict has terrorized me about always adding those silly commas and in some places it changes the meaning.
Ready to embrace black's trailing comma handling...

@098799
Copy link
Author

098799 commented Aug 7, 2019

@peterjc Will you release this version on pypi or are we waiting for figuring out the issues with relative paths etc?

@peterjc
Copy link
Owner

peterjc commented Aug 7, 2019

I wanted to include the bad TOML error handling as part of the release, and ideally the path issue too (putting out a release only to change something like how the configuration is interpreted in the next release is a bad idea).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants