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

Documentation: Comparison between Black and WPS #1196

Closed
DonaldTsang opened this issue Dec 8, 2019 · 18 comments
Closed

Documentation: Comparison between Black and WPS #1196

DonaldTsang opened this issue Dec 8, 2019 · 18 comments
Labels
T: user support OP looking for assistance or answers to a question

Comments

@DonaldTsang
Copy link

DonaldTsang commented Dec 8, 2019

As wemake-services/wemake-python-styleguide#1059 have noted
(and as https://wemake-python-stylegui.de/en/latest/pages/usage/integrations/auto-formatters.html#black have referenced), there are a few noticeable difference between Black and WeMake Python StyleGuide. Here are some questions:

  • Is it possible to set line length maximums as 79/80 characters?
  • Is it possible to preserve trailing comma as a default setting?
  • Is it possible to treat single quote as the standard instead of double quote?
  • Is it possible to have black+WPS instead of autopep8+WPS as a standard?
@hugovk
Copy link
Contributor

hugovk commented Dec 8, 2019

Black intentionally has very few options. See https://github.com/psf/black#command-line-options.

Is it possible to set line length maximums as 79/80 characters?

Yes, see --line-length: https://github.com/psf/black#line-length

  • Is it possible to treat single quote as the standard instead of double quote?

No, but you can use --skip-string-normalization to prevent changing existing singles to doubles. See https://github.com/psf/black#strings.

@zsol zsol added the T: user support OP looking for assistance or answers to a question label Dec 8, 2019
@DonaldTsang
Copy link
Author

@hugovk what about preserving trailing commas in lists and objects (since that is a convention adopted by many projects)?

@hugovk
Copy link
Contributor

hugovk commented Dec 8, 2019

If it's not in the documentation, then no.

I think there is a PR related to trailing commas, but I don't think there'll be an option.

@DonaldTsang
Copy link
Author

@hugovk would like to see the PR exist though, since that would allow maximum compatibility with WPS, turning it into a powerhouse.

Also some have suggested that the max line length limit needs to be hardened (see wemake-services/wemake-python-styleguide#518 and wemake-services/wemake-python-styleguide#890 )

@ferdnyc
Copy link

ferdnyc commented Dec 29, 2019

It strikes me that https://wemake-python-stylegui.de/en/latest/pages/usage/integrations/auto-formatters.html#black is out of date, because according to the Black docs (emphasis mine):

Trailing commas

Black will add trailing commas to expressions that are split by comma where each element is on its own line. This includes function signatures.

Unnecessary trailing commas are removed if an expression fits in one line. This makes it 1% more likely that your line won’t exceed the allotted line length limit. Moreover, in this scenario, if you added another argument to your call, you’d probably fit it in the same line anyway. That doesn’t make diffs any larger.

One exception to removing trailing commas is tuple expressions with just one element. In this case Black won’t touch the single trailing comma as this would unexpectedly change the underlying data type. Note that this is also the case when commas are used while indexing. This is a tuple in disguise: numpy_array[3, ].

One exception to adding trailing commas is function signatures containing *, *args, or **kwargs. In this case a trailing comma is only safe to use on Python 3.6. Black will detect if your file is already 3.6+ only and use trailing commas in this situation. If you wonder how it knows, it looks for f-strings and existing use of trailing commas in function signatures that have stars in them. In other words, if you’d like a trailing comma in this situation and Black didn’t recognize it was safe to do so, put it there manually and Black will keep it.

@DonaldTsang
Copy link
Author

DonaldTsang commented Dec 29, 2019

Is it possible to enforce trailing commas instead of removing them? Assuming that that are out-of-date and that we are trying to be WPS-compliant (since they stated that https://github.com/asottile/add-trailing-comma is perfect for WPS)? Also if you are free, is it possible for us to collaborate over at the Issue page in WPS?

@ferdnyc
Copy link

ferdnyc commented Dec 29, 2019

@DonaldTsang According to the add-trailing-comma readme, it does exactly the same thing as Black for one-line lists:

remove unnecessary commas

yes yes, I realize the tool is called add-trailing-comma 😆

-[1, 2, 3,]
-[1, 2, 3, ]
+[1, 2, 3]
+[1, 2, 3]

@DonaldTsang
Copy link
Author

DonaldTsang commented Dec 29, 2019

@ferdnyc if you are free, is it possible for us to collaborate over at the Issue page in WPS?

Also @hugovk is it possible for us to "force" single quotes instead of just allowing single quotes? It should be simple to regex "'\"" => '\'"' where single quotes have their escapes removed and double quotes have escapes to go with them?

Have anyone tested this statment's validity yet?

black itself violates a lot of flake8 rules that this linter respects.

@hugovk
Copy link
Contributor

hugovk commented Dec 29, 2019

Also @hugovk is it possible for us to "force" single quotes instead of just allowing single quotes? It should be simple to regex "'\"" => '\'"' where single quotes have their escapes removed and double quotes have escapes to go with them?

No, this has been discussed many times and it's unlikely Black will change.

You could use the double-quote-string-fixer pre-commit hook.

@ferdnyc
Copy link

ferdnyc commented Dec 29, 2019

@DonaldTsang The info I posted here is the same information (it turns out) that @uhbif19 included when opening wemake-services/wemake-python-styleguide#890 over two months ago. Seems to me the problem isn't that information is needed, it's that it needs to be acted on by someone involved in WPS. I have nothing more to add, really.

@DonaldTsang
Copy link
Author

DonaldTsang commented Dec 29, 2019

@hugovk is it possible to make the fixer a plugin to black or a separate program? Not everyone uses pre-commit hooks in their workflow.

@hugovk
Copy link
Contributor

hugovk commented Dec 29, 2019

Not as a plugin to Black, but you can run pre-commit as a standalone tool without actually using installing the pre-commit hooks. See the CI in this repo, for example.

@ambv
Copy link
Collaborator

ambv commented Mar 3, 2020

You can use --line-length= and -S. Preserving trailing commas will work consistently when #1288 is finished.

@ambv ambv closed this as completed Mar 3, 2020
@DonaldTsang
Copy link
Author

DonaldTsang commented May 10, 2020

@ambv Can you confirm the edited documentation in https://hackmd.io/3vzJNofVQ_G0aS_QWwvLQA in case I make any errors?

Also to @ferdnyc there has been more discoveries with such incompatibilities wemake-services/wemake-python-styleguide#890 (comment)

@ferdnyc
Copy link

ferdnyc commented May 10, 2020

Please do not tag me on this issue, I have no desire to be involved any further.

@ichard26
Copy link
Collaborator

@DonaldTsang

And there’s no configuration to fix it!

This is partially true, the comments about Black's usage of double quotes and unstable trailing comma enforcement are valid. What is not valid is the comment about line length. There is a way to configure it, it can be done via --line-length=80. I get that having to configure it can be annoying, but to imply that it is not configurable is misleading.

Also a slight nitpick, -S won't really fix the single quotes vs double quotes issue. For the foreseeable future, Black will not be enforcing single quote usage. You can disable the auto-formatting of quotes via -S, but not configure the auto-formatting to enforce single quotes. Instead, I would recommend using a different tool to enforce single quotes.

@DonaldTsang
Copy link
Author

@ichard26 I think I have redacted that specific line in the HackMD document prior to your comment. Other than that, I think that there might be tools to switching from enforcing single quotes to enforcing double quotes like "double-quote-string-fixer".

@ichard26
Copy link
Collaborator

ichard26 commented May 10, 2020

@DonaldTsang, sorry about that, I was looking the version hosted at readthedocs since it looked better formatted and for other reasons... oops.. Anyway, technically it LGTM. Original comment about -S still stands though, it's more like a hack than a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: user support OP looking for assistance or answers to a question
Projects
None yet
Development

No branches or pull requests

6 participants