AndrewRadev / splitjoin.vim

Switch between single-line and multiline forms of code
http://www.vim.org/scripts/script.php?script_id=3613
MIT License
1.91k stars 89 forks source link

vim: gS on dict access dot incorrectly adds space #207

Open flwyd opened 10 months ago

flwyd commented 10 months ago

Given the vimscript code l:my_dict.my_key, using splitjoin's gS at the . character produces

l:my_dict
      \ .my_key

When vim's scriptversion is 1 this creates ambiguity between dict key lookup and string concatenation due to the space between \ and .my_key. (The dot is an attractive split point when writing vim script with an object-oriented style: l:object.SomeMethod(arg, list).AnotherMethod(more, args).)

Vim seems to gracefully handle the \ .my_key ambiguity by checking if the type of l:my_dict is Dict, but NeoVim produces an E731: using Dictionary as a String error. :help expr-entry says "There must not be white space before or after the dot." It would be nice if splitting on a dot without space around it would result in a semantically identical split like

l:my_dict
      \.my_key

(note the lack of space in \.).

AndrewRadev commented 10 months ago

That's a good point. I've pushed a new setting, called vim_split_whitespace_after_backslash, it's set to 1 by default, but you could set it to 0 with:

let g:splitjoin_vim_split_whitespace_after_backslash = 0

This will consistently not add a space when breaking lines. I think that's the right way to go -- I could check for a ., but removing the whitespace only on that one chunk would be inconsistent. And I've seen the no-space style in places anyway, I think the vim documentation uses it often, for example. What do you think?

Additionally, how do you feel about this not being the default? I think it would be a safer default, but it would change behaviour for (the admittedly few) people already using it, and I'd rather avoid this kind of surprise.

flwyd commented 10 months ago

I'm not sure how I feel about the default here, but I collected some data.

According to ripgrep, 108 .vim files among the 98 plugins I have installed, 108 files have at least one line matching ^\s+\\\S (i.e. they use line continuation without a following space) while 966 files have at least one line matching ^\s+\\\s (i.e. they use line continuation with a following space). In /usr/share/vim/vim90 108 files match the former pattern while 328 files match the latter pattern. (The fact that both locations have 108 files is coincidence.) It looks like a majority of the no-space matches in the /usr/share/vim collection are actually metacharacters in regex patterns for compiler output.

It's also worth noting that not adding a space may result in surprising semantics as well. IIUC, the vim code

ca
  \ll Foo('he
  \lo world')

is equivalent to :call Foo('hello world') while

call
  \Foo('hello
  \world')

becomes :callFoo('helloworld'), which may be surprising, since visually there's whitespace in places where it doesn't end up coming through.

AndrewRadev commented 10 months ago

It's also worth noting that not adding a space may result in surprising semantics as well.

That's a fair point. I'm starting to think a preference is not the way to go, but simply whether there is or isn't whitespace at or before the cursor when breaking the line. In your examples, if you broke the call into ca and ll, you don't want to leave whitespace between the words, so it would be \ll, both semantically, and for splitjoin. But if you split between call and Foo, you do want that whitespace.

If you consistently use it to break method calls, they'll all consistently have no whitespace. If you break dictionaries (my main use case), and you have , between the items, they'll have whitespace. Theoretically, you might want to use , on a single line and still remove whitespace when the line is broken, but this feels like too minute (and specific) of a detail to try to handle.

I've pushed a change that implements that and should handle your examples sensibly. Could you try it out and let me know how it feels? If we don't find any issues with the scheme, I'll remove the setting I introduced and just leave this smarter handling.