tweekmonster / braceless.vim

:snake: Text objects, folding, and more for Python and other indented languages.
395 stars 13 forks source link

Removal of whitepsace surround operators on 'J'oin lines #35

Open NoahTheDuke opened 7 years ago

NoahTheDuke commented 7 years ago

Back again with another odd interaction! Test code:

test = [x + 1 for x in range(
    2 * 2)]

Move the cursor to the first line and press J to join the line below. On my machine, it removes all whitespace surrounding the + and * operators:

test = [x+1 for x in range(2*2)]

This also happens with - and /. If I disable the plugin, this behavior disappears. Just to be sure, I left braceless.vim enabled and tried disabling every other plugin I have install one at a time, and braceless.vim is the only one that seems to interact with this.

Thanks!

tweekmonster commented 7 years ago

This looks like a bug in removing spaces in slices. I think it's treating the [ ] list delimiters as slice delimiters. I can't get to this right now, but take a look at :h g:braceless_format if you want to disable this in the mean time. Feel free to tackle the bug if you happen to have time! 😉

NoahTheDuke commented 7 years ago

Oh man, how did I miss that? I feel like a fool. Thanks so much.

And I'm no good with vimscript, but why not? I'll see what I can find tonight. ;)

NoahTheDuke commented 7 years ago

Okay, did some digging and found the offending line in autoload/braceless/python/format.vim, line 53:

let ret = substitute(a:match, '\s*\([+\-\*/%]\)\s*', '\1', 'g')

This removes all surrounding whitespace from the the operators +, -, *, /. Originally, PEP 8 dictated leaving space around arithmetic operators, but that has changed. Post-2012 PEP 8's Other Recommendations says to use spaces when determining priority level (even though I find that unclean), but as it's left up to user discretion, I'd suggest making the default remove extra whitespaces around operators instead:

let ret = substitute(a:match, '\s\{2,}\([+\-\*/%]\)\s\{2,}', '\1', 'g')

What do you think?

Also of note, this doesn't handle the problem of not actually caring whether it's a slice or not, but I feel like the same rules apply either way. A case could be made for handling slices with arithmetic operators (as seen in Pet Peeves: ham[lower + offset:upper + offset] -> ham[lower + offset : upper + offset]), but I didn't attempt to write that regex. Maybe tomorrow.

siavasht commented 5 years ago

I'm experiencing something similar to this for empty strings in pythong. Is there any update?

example:

a = [1, "", 2, "", \
     ]

is changed to

a = [1, , 2, ,]
NoahTheDuke commented 4 years ago

I'm still thinking about this, lol. Given the rise of Black, would it make sense to remove the whitespace trimming? My original example:

test = [x + 1 for x in range(
    2 * 2)]

is reformatted by Black as:

test = [x + 1 for x in range(2 * 2)]

I realize this would be a "big" change, but seeing as it's potentially causing data loss (I can't recreate siavasht's issue, but I trust it's happening), I think fully removing this part might be smartest.