Avoid overlapping selections when removing whitespaces

FlyingWombat commented 5 years ago

Regression of https://github.com/mawww/kakoune/issues/2861

as of
Kakoune git: 1ebea85e6f07aeb6a8287b8043480f56f0e58edb
smarttab.kak git: 933a784e5cab8fe9224b631554576f27e80ca416

andreyorst commented 5 years ago

can you provide any example where smarttab is involved? To me that's an upstream problem

FlyingWombat commented 5 years ago

Sorry, I'm not yet familiar with relevant source of Kakoune and smarttab.kak. So, I can't say whether this is a problem with upstream or the plugin.
But, the regression is present when running kak -n -e "source '/path/to/smarttab.kak'; expandtab", and not present when running kak -n. So the plugin is definitely causing the regression somehow.

andreyorst commented 5 years ago

So what's the actual problem? Any recipe so I could reproduce it? I still don't get what's the issue here.

FlyingWombat commented 5 years ago

Oh, sorry if OP was confusing.

The problem is the same as in the original issue in https://github.com/mawww/kakoune/issues/2861. I'll copy the steps here. Since the problem seemed to be fixed in base Kakoune, and using this plugin somehow re-introduces it, I thought I'd post a new issue here.

The problem is that, when selections are adjacent, touching each other, backspace doesn't work properly when appending with a. Selections are merged instead of backspace working as one would expect in insert mode: where the last entered char in each selection is removed. This is an edge-case of selection merging; it happens with append a, but not with insert i.

Steps to reproduce:

run kak -n -e "source '/path/to/smarttab.kak'; set buffer softtabstop 4; expandtab"
make a number of adjacent selections (such as 2 cursors next to each other)
press a to append to those selections, and type some text
press backspace, and you'll see that instead of instead of deleting the last character typed in each selection, the selections are merged.

Please let me know if you can't reproduce this.

andreyorst commented 5 years ago

I can't reproduce this

FlyingWombat commented 5 years ago

Updated steps.
For me it happens with set buffer softtabstop 4; expandtab, but not just expandtab

Please try it again

andreyorst commented 5 years ago

Still nothing. I'm pressing these keys:

kak -n - start kakoune with no config;
%d - kill buffer contents (scratch message);
i<space><space><esc> - add 2 spaces to file;
ggLs.<ret> - go to beginning of the buffer, select first two spaces, split by any match to obtain two cursors near each other;
atext - append text to each cursor;
<backspace><backspace><backspace><backspace> - remove all text;
- at this point I still have two separate cursors;
anothertext - add anothertext to each cursor.

If you can reproduce it with execute-keys I would like to see it. Ideally something like this:

kak -n -e "source 'smarttab.kak'; set buffer softtabstop 4; expandtab; execute-keys -with-hooks '%di<space><space><esc>ggLs.<ret>atext<backspace><backspace><backspace><backspace>anothertext'"

The command above works for me with no issue whatsoever.

smarttab.kak is at 933a784e5cab8fe9224b631554576f27e80ca416 Kakoune is at 1ebea85e6f07aeb6a8287b8043480f56f0e58edb

FlyingWombat commented 5 years ago

Wow, this is really fickle. It's <space> that makes the difference. I didn't really think of how I kept adding a space out of habit, in my testing.

Two separate selections (as expected):

kak -n -e "source '~/.local/share/kak/plugins/smarttab.kak/rc/smarttab.kak'; set buffer softtabstop 4; expandtab; execute-keys -with-hooks '%di<space><space><esc>ggLs.<ret>atext<backspace>'"

One merged selection (bug):

kak -n -e "source '~/.local/share/kak/plugins/smarttab.kak/rc/smarttab.kak'; set buffer softtabstop 4; expandtab; execute-keys -with-hooks '%di<space><space><esc>ggLs.<ret>atext<space><backspace>'"

Please try these to see if you get the same result.

andreyorst commented 5 years ago

One merged selection (bug):

kak -n -e "source '~/.local/share/kak/plugins/smarttab.kak/rc/smarttab.kak'; set buffer softtabstop 4; expandtab; execute-keys -with-hooks '%di<space><space><esc>ggLs.<ret>atext<space><backspace>'"

Hmm, sure this triggers the issue on my master build of Kakoune. Interestingly enough it happens only when softtabstop option is set. I'll see what I can do.

andreyorst commented 5 years ago

@FlyingWombat I found another way to trigger this bug without any plugin:

kak -n -e "execute-keys '%di<space><space><esc>ggLs.<ret>atext<esc>L'"

The key problem here is L, it merges selections. I use it in InsertDelete ' ' hook. It has nothing to do with the plugin. I'll report this to original issue.

andreyorst commented 5 years ago

I believe I can close this since it reproduces without plugin.

FlyingWombat commented 5 years ago

The problem seems to be with line 56, in the InsertDelete ' ' hook, which only executes when softtabstop == 0.

As you said, L does merge the adjacent selections in normal mode. But AFAIK that's the expected behavior, since we want to avoid overlapping selections in normal mode. I think the problem is that the InsertDelete hook is executing keys in a "temporary normal mode", where L will merge the selections.

And, yes, this is a problem that will likely need to be fixed upstream, so this issue can be closed.

andreyorst commented 5 years ago

The problem seems to be with line 56, in the InsertDelete ' ' hook, which only executes when softtabstop == 0.

Yes. I'll try to think about a way of doing it without extending selections.

andreyorst / smarttab.kak

Avoid overlapping selections when removing whitespaces #8