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

number of selections lost when cursors overlap #2298

Open
eburghar opened this issue Apr 27, 2022 · 11 comments
Open

number of selections lost when cursors overlap #2298

eburghar opened this issue Apr 27, 2022 · 11 comments
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug

Comments

@eburghar
Copy link

The number of selection is lost when selections are overlapping

example with the following text

|    |    |

when I select the spaces (8 sel) on the line with xs and want to replace the spaces with - with `c-' I endup with only

|-|-|

The c turns the 8 sel into only 2.

kakoune correctly manage overlapping selections, keep 8 sel after the c and replace all the spaces with '-'

|----|----|

Of course you have the r primitive for doing that in helix, but I think changing the number of selections is kind of counter intuitive.

@the-mikedavis the-mikedavis added the C-bug Category: This is a bug label Apr 27, 2022
@kirawi kirawi added the A-helix-term Area: Helix term improvements label Apr 28, 2022
@darrigu
Copy link

darrigu commented May 1, 2022

You can count the lines and enter <number_of_lines>x.

@the-mikedavis
Copy link
Member

That's unrelated to the bug. If you follow the example you can see the selection counter at the bottom right-hand side change from 8 to 2 after hitting c because the selections overlap with the change.

@kabouzeid
Copy link
Contributor

kabouzeid commented Nov 13, 2022

Would also be useful for changing indention from tabs to spaces with eg %s\t<CR>c<SPACE><SPACE>. Can't use r in this case because replacing with multiple characters.

@jannschu
Copy link

jannschu commented Jun 5, 2023

I looked a bit into this because I also tried to replace tab characters with spaces and then ran into this issue.

The change command is currently implemented by first deleting the ranges of the selection, and then switching to insert mode (so similar to pressing di). As part of the delete transaction the selected text is removed and the ranges covering the selected text are reduced to zero width (I hope, I only read the code, see below). In the case of adjacent ranges there will then be several zero width ranges at the same position. Now during selection normalization those are merged because zero width ranges at the same position are currently considered to overlap (which might be wrong). So my first idea for fixing this would be:

  1. Ensure that the selection's ranges become zero width (easy to check, but hard to see from just reading the code due to off by one issues), see Range:map/ChangeSet::map_pos.
  2. Stop merging zero width ranges in Selection::normalize, i.e. consider a zero width range adjacent to another range to not be overlapping.

If we are really lucky this is a one line change in Selection::normalize. But since this changes what kind of selections are valid the consequences are hard to oversee: Selections are a fundamental data structure in helix and they are used in nontrivial ways.

edit: Changed Range::overlaps to

!(self.to() <= other.from() || other.to() <= self.from())

and Selection::ensure_invariants to

self.transform(|r| r.grapheme_aligned(text)).normalize()

That clearly now violates the invariants described in the doc comment of Selection::ensure_invariants because ranges will be zero width now not only at the end of the document.

Input is then added multiple times though, as expected, but unfortunately the ranges do not separate after inputs, i.e. input change sets need to be fixed for zero width ranges, well...

So there are potentially a lot changes necessary with this particular approach. Can someone oversee what needs to be changed? What was the background for not allowing zero width ranges except for the document's end?

@kirawi
Copy link
Member

kirawi commented Nov 3, 2023

What was the background for not allowing zero width ranges except for the document's end?

That decision was made in #362

@pascalkuthe
Copy link
Member

A lot of our editing model relies on overlapping ranges being merged. I don't think we should ever change that. I think the only case Quere this really is useful is when entering insertmode (so di or c) so there may be a way to special case that particular case (maybe just for c?)

I do want to stop merging djecent (non-overlapping) selections (that will not work with c/d ofcourse but it will work with r and R and other commands that producer adjecent selections)

@ctem
Copy link

ctem commented Nov 8, 2023

In addition to formatting Markdown tables and changing tabs to spaces, another common use case is redacting (i.e., "masking" or "zeroing out") a multi-character value—e.g., a hash, secret, color hex value, etc.—while retaining its character count.

I think the only case [where] this really is useful is when entering [insert mode] (so di or c) so there may be a way to special case that particular case (maybe just for c?)

Indeed, all these use cases exploit insert mode. However, it seems worth noting that in Kakoune, selections are respected in normal mode as well. For example, starting with text abc on a line:

  1. xs.Enter makes four selections: a, b, c, and the line ending. (Note that Helix ignores the line ending, resulting in only three selections, but that's unrelated to the current issue.)
  2. d cuts all four selections. At this point, it looks like a single cursor at the start of an empty line, but the four cursors remain; they're simply "stacked atop each other", sharing the only available column.
  3. i123 enters insert mode and adds text 123123123123 as expected.

@kirawi
Copy link
Member

kirawi commented Mar 6, 2024

@kirawi
Copy link
Member

kirawi commented Apr 14, 2024

@pascalkuthe I think we can rename this issue to explicitly the change_selection command since Kakoune seems to have special behavior for it.

@pascalkuthe
Copy link
Member

I haven't looked too much into what kakoune does yet ao I cant comment on that. If you looked into that and that's how ti works feel free to rename

@kanashimia
Copy link

Example, assuming you have a buffer with a word foo in it:
By executing %s.<enter>a. your buffer will contain: foo.
By executing %s.<enter>i. your buffer will contain: .f.o.o

So this doesn't only affect c.
The fact that a behaves differently from i is stupid.

Originally posted in: #10861

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug
Projects
None yet
Development

No branches or pull requests

9 participants