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

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

Open
NoahTheDuke opened this issue Nov 29, 2016 · 5 comments
Open

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

NoahTheDuke opened this issue Nov 29, 2016 · 5 comments
Labels

Comments

@NoahTheDuke
Copy link

NoahTheDuke commented Nov 29, 2016

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

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.

NoahTheDuke added a commit to NoahTheDuke/braceless.vim that referenced this issue Dec 1, 2016
@siavasht
Copy link

siavasht commented Aug 6, 2019

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants