black icon indicating copy to clipboard operation
black copied to clipboard

Enforce leading vs trailing spaces in multiline strings

Open jakkdl opened this issue 2 years ago • 5 comments

Describe the style change I've repeatedly messed this up as of late, and only afterwards realized I've been horribly inconsistent on where I place the spaces in multi-line strings.

Examples in the current Black style

Black will currently not reformat either of these multi-line strings

my_string_1 = (
    "words that are in the first line "
    "words that are in the second line "
    "words that are in the third line."
)
my_string_2 = (
    "words that are in the first line"
    " words that are in the second line"
    " words that are in the third line."
)

Desired style

I don't really care which one of the two is considered the correct style, as long as both are formatted the same way.

Additional context This should be a relatively simple check, without any real false positives afaict. Although I'm not totally sure if black should reformat lines according to one of the above where only some of the strings have leading/trailing spaces, or in the case of multiple on the same line.

jakkdl avatar Jan 18 '23 10:01 jakkdl

Hi! With --preview on, we do split strings according to line length, leaving spaces at the front. Is that sufficient? Example here

We respect user-made splits if they don't exceed line length but that could be a discussion too.

felix-hilden avatar Jan 18 '23 12:01 felix-hilden

Ooh that's nice!

I guess I could switch to a workflow where I write everything in a long line and let black handle line splits, but I would like it if all existing code was reformatted as well so I don't have to go back and manually check them all. And I know for sure that some projects have it as standard to have trailing spaces (it seemed more common when I looked around a while back, though I prefer leading spaces myself), so existing user-made splits and reformatted splits from black will disagree which isn't .. great.

jakkdl avatar Jan 19 '23 08:01 jakkdl

It's also not too rare that I write a multiline string, format it for consistent line length, but then reword / drop a word from an early sentence, and then end up having to do a bunch of moving around words from line to line to get back to a nice split. Though always going ham at existing strings might be a tad aggressive for some people/existing code.

jakkdl avatar Jan 19 '23 08:01 jakkdl

Yep, fair enough. We do reformat the whole string if line length is violated, so when editing you'd only need to go a bit over on one of the lines. A trade-off much like the magic comma. I'm not sure why the user-made split respecting behavior was first introduced, I guess to allow more control over string formatting. But then again, we're deving Black.

In terms of style, I do prefer leading spaces as well, and we won't be introducing any style toggles there 😅

felix-hilden avatar Jan 19 '23 09:01 felix-hilden

Right, so a temp workaround (with preview) is to just join two of the lines and then let Black go at it.

Hell yeah, Black or Bust 🤘

jakkdl avatar Jan 19 '23 09:01 jakkdl

Before closing this issue (since we do "enforce" leading spaces), are we willing to entertain a --skip-string-user-splits or similar?

felix-hilden avatar Jan 29 '23 15:01 felix-hilden

preview still won't reformat the original example, which I think is going to be quite common in existing code due to this not having been enforced previously, so I don't think it should be closed? Temp workarounds are good and all, but I don't want to have to manually go through existing code and fix it.

I don't have strong opinions on adding a flag.

jakkdl avatar Jan 30 '23 10:01 jakkdl