From b50fdcba4a1dadda5db7cfb527471b9956768ac2 Mon Sep 17 00:00:00 2001 From: Luuk van Baal Date: Sat, 27 Jan 2024 13:04:58 +0100 Subject: [PATCH] fix(column): clear "b_signcols" before moving saved marks Problem: Marks moved by undo may be lost to "b_signcols.count". Solution: Count signs for each undo object separately instead of once for the entire undo. --- src/nvim/decoration.c | 11 ++----- src/nvim/drawscreen.c | 2 +- src/nvim/extmark.c | 41 ++++++++++++++----------- src/nvim/undo.c | 33 ++------------------ test/functional/ui/decorations_spec.lua | 27 ++++++++++++++++ 5 files changed, 57 insertions(+), 57 deletions(-) diff --git a/src/nvim/decoration.c b/src/nvim/decoration.c index 8d9b234bbc..60213109fb 100644 --- a/src/nvim/decoration.c +++ b/src/nvim/decoration.c @@ -684,15 +684,16 @@ static const uint32_t sign_filter[4] = {[kMTMetaSignText] = kMTFilterSelect, void decor_redraw_signs(win_T *wp, buf_T *buf, int row, SignTextAttrs sattrs[], int *line_id, int *cul_id, int *num_id) { - MarkTreeIter itr[1]; - if (!marktree_itr_get_overlap(buf->b_marktree, row, 0, itr)) { + if (!buf_has_signs(buf)) { return; } MTPair pair; int num_text = 0; + MarkTreeIter itr[1]; kvec_t(SignItem) signs = KV_INITIAL_VALUE; // TODO(bfredl): integrate with main decor loop. + marktree_itr_get_overlap(buf->b_marktree, row, 0, itr); while (marktree_itr_step_overlap(buf->b_marktree, itr, &pair)) { if (!mt_invalid(pair.start) && mt_decor_sign(pair.start)) { DecorSignHighlight *sh = decor_find_sign(mt_decor(pair.start)); @@ -773,12 +774,6 @@ void buf_signcols_count_range(buf_T *buf, int row1, int row2, int add, TriState return; } - static int nested = 0; - // An undo/redo may trigger subsequent calls before its own kNone call. - if ((nested += clear) > (0 + (clear == kTrue))) { - return; // Avoid adding signs more than once. - } - // Allocate an array of integers holding the number of signs in the range. assert(row2 >= row1); int *count = xcalloc(sizeof(int), (size_t)(row2 + 1 - row1)); diff --git a/src/nvim/drawscreen.c b/src/nvim/drawscreen.c index 8d67def774..b0c97f5be3 100644 --- a/src/nvim/drawscreen.c +++ b/src/nvim/drawscreen.c @@ -1199,7 +1199,7 @@ void comp_col(void) set_vim_var_nr(VV_ECHOSPACE, sc_col - 1); } -/// Redraw entire window "wp" if configured 'signcolumn' width changes. +/// Redraw entire window "wp" if "auto" 'signcolumn' width has changed. static bool win_redraw_signcols(win_T *wp) { buf_T *buf = wp->w_buffer; diff --git a/src/nvim/extmark.c b/src/nvim/extmark.c index f7c16964b9..93a2b9c16a 100644 --- a/src/nvim/extmark.c +++ b/src/nvim/extmark.c @@ -107,20 +107,33 @@ revised: } } -static bool extmark_setraw(buf_T *buf, uint64_t mark, int row, colnr_T col) +static void extmark_setraw(buf_T *buf, uint64_t mark, int row, colnr_T col, bool invalid) { MarkTreeIter itr[1] = { 0 }; MTKey key = marktree_lookup(buf->b_marktree, mark, itr); - if (key.pos.row == -1) { - return false; + if (key.pos.row < 0 || (key.pos.row == row && key.pos.col == col)) { + return; } - if (key.pos.row == row && key.pos.col == col) { - return true; + int row1 = 0; + int row2 = 0; + if (invalid) { + mt_itr_rawkey(itr).flags &= (uint16_t) ~MT_FLAG_INVALID; + } else if (key.flags & MT_FLAG_DECOR_SIGNTEXT && buf->b_signcols.autom) { + MTPos end = marktree_get_altpos(buf->b_marktree, key, NULL); + row1 = MIN(end.row, MIN(key.pos.row, row)); + row2 = MAX(end.row, MAX(key.pos.row, row)); + buf_signcols_count_range(buf, row1, row2, 0, kTrue); } marktree_move(buf->b_marktree, itr, row, col); - return true; + + if (invalid) { + MTPos end = marktree_get_altpos(buf->b_marktree, key, NULL); + buf_put_decor(buf, mt_decor(key), row, end.row); + } else if (key.flags & MT_FLAG_DECOR_SIGNTEXT && buf->b_signcols.autom) { + buf_signcols_count_range(buf, row1, row2, 0, kNone); + } } /// Remove an extmark in "ns_id" by "id" @@ -401,16 +414,8 @@ void extmark_apply_undo(ExtmarkUndoObject undo_info, bool undo) // kExtmarkSavePos } else if (undo_info.type == kExtmarkSavePos) { ExtmarkSavePos pos = undo_info.data.savepos; - if (undo) { - if (pos.old_row >= 0 - && extmark_setraw(curbuf, pos.mark, pos.old_row, pos.old_col) - && pos.invalidated) { - MarkTreeIter itr[1] = { 0 }; - MTKey mark = marktree_lookup(curbuf->b_marktree, pos.mark, itr); - mt_itr_rawkey(itr).flags &= (uint16_t) ~MT_FLAG_INVALID; - MTPos end = marktree_get_altpos(curbuf->b_marktree, mark, itr); - buf_put_decor(curbuf, mt_decor(mark), mark.pos.row, end.row); - } + if (undo && pos.old_row >= 0) { + extmark_setraw(curbuf, pos.mark, pos.old_row, pos.old_col, pos.invalidated); } // No Redo since kExtmarkSplice will move marks back } else if (undo_info.type == kExtmarkMove) { @@ -527,7 +532,7 @@ void extmark_splice_impl(buf_T *buf, int start_row, colnr_T start_col, bcount_t // Remove signs inside edited region from "b_signcols.count", add after splicing. if (old_row > 0 || new_row > 0) { - buf_signcols_count_range(buf, start_row, start_row + old_row + 1, 0, kTrue); + buf_signcols_count_range(buf, start_row, start_row + old_row, 0, kTrue); } marktree_splice(buf->b_marktree, (int32_t)start_row, start_col, @@ -535,7 +540,7 @@ void extmark_splice_impl(buf_T *buf, int start_row, colnr_T start_col, bcount_t new_row, new_col); if (old_row > 0 || new_row > 0) { - buf_signcols_count_range(buf, start_row, start_row + new_row + 1, 0, kNone); + buf_signcols_count_range(buf, start_row, start_row + new_row, 0, kNone); } if (undo == kExtmarkUndo) { diff --git a/src/nvim/undo.c b/src/nvim/undo.c index 3eb0f1f2ec..547ac605e7 100644 --- a/src/nvim/undo.c +++ b/src/nvim/undo.c @@ -2427,39 +2427,15 @@ static void u_undoredo(bool undo, bool do_buf_event) curbuf->b_op_end.lnum = curbuf->b_ml.ml_line_count; } - int row1 = MAXLNUM; - int row2 = -1; - int row3 = -1; - // Tricky: ExtmarkSavePos may come after ExtmarkSplice which does call - // buf_signcols_count_range() but then misses the yet unrestored marks. - if (curbuf->b_signcols.autom && buf_meta_total(curbuf, kMTMetaSignText)) { - for (int i = 0; i < (int)kv_size(curhead->uh_extmark); i++) { - ExtmarkUndoObject undo_info = kv_A(curhead->uh_extmark, i); - if (undo_info.type == kExtmarkSplice) { - ExtmarkSplice s = undo_info.data.splice; - if (s.old_row > 0 || s.new_row > 0) { - row1 = MIN(row1, s.start_row); - row2 = MAX(row2, s.start_row + (undo ? s.new_row : s.old_row) + 1); - row3 = MAX(row3, s.start_row + (undo ? s.old_row : s.new_row) + 1); - } - } - } - if (row2 != -1) { - // Remove signs inside edited region from "b_signcols.count". - buf_signcols_count_range(curbuf, row1, row2, 0, kTrue); - } - } // Adjust Extmarks if (undo) { for (int i = (int)kv_size(curhead->uh_extmark) - 1; i > -1; i--) { - ExtmarkUndoObject undo_info = kv_A(curhead->uh_extmark, i); - extmark_apply_undo(undo_info, undo); + extmark_apply_undo(kv_A(curhead->uh_extmark, i), undo); } // redo } else { for (int i = 0; i < (int)kv_size(curhead->uh_extmark); i++) { - ExtmarkUndoObject undo_info = kv_A(curhead->uh_extmark, i); - extmark_apply_undo(undo_info, undo); + extmark_apply_undo(kv_A(curhead->uh_extmark, i), undo); } } if (curhead->uh_flags & UH_RELOAD) { @@ -2467,10 +2443,7 @@ static void u_undoredo(bool undo, bool do_buf_event) // should have all info to send a buffer-reloaing on_lines/on_bytes event buf_updates_unload(curbuf, true); } - // Finish adjusting extmarks: add signs inside edited region to "b_signcols.count". - if (row2 != -1) { - buf_signcols_count_range(curbuf, row1, row3, 0, kNone); - } + // Finish adjusting extmarks curhead->uh_entry = newlist; curhead->uh_flags = new_flags; diff --git a/test/functional/ui/decorations_spec.lua b/test/functional/ui/decorations_spec.lua index 7b3533454c..524a1351f3 100644 --- a/test/functional/ui/decorations_spec.lua +++ b/test/functional/ui/decorations_spec.lua @@ -5058,6 +5058,33 @@ l5 ]]} end) + it('correct width with moved marks before undo savepos', function() + screen:try_resize(20, 4) + insert(example_test3) + feed('gg') + exec_lua([[ + local ns = vim.api.nvim_create_namespace('') + vim.api.nvim_buf_set_extmark(0, ns, 0, 0, { sign_text = 'S1' }) + vim.api.nvim_buf_set_extmark(0, ns, 1, 0, { sign_text = 'S2' }) + local s3 = vim.api.nvim_buf_set_extmark(0, ns, 2, 0, { sign_text = 'S3' }) + local s4 = vim.api.nvim_buf_set_extmark(0, ns, 2, 0, { sign_text = 'S4' }) + vim.schedule(function() + vim.cmd('silent d3') + vim.api.nvim_buf_set_extmark(0, ns, 2, 0, { id = s3, sign_text = 'S3' }) + vim.api.nvim_buf_set_extmark(0, ns, 2, 0, { id = s4, sign_text = 'S4' }) + vim.cmd('silent undo') + vim.api.nvim_buf_del_extmark(0, ns, s3) + end) + ]]) + + screen:expect{grid=[[ + S1^l1 | + S2l2 | + S4l3 | + | + ]]} + end) + it('no crash with sign after many marks #27137', function() screen:try_resize(20, 4) insert('a')