WIP: Added function for vertically aligning inline comments

[rsnk96]

Enforces vertical alignment of inline comments in consecutive lines

Also addresses #379 and partially #1696

Input:

# Inline comments on simple statements
v = 10 #variable 1
my_variable_2 = 20 # variable 2

# Inline comments on multi-line statement
my_dict = {
    "a": 10,  # key a
    "b": 20, # key b
    "c": my_variable_2//2,   # key c
    "d": my_variable_2//10,   # key d
    "e": 50,   # key d
}

Output:

# Inline comments on simple statements
v = 10              # variable 1
my_variable_2 = 20  # variable 2

# Inline comments on multi-line statement
my_dict = {
    "a": 10,                   # key a
    "b": 20,                   # key b
    "c": my_variable_2 // 2,   # key c
    "d": my_variable_2 // 10,  # key d
    "e": 50,                   # key d
}

[rsnk96]

Can someone help me understand why it is bugging out and raising the not equivalent to the source error for the following input (from __init__.py)

matches = re.findall(
    r"""
    (?:[^{]|^)\{  # start of the string or a non-{ followed by a single {
        ([^{].*?)  # contents of the brackets except if begins with {{
    \}(?:[^}]|$)  # A } followed by end of the string or a non-}
    """
)

[JelleZijlstra]

I don't think we want to do this. It's going to be disruptive to existing codebases that use Black, and it's not clear that aligning the comments is always better.

[JelleZijlstra]

Also, your error is because it's changing things inside a string literal, which is not a (Python) comment. If we do this, it should affect only Python comments.

[rsnk96]

Thanks for your review @JelleZijlstra

There are two options on integration which won't always-enforce this (and hence break codebases as you suggested):

1. Add it as an optional flag in black, to enable vertical alignment of inline comments
2. Make it such that inline comments which were previously vertically aligned remain so after formatting with black

Do share your thoughts on these options, I think the first one would make for a good fit

[rsnk96]

Hey @JelleZijlstra

Gentle reminder if you have the time, do give your opinion on what would make for a good integration option from the above^ 😄

[JelleZijlstra]

My opinion is that we shouldn't do it. The strength of Black is a consistent style with few options, and I don't see a strong reason to go against that principle.

Other maintainers may have different opinions.

[rsnk96]

Okay thanks for your take @JelleZijlstra. I'm sorry I haven't contributed to black before, so can you tag some other maintainers who might be interested in reviewing this, just so that I know if I should continue working on this?

I see @ichard26 who assigned a label to #1696, and @carljm @zsol @ambv who gave suggestions on #1696. Sorry folks, if I dragged your name out of the blue, just wanted to know if I should pursue this PR 😅

[cooperlees]

I'll share that I like this since you're asking, but it's not a major win or a must have I feel. I also agree it's another change "black conformant" repos will have CI fail on and I can see it annoying people.

PEP8 is also very loose here:

Use inline comments sparingly.

An inline comment is a comment on the same line as a statement. Inline comments should be separated by at least two spaces from the statement. They should start with a # and a single space.

I think black already ensures there are 2 spaces and a space after the # ... i.e. # ...

[rsnk96]

Thanks for the quick response @cooperlees ! Any thoughts on the alternate modes of integration?

There are two options on integration which won't always-enforce this (and hence break codebases as you suggested):

1. Add it as an optional flag in black, to enable vertical alignment of inline comments
2. Make it such that inline comments which were previously vertically aligned remain so after formatting with black

[cooperlees]

When @ambv set out to make black, we (as I sat next to him @ work) decided one of the main goals was to really limit the number of flags so that the style is actually a consistent style. Not one of X variations that adding flags creates. This is modeled after gofmt and other less configurable autoformatters that have had great success.

Due to this, I vote for 2. If you can detect that they were aligned before formatting and uphold that, then this would truly be a killer feature that I would condone.

[rsnk96]

Thanks, that gives me a direction, will give it a shot

[JelleZijlstra]

For what it's worth, I wouldn't support that change either. I prefer for Black not to take into account previous formatting as much as possible, so people writing code have to worry about formatting as little as possible.

[rsnk96]

Oh, alright. I'll wait for suggestions from other contributors and maintainers in that case! Do tag anyone you might find relevant.

[rsnk96]

so people writing code have to worry about formatting as little as possible

P.S. just to clarify why I opened this PR, I like black not just because it takes care of formatting, but also because it makes code a lot more readable. Just thought this feature would be a good addition for the latter reason, I find py code with aligned inline comments very easy to interpret :)

But I do understand that it shouldn't introduce complications since black is already widely adopted. Will await comments from other contributors and maintainers

[ichard26]

I wasn't part of this project when its philosophy and design goals were hashed out (I only joined mid-way through 2020!) but here's my thoughts (for whatever weight they may or may not carry):

