[vim/vim] text property continuation wrong after undo/redo (PR #21366)
h_east
Problem: After undo or redo a text property may still be marked as
continuing to or from a line that is no longer next to it,
when the property was added after the change was saved
(david-k).
Solution: After undo and redo clear the continuation flags that have
no partner in the line above or below the restored lines.
Undo and redo restore the saved lines as they were, but a line just outside of them may have got or lost a property since. Undoing the redo then gives two properties where there was one, since the flag of the line outside of the undo entry cannot be restored.
Add Test_prop_undo_redo_added_across_lines(), which checks prop_list() after redo deletes the line where a property starts and after undo brings it back, and after redo deletes the line where a property ends.
fixes: #21350
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/21366
Commit Summary
- 4454421 text property continuation wrong after undo/redo
File Changes
(4 files)
- M src/proto/textprop.pro (1)
- M src/testdir/test_textprop.vim (41)
- M src/textprop.c (93)
- M src/undo.c (8)
Patch Links:
—
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.
Christian Brabandt
chrisbra left a comment (vim/vim#21366)Thanks, but I am not sure this is correct. Using the example provided:
new call prop_type_add('comment', {'highlight': 'Directory'}) call setline(1, ['one', 'two', 'three', 'four']) set ul& 2d undo call prop_add(2, 1, {'end_lnum': 3, 'end_col': 6, 'type': 'comment'}) echomsg "after undo" " state 1 echo prop_list(1, {'end_lnum': -1}) redo echomsg "after redo" " state 2 echo prop_list(1, {'end_lnum': -1}) undo echomsg "after undo again" " state 3 echo prop_list(1, {'end_lnum': -1})
The patch fixed state 2, e.g. after the :redo start: 1 and end: 1 will be set, which is correct. But the following undo then has the property split, e.g. we have 2 properties set with start: 1 and end: 1, while initially on state 1 it was 1 single propery spanning 2 lines, e.g. start: 1... end: 0 (line 2) and start: 0... end: 1 (line 3). That means at state 3 is no longer symmetric to state 1.
I think we may need to further adjust it like this (on top of yours):
diff --git a/src/undo.c b/src/undo.c index ac69e63f6..b1677b6e1 100644 --- a/src/undo.c +++ b/src/undo.c @@ -398,6 +398,51 @@ has_prop_w_flags(linenr_T lnum, int flags) } return FALSE; } + + static void +u_extend_entry_for_props(u_entry_T *uep, linenr_T *top, linenr_T *bot) +{ + bool ext_top; + bool ext_bot; + undoline_T *newarray; + long newsize; + + if (!curbuf->b_has_textprop || *bot - *top <= 1) + return; + + ext_top = *top > 0 && has_prop_w_flags(*top + 1, TP_FLAG_CONT_PREV); + ext_bot = *bot <= curbuf->b_ml.ml_line_count + && has_prop_w_flags(*bot - 1, TP_FLAG_CONT_NEXT); + if (!ext_top && !ext_bot) + return; + + newsize = uep->ue_size + ext_top + ext_bot; + newarray = U_ALLOC_LINE(sizeof(undoline_T) * newsize); + if (newarray == NULL) + return; + if (ext_top && u_save_line(&newarray[0], *top) == FAIL) + { + vim_free(newarray); + return; + } + if (ext_bot && u_save_line(&newarray[newsize - 1], *bot) == FAIL) + { + if (ext_top) + vim_free(newarray[0].ul_line); + vim_free(newarray); + return; + } + if (uep->ue_size > 0) + mch_memmove(newarray + ext_top, uep->ue_array, + sizeof(undoline_T) * uep->ue_size); + vim_free(uep->ue_array); + uep->ue_array = newarray; + uep->ue_size = newsize; + if (ext_top) + uep->ue_top = --*top; + if (ext_bot) + uep->ue_bot = ++*bot; +} #endif /* @@ -2758,6 +2803,9 @@ u_undoredo(int undo) return; } +#ifdef FEAT_PROP_POPUP + u_extend_entry_for_props(uep, &top, &bot); +#endif oldsize = bot - top - 1; // number of lines before undo newsize = uep->ue_size; // number of lines after undo @@ -2868,7 +2916,11 @@ u_undoredo(int undo) // restored lines after they were saved. adjust_props_for_undo(curbuf, top); if (newsize > 0) + { + adjust_props_for_undo(curbuf, top + 1); + adjust_props_for_undo(curbuf, top + newsize - 1); adjust_props_for_undo(curbuf, top + newsize); + } #endif // adjust marks
What do you think?
—
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.
h_east
h-east left a comment (vim/vim#21366)Thanks, you are right: undoing the redo splits the property, so
state 3 does not match state 1.
Your diff fixes that, but it has a side effect. It deletes the
unchanged line next to the block and inserts it again, so
mark_adjust() treats that line as deleted, and marks and signs on it
are lost or moved. Using your example with a mark, a file mark and a
sign on line 3 ("three"), which the redo does not change:
new call prop_type_add('comment', {'highlight': 'Directory'})
call sign_define('S', {'text': '>>'})
call setline(1, ['one', 'two', 'three', 'four']) set ul& 2d undo call prop_add(2, 1, {'end_lnum': 3, 'end_col': 6, 'type': 'comment'})
3mark a 3mark A call sign_place(1, '', 'S', '%', {'lnum': 3}) redo echo line("'a") line("'A") sign_getplaced('%')[0].signs[0].lnum undo echo line("'a") line("'A") sign_getplaced('%')[0].signs[0].lnum
"three" is line 2 after the redo and line 3 after the undo, so it
should give "2 2 2" and "3 3 3". With your diff it gives "0 2 3" and
"3 2 4". The listener callbacks and the '] mark also cover the
unchanged line.
So I took another way and pushed it: when undo or redo clears a
continuation flag on the line above or below the block, the undo
entry remembers which property it was (type, id and column). When
the entry is applied the other way, the flag is put back first, and
then the flags without a partner are cleared again. The lines
around the block are not deleted, so marks and signs stay where they
are, and state 3 matches state 1 again. The test checks that for
both the line where the property starts and the one where it ends.
What is remembered is not written to the undo file, so after reading
the undo history from a file, the property is still split.
—
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.