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

Configurable Indent Levels? #378

Closed
kwlzn opened this issue Jun 25, 2018 · 6 comments
Closed

Configurable Indent Levels? #378

kwlzn opened this issue Jun 25, 2018 · 6 comments

Comments

@kwlzn
Copy link

kwlzn commented Jun 25, 2018

before lobbing a PR over, I wanted to spin up a quick discussion around adding an option to black to permit smoother interop with 2-space indent code bases.

At Twitter, we would like to experiment with adopting black as autofmt tooling in a few of our open source projects and potentially more widely across our internal monorepo. Our python code style is currently defined as such:

Python code style at Twitter closely follows PEP-8 and PEP-257 with the following modifications:

- Indent using two spaces (instead of four).
- Break lines at 100 characters or less.

we have on the order of many millions of lines of code that currently adopt this style and generally feel like it's a net improvement over 4 space indent (nested blocks can fit more code per line, etc etc). pants and pex are good concrete examples of projects that started at Twitter and were later open sourced with a continuation of this python code style.

I realize black is intended to be opinionated and have little to no config - but to me, line length and indent level both seem like a fairly fundamental configurable for most shops, so I'd put this in a very similar category as the existing --line-length option:

-l, --line-length INTEGER   Where to wrap around.  [default: 88]

with that said, how would folks feel about adding a new --indent-level={2|4} flag to black to permit easier interop for code bases that take this fundamental stance? I would be happy to cut a PR that implements this as long as there's consensus that this would be reasonable.

@ambv
Copy link
Collaborator

ambv commented Jun 25, 2018

Please consider dropping two-space indents. Black will not support this style.

@kwlzn
Copy link
Author

kwlzn commented Jun 25, 2018

k, thanks.

@kwlzn kwlzn closed this as completed Jun 25, 2018
@pausan
Copy link

pausan commented Sep 11, 2018

Łukasz I've been playing with Black for a while, and I'm pretty sure, that looking at the aim of the project you'll have a strong position about why something might or might not be accepted, since basically as the project states, this is an opinionated code formatter, right?

Actually, when I look at the output, the thing is that I can agree on some formatting and maybe dislike or not be completely comfortable with other formatting options. But guess what... that's the aim of the project, to have an opinionated standard that people might or might not like, so if you like it you use it, and if you don't then... you'll go for yapf, autopep8, or any other formatter.

The thing is, I've been experimenting personally with several formatters, and several options for some formatters, and black seems the one that fits better if only for one small, tiny thing.... 4 spaces. Please don't reply quite yet and bear with me on the next paragraphs.

At this point, if I haven't missed anything, in black, there are basically three arguments that change the output of the program:

  • string normalization
  • trailing commas for python 3.6+
  • line length

If you think about it, the formatting of the code, the actual text, expressions, function calls, definitions, ... are only affected by string normalization and by trailing commas.

Line length affects spacing and affects indentation, but it does not change formatting rules. Why do I say that line length affects indentation? Basically because if you have a line length of 80 and the function call has 6 parameters and ends in the column 95, then it will get indented in multiple lines. If for example, you rerun black with a line length of 100, then the indentation of the script will change, it will fit in one line and won't get indented the same way. Still the formatting rules have not changed.

Now back to spaces. Spaces affect indentation, they don't affect formatting rules either. Over the years I've seen basically 2 kind of spacing standards used in projects with 3 flavours each:

  • tabs (configured to either 2, 4 or 8 chars)
  • spaces (also configured to either 2, 4, 8)

In most cases, using tabs has a major drawback over spaces. Main drawback is that different editors with different configurations might display code with different number of spaces, thus when a programmer opens the file with a different tab length than the original programmer, the file might look broken. So I totally agree of going for spaces instead of tabs and forcing that choice.

Now, regarding the different spacing options:

  • 8 spaces: I would disallow this option, since using an indentation of 8 spaces is adding a lot of blank space while it does not add readability.
  • 4 spaces: it is readable and indentation does not consume as much space as 8 space characters does, so I understand that some people chose it over 2 spaces.
  • 2 spaces: it is readable and indentation consumes half the spaces as previous option, to me this is the minimum number of spaces a coding standard should allow.

Even though 4 spaces has been chosen over 2, and that's why black forces this amount of spaces on the first place, I see no difference in being able to chose spacing vs being able to chose line length. I mean, there should be a default, and the default might be 4 spaces, like for line length it is 88, but if there is an option to adjust and customize the line length, then why there is no option to adjusting the spaces?

Spaces, like line length, only change how the code will get indented, but won't change any actual formatting rules...

To stay consistent, I only see two alternatives, either to allow 2 spaces or to remove line-length altogether, since if everything was opinionated, then there should be no option for trailing commas or string quotes or line length.... but in fact those parameters exist, and I really like to be able to chose line length. And I understand that different companies or different projects might have chosen a line length for one reason or the other (e.g different screen sizes, accessing code through ssh, ...). And none of the existing parameters really change how the code gets formatted in the end. Even with string quotes, code looks the same, all characters in same position, only one quote is replaced by the other.

Can you please reconsider your reasoning for not allowing 2 spaces as well?

(as a final note: I actually made a change locally to test black with 2 spaces, and I only had to removing 2 characters in black.py, so I guess adding an option for this optional behaviour should not be hard, in fact, if needed I offer myself volunteer to do the changes and add the parameter if you would reconsider and accept the pull request).

@ambv
Copy link
Collaborator

ambv commented Sep 13, 2018

Thanks for your detailed comment. Note you are commenting on a closed issue. Your attempt to use --line-length= as a precedent for allowing two-space indents is understandable but I disagree with it.

Line length controls what happens on the right of the line, whereas a different indentation level would change what happens on the left of the line. The former affects some lines whereas the latter would affect all lines.

Two space indents are not distinct enough to be recommended by the Black coding style.

@pausan
Copy link

pausan commented Sep 15, 2018

True. I'm also aware the issue is closed, but I wanted to make my point to see if it could be reopened and reconsidered. It won't make sense to open a new issue if the topic is going to be the same as this one.

Just for a final argument, consider the choice of 88 characters by default. It is an interesting choice made because:
"This number was found to produce significantly shorter files than sticking with 80 (the most popular), or even 79 (used by the standard library)."

Shorter files in number of lines... I'm sure indentation level of 2 also produces shorter files.

I mean, we come back to styling choices which are subjective. If you prefer 4-space indented files and you don't want to provide an option in black for 2 spaces, it is your project and your choice, I just though it would be an interesting addition that could potentially benefit the project, spreaden its adoption and still preserve its essence & formatting rules. There is nothing else I can say. Totally your choice.

I hope you can re-reconsider, but otherwise thanks for your time reading and answering these thoughts! The project is still really cool.

@psf psf deleted a comment from pausan Oct 29, 2018
@ghost
Copy link

ghost commented Jan 17, 2019

For those of you who want to try black, but only want 2 spaces indentation, I have made a fork: https://github.com/desbma/black-2spaces

Why can both 2 and 4 space supporters not just agree to compromise on 3?

@psf psf deleted a comment from desbma Jan 23, 2019
@psf psf locked as resolved and limited conversation to collaborators Jan 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants