[vim/vim] Cursor wrong after autoindent strip is skipped (PR #20332)
glepnir
Problem:
cursor lands on the wrong line when a
mapping or autocmd modifies lines during insert and the strip is skipped.
Solution:
Restore cursor to tpos when skipwhite skips the strip, instead of leaving it at end_insert_pos.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/20332
Commit Summary
- ecdcbc7 Cursor wrong after autoindent strip is skipped
File Changes
(2 files)
- M src/edit.c (5)
- M src/testdir/test_edit.vim (15)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#20332)Review comment for test naming
Test function name: drop the z_ prefix
The new test is named Test_z_autoindent_no_strip_cross_line, but the z_
prefix looks unintentional here.
In test_edit.vim the only other z_-prefixed test is
Test_z_edit_insertmode_esc_beeps, where the prefix exists to force the test
to run last. This new test has no such ordering requirement, and its closest
relatives — Test_autoindent_no_strip_after_cmd_setline and
Test_autoindent_no_strip_after_cursorholdi — do not use the prefix.
For consistency with those sibling tests, I'd suggest renaming it to
Test_autoindent_no_strip_cross_line.
Minor: also assert the cursor column
The test checks line('.') but not the column. Adding a column assertion
(e.g. via getcurpos()/col('.')) would make the intended final cursor
position unambiguous and guard a bit more of the restored tpos.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
glepnir
@glepnir pushed 1 commit.
- fdffa11 Cursor wrong after autoindent strip is skipped
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#20332)@glepnir
Thanks for addressing the review points; the change is in good shape now.
I'd like to raise one thing about collaboration. Reviewing and reconciling the
specification of a patch is real work, and right now that effort often goes
unacknowledged. A simple "thanks" to the people who help get a change correct
costs nothing and keeps others motivated to keep contributing. I'm raising this
because I'd like the collaboration here to stay healthy, not to single anyone
out.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
glepnir
glepnir left a comment (vim/vim#20332)Fair point. appreciate it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#20332)@glepnir Thank you for your understanding.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#20332)Thanks all
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.