black icon indicating copy to clipboard operation
black copied to clipboard

Line too long: Comments

Open anirudnits opened this issue 4 years ago • 30 comments

Problem

Consider the below comment: # this comment is longer than what is specified in line length arguments passed to black blah blah blah blah blah blah

If I run black on this file the comment remains intact, even though its clearly over the line length limit.

Split the comment to fit into the line length

Something like # this comment is longer than what is # specified in the line length ... ... The same problem also exists for strings as well but I believe that's better left intact but for comments I think splitting them up into multiple lines makes sense.

If this is a feature that you guys believe will be a good addition to black, then I can work on this and open a PR.

Thanks

anirudnits avatar Sep 16 '20 16:09 anirudnits

I would love to solve this issue

SandeepHG avatar Sep 22 '20 10:09 SandeepHG

and for situations like

x = 1
z = x + 2  # a long comment blah blah blah ... blah and second line part - so you know it's long
print(z)

does this become:

x = 1
# a long comment blah blah blah ... blah and second line 
# part - so you know it's long
z = x+2
print(z)

or

x = 1
z = x + 2  # a long comment blah blah blah ... blah
           # and second line part - so you know it's long
print(z)

jvahue avatar Dec 03 '20 19:12 jvahue

I'd like to see this change. Maybe a reasonable first step would be to only format comments that are on a dedicated line, so splitting should cause no issues whatsoever? And in line with the upcoming string processing, should short comments also be combined into a single line? There probably are some complications with that though.

# this
# is
# redundant
---
# this is redundant

Just for reference, #2306 (which fixes #1017) will add a section to the documentation for comments that should be changed accordingly if this is implemented.

felix-hilden avatar Jun 04 '21 13:06 felix-hilden

This seems risky. Comments are often commented out code or other structured data. I think we should not merge short comments, but splitting long ones is probably OK.

JelleZijlstra avatar Jun 04 '21 14:06 JelleZijlstra

I would also really love to have this made possible, maybe as a default off option (although Black doesn't like having too many options, which I respect). For what it's worth, ClangFormat does reflow comments so they fit within the specified line length, and having used it for a few years now, it rarely causes any annoyance.

mortenfyhn avatar Jun 10 '21 09:06 mortenfyhn

I would go for a config option, something like split-singleline-comments, because as already mentioned in the thread there are complications when it comes to splitting in-line comments and I would like if a default option covered both the cases to avoid confusion.

Also if no-one is currently working on this, can you assign this issue to me, so I can work and open a PR.

anirudnits avatar Jun 10 '21 09:06 anirudnits

Most likely a new configuration option is a no-go. But seems like there is some acceptance for just splitting them, despite the edge cases. As I said, for starters we could do it for dedicated comment lines only, not inline comments. One discussion that should be had is the style of split. I quite like equalising the line lengths if one would be radically shorter. But that probably is also related to string processing, which I'm not familiar with (yet).

# this is a very long comment that should probably be on multiple lines if I was to decide

# this is a very long comment that should probably be on multiple lines if
# I was to decide

# this is a very long comment that should
# probably be on multiple lines if I was to decide

So I'd prefer the last option.

felix-hilden avatar Jun 10 '21 09:06 felix-hilden

[...]

# this is a very long comment that should probably be on multiple lines if I was to decide

# this is a very long comment that should probably be on multiple lines if
# I was to decide

# this is a very long comment that should
# probably be on multiple lines if I was to decide

So I'd prefer the last option.

Just to make this more difficult: I prefer the first. That will generally make diffs smaller, if say I decide to append some more text.

mortenfyhn avatar Jun 10 '21 09:06 mortenfyhn

That's a fair point as well!

felix-hilden avatar Jun 10 '21 09:06 felix-hilden

@felix-hilden Make ssense then to make this a default behavior, the inline scenario needs to be specified in the docs. I also prefer the 2nd one, but I believe that would require separate computation to calculate the lengths and then split the comments, I don't know how black processes comments but that could add some latency to generate the output.

Either way, I like the problem in itself, to think algorithmically and come up with an optimal solution.

anirudnits avatar Jun 10 '21 09:06 anirudnits

Honestly I'd be tempted to say that it is bad code style to even have long inline comments. So leaving them out of formatting would be more convenient, but then again Black is specifically built to not only enforce but help in achieving a certain style, so we should also try to format them. But I might be thinking too narrowly 😅

felix-hilden avatar Jun 10 '21 09:06 felix-hilden

As stated by @jvahue,

and for situations like

x = 1
z = x + 2  # a long comment blah blah blah ... blah and second line part - so you know it's long
print(z)

does this become:

x = 1
# a long comment blah blah blah ... blah and second line 
# part - so you know it's long
z = x+2
print(z)

or

x = 1
z = x + 2  # a long comment blah blah blah ... blah
           # and second line part - so you know it's long
print(z)

I don't know if either seem "right" to me.

anirudnits avatar Jun 10 '21 10:06 anirudnits

Just for reference, here are some examples of how ClangFormat handles inline comments:

int main() {

  int fairly_long_variable_name_1 = 9384759837459; // Short inline comment.

  int fairly_long_variable_name_2 =
      9384759837459; // Slightly longer inline comment.

  int fairly_long_variable_name_3 =
      9384759837459; // Really long inline comment, so long that it needs two
                     // lines.
}

Maybe this behaviour is good? It allows inline comments to be infinitely long if you want to, but it's not super pretty (I don't think there is a pretty way to format this), so that might discourage very long inline comments.

mortenfyhn avatar Jun 10 '21 10:06 mortenfyhn

I'll say that I like the way that clang-format does it decently well and doing it their way would make what black does familiar to many users. If that output gets truly unwieldy, the user can just make it not be a block comment.

int main ()
{
  int long_variable_name_with_inline_comment =
    9384759837459;  // Lorem ipsum dolor sit amet, consectetur adipiscing elit,
                    // sed do eiusmod tempor incididunt ut labore et dolore
                    // magna aliqua. Ut enim ad minim veniam, quis nostrud
                    // exercitation ullamco laboris nisi ut aliquip ex ea
                    // commodo consequat. Duis aute irure dolor in reprehenderit
                    // in voluptate velit esse cillum dolore eu fugiat nulla
                    // pariatur. Excepteur sint occaecat cupidatat non proident,
                    // sunt in culpa qui officia deserunt mollit anim id est
                    // laborum.

  // Lorem ipsum dolor sit amet, consectetur adipiscing elit,
  // sed do eiusmod tempor incididunt ut labore et dolore
  // magna aliqua. Ut enim ad minim veniam, quis nostrud
  // exercitation ullamco laboris nisi ut aliquip ex ea
  // commodo consequat. Duis aute irure dolor in reprehenderit
  // in voluptate velit esse cillum dolore eu fugiat nulla
  // pariatur. Excepteur sint occaecat cupidatat non proident,
  // sunt in culpa qui officia deserunt mollit anim id est
  // laborum.
  int long_variable_name_with_block_comment = 9384759837459;
}

fpdotmonkey avatar Jun 18 '21 19:06 fpdotmonkey

and for situations like

x = 1
z = x + 2  # a long comment blah blah blah ... blah and second line part - so you know it's long
print(z)

does this become:

x = 1
# a long comment blah blah blah ... blah and second line 
# part - so you know it's long
z = x+2
print(z)

or

x = 1
z = x + 2  # a long comment blah blah blah ... blah
           # and second line part - so you know it's long
print(z)

I would say neither. Moreover, if you have to break in-line comments, the latter option is the most reasonable in terms of aesthetics.

caniko avatar Jun 29 '21 10:06 caniko

I want this feature so much, but I don't think it is possible to do it in a sensible way.

Does this

# This is me very long comment that is too long for one line.

format to this?

# This is me very long comment that is too long for one 
# line.

But what if I remove the very from the last comment

# This is me long comment that is too long for one 
# line.

so that it fits now into one line? Does the comment stay as it is, even if it fits onto one line? Or does it change to

# This is me long comment that is too long for one line.

If the latter should hold, what happens with stuff like this:

#! /bin/python
# test
# 
# test

Also, if you for example use jupytext, black could scramble its metadata like

# ---
# jupyter:
#   jupytext:
#     cell_metadata_filter: -all
#     text_representation:
#       extension: .py
#       format_name: percent
#       format_version: '1.3'
#       jupytext_version: 1.11.5
#   kernelspec:
#     display_name: Python
#     language: python
#     name: test
# ---

or markdown cells

# %% [markdown]
# # Title
#
# # # Another title
#
# - the data comes from an autoregressive model with moderate autocorrelation
# - another bullet point
#
# And a formula:
# $$x \sim \mathcal{N}(0, 1)$$

In summary, I don't think formatting comments should be done by black. There are just too many possibilities for how comments can be structured. Also, there are no PEP guidelines (or sth equivalent) telling us how it is done correctly.

cgahr avatar Sep 08 '21 09:09 cgahr

My opinion:

I want this feature so much, but I don't think it is possible to do it in a sensible way.

Does this

# This is me very long comment that is too long for one line.

format to this?

# This is me very long comment that is too long for one 
# line.

Yes.

But what if I remove the very from the last comment

# This is me long comment that is too long for one 
# line.

so that it fits now into one line? Does the comment stay as it is, even if it fits onto one line? Or does it change to

# This is me long comment that is too long for one line.

It stays as it is, and you can manually rewrite it if you want it back on one line.

ClangFormat does it this way, and in my experience it works well.

mortenfyhn avatar Sep 08 '21 11:09 mortenfyhn

The concern raised about commented code goes the other way too. If we have a line that is just under the limit, and is then commented, bringing it over the limit, making a new line could break it. Maybe it won't, but for example this will:

if longgggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg:
    pass

# Commented:
# if longgggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg:
#     pass

# And reformatted for LL:
# if
# longgggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg:
#     pass

# Uncommented and invalid syntax:
if
longgggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg:
    pass

felix-hilden avatar Sep 08 '21 11:09 felix-hilden

The concern raised about commented code goes the other way too. If we have a line that is just under the limit, and is then commented, bringing it over the limit, making a new line could break it. Maybe it won't, but for example this will:

That's a valid concern, and different from other languages (which would just parse the still valid code and reformat it properly if it gets wonky when un-commenting some code).

I still think this is fine, though. When you uncomment some commented out code, it's not unreasonable to expect that you also fix it if its formatting gets weird.

mortenfyhn avatar Sep 09 '21 10:09 mortenfyhn

Wouldn't splitting inline comments upset linters? For example, with mypy:

# This works...
import exchange_calendars as some_really_overly_long_name_just_to_offer_example  # type: ignore[import]

# This doesn't...
import exchange_calendars as some_really_overly_long_name_just_to_offer_example  # type:
# ignore[import]

maread99 avatar Nov 04 '21 12:11 maread99

What's the status of this issue?

In my project, we run black to reformat our files. Then we run pylint, which complains about the long comment lines, and we have to manually (!!) shorten them one-by-one. It always feels strange that I have to do manually something that can be so easily automated, especially considering that we do use a tool (black) whose job is exactly that, to automatically reformat my files.

I understand that there may be complications that I don't see, and it's fine to have it as a non-default option. I'll be happy to see it happen.

yaelbh avatar Jan 31 '22 11:01 yaelbh

@yaelbh are you saying that Black is causing the lines to become too long? As I understand it, this issue is about comments that are too long by themselves, not comments that get put on one line.

I am pretty skeptical about should be splitting up comments. @ConstantinGahr's comment (https://github.com/psf/black/issues/1713#issuecomment-915096161) gives some good examples where it's problematic.

JelleZijlstra avatar Jan 31 '22 15:01 JelleZijlstra

I think the main issue is that Black fixes all of your too-long code lines, but not your too-long comment lines. So if you format to some line length with Black, and then enforce line length with another tool (which is reasonable), you'll end up with the comment lines still being too long, and your check will fail.

It is possible for a tool like Black to re-wrap comment lines, as other code formatter tools already do. If so, it should be an opt-in feature, because it will mess up comments in codebases where comments don't adhere to a max line length. I absolutely think it would be useful.

I also think we've discussed more or less all sides of this, and the next step should just be to decide whether Black should have this functionality or not.

mortenfyhn avatar Jan 31 '22 15:01 mortenfyhn

I'm becoming slightly in favor of having this as an opt-in. Either that, or not at all. We could start by just splitting lines that only have comments, to get rid of some complexity. That shouldn't be too hard.

felix-hilden avatar Jan 31 '22 15:01 felix-hilden

I'd like to state, that if we take care of some of the simple scenarios here, that it becomes next years default and it's only gated by the --preview flag. I'm still a big proponent of as little formatting configuration options as possible please.

cooperlees avatar Jan 31 '22 16:01 cooperlees

@JelleZijlstra

@yaelbh are you saying that Black is causing the lines to become too long?

No, it's the other option that you state, of comments that are long in the first place. I expect a formatting tool like Black to split them for me. I understand that it can be problematic sometimes, but it's also a burden to have to split them by myself.

yaelbh avatar Jan 31 '22 16:01 yaelbh

i agree, black behaves very oddly with comments

while small_cond:  # long comment here...
    do()

becomes

while (
    small_cond: 
):  # long comment here...
    do()

which neither improves readability, nor keeps the comment on the same line as either the while loop or the condition, which it may have referred to

with regards to comments, i would recommend:

  • moving comments to the previous line before splitting
  • breaking up comments if they are too long for one line
  • avoiding any splitting on lines with linter rules (pylint, noqa)

IE do this

# long comment here...
while small_cond:
    do()

Or this:

while small_cond:  # pylint: long comment breaks line length rules... fix it by hand
    do()

earonesty avatar Mar 11 '22 18:03 earonesty

Just checking in as its been a while... are there any plans to add support for shortening lines? Will there be at least an optional way to have black shorten too long lines?

AniAggarwal avatar Apr 22 '22 22:04 AniAggarwal

For comment lines specifically, there is a good chance we'll never split them. There are other issues in https://github.com/psf/black/labels/F%3A%20linetoolong that we should fix though.

JelleZijlstra avatar Apr 22 '22 22:04 JelleZijlstra

Maybe we just need to write a separate tool for this, outside of Black? While I personally would like to have this feature in Black, because it would be convenient for me, I acknowledge that it might not fit with Black's goals.

mortenfyhn avatar Apr 25 '22 07:04 mortenfyhn