smarttab.kak icon indicating copy to clipboard operation
smarttab.kak copied to clipboard

Avoid overlapping selections when removing whitespaces

Open FlyingWombat opened this issue 6 years ago • 13 comments

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

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

FlyingWombat avatar Jun 04 '19 03:06 FlyingWombat

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

andreyorst avatar Jun 04 '19 05:06 andreyorst

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.

FlyingWombat avatar Jun 04 '19 16:06 FlyingWombat

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

andreyorst avatar Jun 04 '19 18:06 andreyorst

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:

  1. run kak -n -e "source '/path/to/smarttab.kak'; set buffer softtabstop 4; expandtab"
  2. make a number of adjacent selections (such as 2 cursors next to each other)
  3. press a to append to those selections, and type some text
  4. 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.

FlyingWombat avatar Jun 04 '19 18:06 FlyingWombat

I can't reproduce this

andreyorst avatar Jun 04 '19 18:06 andreyorst

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

Please try it again

FlyingWombat avatar Jun 04 '19 21:06 FlyingWombat

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. image

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

andreyorst avatar Jun 05 '19 06:06 andreyorst

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.

FlyingWombat avatar Jun 05 '19 06:06 FlyingWombat

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 avatar Jun 05 '19 07:06 andreyorst

@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 avatar Jun 05 '19 08:06 andreyorst

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

andreyorst avatar Jun 05 '19 09:06 andreyorst

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.

FlyingWombat avatar Jun 05 '19 17:06 FlyingWombat

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 avatar Jun 06 '19 05:06 andreyorst