diff --git a/src/nvim/extmark.c b/src/nvim/extmark.c index 16ddc04f3a..52c56f557b 100644 --- a/src/nvim/extmark.c +++ b/src/nvim/extmark.c @@ -47,6 +47,30 @@ #include "extmark.c.generated.h" +/// Records a raw mark's position in the open undo block, for `extmark_apply_undo()` to replay. +/// +/// Pass -1 for a row to skip that direction: the mark is left wherever replaying the surrounding +/// splices puts it. +static void extmark_push_savepos(buf_T *buf, uint64_t mark, int undo_row, colnr_T undo_col, + int redo_row, colnr_T redo_col) +{ + u_header_T *uhp = u_force_get_undo_header(buf); + if (uhp == NULL) { + return; + } + kv_push(uhp->uh_extmark, ((ExtmarkUndoObject){ + .type = kExtmarkSavePos, + .data.savepos = { + .mark = mark, + .undo_row = undo_row, + .undo_col = undo_col, + .redo_row = redo_row, + .redo_col = redo_col, + .invalidated = false, + } + })); +} + /// Create or update an extmark /// /// must not be used during iteration! @@ -56,6 +80,7 @@ void extmark_set(buf_T *buf, uint32_t ns_id, uint32_t *idp, int row, colnr_T col { uint32_t *ns = map_put_ref(uint32_t, uint32_t)(buf->b_extmark_ns, ns_id, NULL, NULL); uint32_t id = idp ? *idp : 0; + bool created = true; // Mark did not exist before this call. uint16_t flags = mt_flags(right_gravity, no_undo, invalidate, decor.ext) | decor_flags; if (id == 0) { @@ -64,24 +89,13 @@ void extmark_set(buf_T *buf, uint32_t ns_id, uint32_t *idp, int row, colnr_T col MarkTreeIter itr[1] = { 0 }; MTKey old_mark = marktree_lookup_ns(buf->b_marktree, ns_id, id, false, itr); if (old_mark.id) { + created = false; // Moved inside an open undo block: record BOTH positions, because replaying the surrounding // splices reproduces only the shifts they caused, not an explicit extmark_set(). if (!no_undo && !buf->b_u_synced && (old_mark.pos.row != row || old_mark.pos.col != col)) { - u_header_T *uhp = u_force_get_undo_header(buf); - if (uhp) { - kv_push(uhp->uh_extmark, ((ExtmarkUndoObject){ - .type = kExtmarkSavePos, - .data.savepos = { - .mark = mt_lookup_key(old_mark), - .undo_row = old_mark.pos.row, - .undo_col = old_mark.pos.col, - .redo_row = row, - .redo_col = col, - .invalidated = false, - } - })); - } + extmark_push_savepos(buf, mt_lookup_key(old_mark), old_mark.pos.row, old_mark.pos.col, row, + col); } if (mt_paired(old_mark) || end_row > -1) { extmark_del_id(buf, ns_id, id); @@ -114,6 +128,16 @@ void extmark_set(buf_T *buf, uint32_t ns_id, uint32_t *idp, int row, colnr_T col marktree_put(buf->b_marktree, mark, end_row, end_col, end_right_gravity); decor_state_invalidate(buf); + // Created inside an open undo block: record the position for redo. Undo leaves the mark where the + // splice replay puts it (it did not exist before the edit), but redo must re-apply the explicit + // position, else a mark over text that undo deleted never comes back. + if (created && !no_undo && !buf->b_u_synced) { + extmark_push_savepos(buf, mt_lookup_id(ns_id, id, false), -1, 0, row, col); + if (end_row > -1) { + extmark_push_savepos(buf, mt_lookup_id(ns_id, id, true), -1, 0, end_row, end_col); + } + } + revised: if (decor_flags || decor.ext) { buf_put_decor(buf, decor, row, end_row > -1 ? end_row : row); @@ -125,11 +149,16 @@ revised: } } -static void extmark_setraw(buf_T *buf, uint64_t mark, int row, colnr_T col, bool invalid) +/// Moves a raw mark back to a recorded position. +/// +/// @param revive Clear MT_FLAG_INVALID, if set: the associated text exists again. +static void extmark_setraw(buf_T *buf, uint64_t mark, int row, colnr_T col, bool revive) { MarkTreeIter itr[1] = { 0 }; MTKey key = marktree_lookup(buf->b_marktree, mark, itr); bool move = key.pos.row != row || key.pos.col != col; + // Only an invalidated mark is revived; reviving a valid one would double-count its decor. + bool invalid = revive && mt_invalid(key); if (key.pos.row < 0 || (!move && !invalid)) { return; // Mark was deleted or no change needed } @@ -478,8 +507,8 @@ void extmark_apply_undo(ExtmarkUndoObject undo_info, bool undo) if (undo && pos.undo_row >= 0) { extmark_setraw(curbuf, pos.mark, pos.undo_row, pos.undo_col, pos.invalidated); } else if (!undo && pos.redo_row >= 0) { - // Redo an explicit extmark_set (kExtmarkSplice only redo's the shifts). - extmark_setraw(curbuf, pos.mark, pos.redo_row, pos.redo_col, false); + // Redo an explicit extmark_set. The mark was valid when set, revive if undo invalidated it. + extmark_setraw(curbuf, pos.mark, pos.redo_row, pos.redo_col, true); } } else if (undo_info.type == kExtmarkMove) { ExtmarkMove move = undo_info.data.move; diff --git a/test/functional/api/extmark_spec.lua b/test/functional/api/extmark_spec.lua index 8371056c01..d7560305c3 100644 --- a/test/functional/api/extmark_spec.lua +++ b/test/functional/api/extmark_spec.lua @@ -983,6 +983,28 @@ describe('API/extmarks', function() eq(2, #rv) end) + it('undo and redo of a mark created during an edit #30331', function() + local function range(id) + local m = get_extmark_by_id(ns, id, { details = true }) + return { m[1], m[2], m[3].end_row, m[3].end_col, m[3].invalid } + end + -- Create the marks over text the edit added, while the undo block is still open. + feed('ggdGifoobar') + set_extmark(ns, marks[1], 0, 3, { end_col = 6 }) + set_extmark(ns, marks[2], 0, 3, { end_col = 6, invalidate = true }) + feed('') + eq({ 0, 3, 0, 6 }, range(marks[1])) + feed('u') + -- The text is gone, so the range collapses; the invalidating mark also goes invalid. + eq({ 0, 0, 0, 0 }, range(marks[1])) + eq({ 0, 0, 0, 0, true }, range(marks[2])) + feed('') + -- Redo re-applies the position each side of the range was set to, and revives the + -- invalidated mark; replaying the splices alone leaves the range collapsed. + eq({ 0, 3, 0, 6 }, range(marks[1])) + eq({ 0, 3, 0, 6 }, range(marks[2])) + end) + it('undo and redo of a mark explicitly moved during an edit', function() feed('ggdGiabcdef') set_extmark(ns, marks[1], 0, 4)