helix icon indicating copy to clipboard operation
helix copied to clipboard

Published 20 hours ago • verified publisher icon helix-editor

Properly handle case where last character in a cancelled command begins a new command

Open sscheele opened this issue 2 years ago • 4 comments

This is per issue #6878

sscheele avatar May 04 '23 02:05 sscheele

Thanks! I've updated to code to do the recursive call (locally) and am working on the integration tests now.

sscheele avatar May 05 '23 01:05 sscheele

I've updated the code to make the recursive call and added two test cases to test/movement.rs, but I think I might need some help with the tests, as the second case is failing for reasons I don't fully understand. I'm pretty sure my code is working and I've written the test itself wrong, but I'm having trouble understanding the formatting for the string that's supposed to represent the editor state. Could someone with more insight into this maybe explain where I've gone wrong?

sscheele avatar May 07 '23 15:05 sscheele

I think this might be a bug with the integration testing Framework. It seems that \n\n gets turned into \n somehow.

CC @dead10ck

pascalkuthe avatar May 09 '23 16:05 pascalkuthe

I actually think that in addition to the integration test problem, there's unexpected behavior associated with my code here. I've been running this code for a bit and a weird thing is that when I type j and then a non-display character, they either register in reverse order (eg: j then <up> will move the cursor up and then insert a j) or the j gets ignored entirely (this happens if you switch modes, which makes sense - so j then <esc> simply exits to normal mode without moving down a line). I don't have time to work on a fix for this atm, but I didn't want this to get merged with a bug in it.

sscheele avatar May 14 '23 03:05 sscheele

I think this might be a bug with the integration testing Framework. It seems that \n\n gets turned into \n somehow.

CC @dead10ck

This is actually intentional, see https://github.com/helix-editor/helix/pull/6156. It's a somewhat hacky way to be able to explicitly express the cursor being at the end of the line, on the EOF. I'm open to suggestions, if you can think of any ideas for a better way to do this.

dead10ck avatar May 24 '23 15:05 dead10ck

I think this might be a bug with the integration testing Framework. It seems that \n\n gets turned into \n somehow. CC @dead10ck

This is actually intentional, see #6156. It's a somewhat hacky way to be able to explicitly express the cursor being at the end of the line, on the EOF. I'm open to suggestions, if you can think of any ideas for a better way to do this.

Hmm I see, I will try to thinm of omething but nothing comes to mind right now. If just a single line ending si removed would just adding an extra newline \n|]\n\n work as a workaround?

pascalkuthe avatar May 25 '23 12:05 pascalkuthe

Hmm I see, I will try to thinm of omething but nothing comes to mind right now. If just a single line ending si removed would just adding an extra newline \n|]\n\n work as a workaround?

Yes it would. I did that in a few tests when I made the change.

dead10ck avatar May 25 '23 12:05 dead10ck

FWIW, tested this on my end, checked out @sscheele's branch and this worked 👍 I was trying to fix this one on my end but didn't notice this already had a PR hehehe

aromeronavia avatar Jul 06 '23 05:07 aromeronavia