Honestly I'm not a big fan of either option. Adding a new option / flag is a big no-no since it goes against the philosophy of Black: to be as non-configurable as possible so style-configuration wars don't break out and waste time. For the second option, I still have an aversion towards it because it still goes against Black's philosophy in a way IMO. Flags directly introduce variants, but previous-formatting-dependent formatting (note-to-self: need a better term) still introduces variants. Except instead of being explicit as you can just look at the Black config and be like "Oh X option is turned, that's why Black does Y formatting", you must read the docs for an explication why Black is doing something unexpected: "Wait what, why did Black format this like this? I know there isn't any config for it..." Black tries to make you stop worrying about formatting and variants (especially hidden variants) breaks that illusion since your expectations end up broken. It boils down to that Blackened code loses a bit of its consistency and its 'uncompromising' nature with either. Being uncompromising is Black's strength and I don't think weakening it is worth it in most cases.

*But*, there's an argument to favour practicality over purity. And I see the value of supporting aligned inline comments, especially given that sometimes they are critical to the understanding of some code. Like keeping aligned comments still aligned after formatting sounds nice; and I do feel you when Black makes my code look (subjectively) worse and less readable. It sucks, but I find Black's advantages to be worth it.

I guess a list of (relevant to this discussion) priorities an autoformatter like Black must content with are:

1. Consistency
2. Readability
3. Practicality

Personally I hold 1. as the most important thing to work towards. It's what made Black unique, but Black isn't a tiny project. It plays a huge role in the Python ecosystem and maybe keeping 1. as the highest priority isn't worth it anymore. I don't think Black was intended to be *the* Python autoformatter, ¹ but Black is not getting less popular and influential anytime soon so... hmm

I'm split but for now consider me a weak -0.5 for this functionality not because it's a bad idea (I actually like it personally), but because of the slippery slope it can push us down. Feel free to convince me otherwise!

---

1. I don't mean to sound ungrateful being part of a huge project like Black. It's a honour that I get to help out and sometimes change the direction the project is going through my participation. I just feel like Black is too opinionated to where it isn't intended to be useful (and maybe shouldn't try to be useful) to all of the general Python developer community at large. It's great a lot of the community finds it worth it though.

[felix-hilden]

Just to provide one more opinion to the list, I agree with the previous comments in that I'm 100 % against this being the default or configurable. I'm not completely against Black detecting existing aligned comments, but I'd want a stronger go-ahead from other maintainers to actually pursue that.

[ambv]

Thanks for your work on Black but as the previous maintainer comments tell you, this is out of scope for inclusion.

[li1234yun]

Why assume that the current black configuration is optimal? It may be necessary to vote to evaluate the controversial areas for further optimization, rather than complacency.

Maybe most people think alignment is good! So We should not assume and need to evolve！

Black is very good, but it maybe not be perfect. It needs to evolve.

Thank you for providing so good tool!

[Anti-Distinctlyminty]

My comment may be parochial, but I did end up here looking for a solution to this, so with that in mind...
This is one of the very rare times that Black takes my code and makes it less readable.

[dseynhae]

It looks like I'm a day late and a dollar short here.
I would love to see Black used by my colleagues, as it makes the diff between different authors working on the same code independent of any configuration options. There aren't any configuration options...

But I'm dealing with people that are against formatters in general, not even Black specifically. The arguments are always about readability... In some cases, consistency is NOT the most important thing.

• I agree that we shouldn't make the alignment of comments configurable (this objection is also advocated by @cooperlees and others).
• And I think it is reasonable to maintain existing alignment (which I think was the suggestion from @rsnk96).

When I was reading the documentation on how Black deals with comments, I thought that it was already resolved:

Some types of comments that require specific spacing rules are respected: doc comments (#: comment),

So I tried it:

proc = subprocess.run(
    cmd,
    capture_output=True,  #:PIPE results to stdout and stdin
    shell=True,           #:Command is specified as a string
    text=True,            #:Open stdout and stdin in text mode
    check=False,          #:Raise CalledProcessError exception
)

👉 I was hoping that the #: syntax had some alignment power... But Black 22.3.0 turned this promptly into an obvious mess:

proc = subprocess.run(
    cmd,
    capture_output=True,  #:PIPE results to stdout and stdin
    shell=True,  #:Command is specified as a string
    text=True,  #:Open stdout and stdin in text mode
    check=False,  #:Raise CalledProcessError exception
)

💡 But wouldn't this #: comment be a nice compromise?

• No flag needed.
• Existing code is not affected (unless the rare case where the comment starts with #:)
• The alignment is not based on the alignment existing in the existing code (taking care of @JelleZijlstra 's objection)
• Alignment is an intent. And this intent can now subtly yet clearly be communicated. And enforced by Black...

[aldanor]

Agreed with @dseynhae here, having a clean and easy way of communicating to black "please, don't mess up alignment of these" would be very nice. And something like a #: prefix should be good enough indeed.

Are there any serious objections? Could this issue be reopened or is it closed for good and is not open for discussion?