Slave selection strangled by view cliff

[jeremybobbin]

BUG seq 100 | vis - | diff - <(seq 100 | awk '!/9$/')

The above should yield no difference when the following MARTANNE vis keystrokes are done:

:x/$9/
V
8k
8j
d
<Esc><Esc>
:wq

This is the output I see instead:

35a36,44
> 40
> 41
> 42
> 43
> 44
> 45
> 46
> 47
> 48

[tosch42]

Hi Jeremy,

I can't reproduce this. Could you describe what you try to archive here?

[jeremybobbin]

Hi Tom,

Thanks for your help; I really appreciate it.

I meant to say :x/9$/ In English for clarity:

1. Open a file(populated by seq 100)
2. select all lines ending in "9"
3. Visual-Line select mode
4. Go up 8 lines
5. Go down 8 lines
6. Delete the selection

Here is a recording: https://asciinema.org/a/j7JkULsz1fzJvZRP7I3UtxxOX

Looking into this further.

[mcepl]

Hmm, for me it is:

vis@stitny (master)$ seq 100 | vis - | diff - <(seq 100 | awk '!/9$/')
45a46,53
> 51
> 52
> 53
> 54
> 55
> 56
> 57
> 58
vis@stitny (master)$ 

And yes, it seems to be fixed by #1075.

[tosch42]

It seems to be different for every run, typical UB. I sometimes get 4x, 5x or 7x. I agree that the commit fixes this but I wonder if there might be a bigger issue which we currently oversee. I'll do some more testing in the next few days/weeks (depending on how quick Felix merges commits and I get time :-) and look at this in depth. On the other hand, it might just be as simple as Jeremy pointed out.

@jeremybobbin it would be cool if your commit message would include a brief description of the problem and how you solved it. The "fixes #1074" should go to the bottom of the message and optionally include a link to the github issue since sr.ht is (or should be) our main git provider.

[jeremybobbin]

I'm not too sure how I managed to solve it.

I was trying to parse lsblk output on a 90-drive system. I wanted the paths of only the non-raided drives, by removing the entries for the raided drives:

x/md[0-9]+/

Just for reference, one of those entries look like this:

sdcl                       69:144  0 16.4T  0 disk
├─sdcl1                    69:145  0    1G  0 part
└─sdcl2                    69:146  0 16.4T  0 part
  └─md45                    9:45   0 16.4T  0 raid1 /mnt/raid-45

Because all of our raided drives only had 2 partions like the above, I just did Vkkkd, which is when I noticed something was missing.

After teasing out the problem, I figured something was off by one. I was stepping through GDB, breaking on view_line_down & noticed that the pos > view->end & made the change.

@tosch42, I believe you're correct in that it's a deeper problem.

I will try & find where sel->line is set once I have access to a faster computer. gdb's watch is really slow for me for some reason.

Thank you for both for testing the patch :)

[jeremybobbin]

I was thinking that the visibility of the cursor shouldn't affect cursor position, but one can scroll off-screen with <C-y> & <C-e> & it makes sense that a sluggish cursor would follow. Maybe this shouldn't be the case when one has more than one selection.

That aside, I believe the root of the problem is that Selection's cursor & pos are both used to determine the position of the cursor.

To demonstrate:

• Try commenting out s->pos = pos; in cursor_to & see what movements you can do
• Try making selections below your cursor with <C-k>, then move those lower selections out of view(j), & move left or right, then move the selections back up, as if stirring peanut-butter with a long spoon. You will notice the lower selections are all displaced.

This is because k & j modify pos, while h & l modify cursor.

I will try removing pos.

[ninewise]

Good catch, this one. Thanks for the patch and mcepl and Tom for reviewing!

[mcepl]

@jeremybobbin What about https://github.com/martanne/vis/issues/1084?

[jeremybobbin]

Yep - I'm able to reproduce that; I'd roll it back.

[rnpnr]

Hi, given 1bfe132 I'm reopening this. It is still a valid issue but the fix was incorrect.

[jeremybobbin]

c22b2c2 is correct. I believe the issue causing #1143 is here: https://github.com/martanne/vis/blob/a7aac1044856abc4d1f133c6563fc604d7fe6295/vis.c#L272-L282

It's saying "if neither the start nor end lines are within the view, don't highlight the selection".

It would make sense that if your anchor is at the top of the view while your cursor dips out, your selection would no longer be highlighted.

If you comment out the first if-statement, then, because of the two following if-statements, selections that simply exist outside of the view will cause your whole thing to be highlighted.