Merge pull request #27061 from luukvbaal/extmark

fix(extmarks): do not remove invalid marks from decor upon deletion

6 files changed, 45 insertions, 29 deletions

diff --git a/runtime/doc/api.txt b/runtime/doc/api.txt
index b5a49f6002..f37d349fd8 100644
--- a/runtime/doc/api.txt
+++ b/runtime/doc/api.txt
@@ -2594,8 +2594,8 @@ nvim_buf_get_extmark_by_id({buffer}, {ns_id}, {id}, {*opts})
 
                                                      *nvim_buf_get_extmarks()*
 nvim_buf_get_extmarks({buffer}, {ns_id}, {start}, {end}, {*opts})
-    Gets |extmarks| (including |signs|) in "traversal order" from a |charwise|
-    region defined by buffer positions (inclusive, 0-indexed |api-indexing|).
+    Gets |extmarks| in "traversal order" from a |charwise| region defined by
+    buffer positions (inclusive, 0-indexed |api-indexing|).
 
     Region can be given as (row,col) tuples, or valid extmark ids (whose
     positions define the bounds). 0 and -1 are understood as (0,0) and (-1,-1)
@@ -2611,6 +2611,10 @@ nvim_buf_get_extmarks({buffer}, {ns_id}, {start}, {end}, {*opts})
     the `overlap` option might be useful. Otherwise only the start position of
     an extmark will be considered.
 
+    Note: legacy signs placed through the |:sign| commands are implemented as
+    extmarks and will show up here. Their details array will contain a
+    `sign_name` field.
+
     Example: >lua
         local api = vim.api
         local pos = api.nvim_win_get_cursor(0)
@@ -2742,7 +2746,9 @@ nvim_buf_set_extmark({buffer}, {ns_id}, {line}, {col}, {*opts})
                     text around the mark was deleted and then restored by
                     undo. Defaults to true.
                   • invalidate : boolean that indicates whether to hide the
-                    extmark if the entirety of its range is deleted. If
+                    extmark if the entirety of its range is deleted. For
+                    hidden marks, an "invalid" key is added to the "details"
+                    array of |nvim_buf_get_extmarks()| and family. If
                     "undo_restore" is false, the extmark is deleted instead.
                   • priority: a priority value for the highlight group or sign
                     attribute. For example treesitter highlighting uses a
diff --git a/runtime/lua/vim/_meta/api.lua b/runtime/lua/vim/_meta/api.lua
index 3f06d4fd43..f773ddd75c 100644
--- a/runtime/lua/vim/_meta/api.lua
+++ b/runtime/lua/vim/_meta/api.lua
@@ -323,8 +323,8 @@ function vim.api.nvim_buf_get_commands(buffer, opts) end
 --- @return integer[]
 function vim.api.nvim_buf_get_extmark_by_id(buffer, ns_id, id, opts) end
 
---- Gets `extmarks` (including `signs`) in "traversal order" from a `charwise`
---- region defined by buffer positions (inclusive, 0-indexed `api-indexing`).
+--- Gets `extmarks` in "traversal order" from a `charwise` region defined by
+--- buffer positions (inclusive, 0-indexed `api-indexing`).
 --- Region can be given as (row,col) tuples, or valid extmark ids (whose
 --- positions define the bounds). 0 and -1 are understood as (0,0) and (-1,-1)
 --- respectively, thus the following are equivalent:
@@ -339,6 +339,9 @@ function vim.api.nvim_buf_get_extmark_by_id(buffer, ns_id, id, opts) end
 --- Note: when using extmark ranges (marks with a end_row/end_col position)
 --- the `overlap` option might be useful. Otherwise only the start position of
 --- an extmark will be considered.
+--- Note: legacy signs placed through the `:sign` commands are implemented as
+--- extmarks and will show up here. Their details array will contain a
+--- `sign_name` field.
 --- Example:
 ---
 --- ```lua
@@ -567,7 +570,9 @@ function vim.api.nvim_buf_line_count(buffer) end
 ---                 text around the mark was deleted and then restored by
 ---                 undo. Defaults to true.
 ---               • invalidate : boolean that indicates whether to hide the
----                 extmark if the entirety of its range is deleted. If
+---                 extmark if the entirety of its range is deleted. For
+---                 hidden marks, an "invalid" key is added to the "details"
+---                 array of `nvim_buf_get_extmarks()` and family. If
 ---                 "undo_restore" is false, the extmark is deleted instead.
 ---               • priority: a priority value for the highlight group or sign
 ---                 attribute. For example treesitter highlighting uses a
diff --git a/src/nvim/api/extmark.c b/src/nvim/api/extmark.c
index fd07ec96fe..d7e1aa2d0f 100644
--- a/src/nvim/api/extmark.c
+++ b/src/nvim/api/extmark.c
@@ -215,8 +215,8 @@ ArrayOf(Integer) nvim_buf_get_extmark_by_id(Buffer buffer, Integer ns_id,
   return extmark_to_array(extmark, false, details, hl_name);
 }
 
-/// Gets |extmarks| (including |signs|) in "traversal order" from a |charwise|
-/// region defined by buffer positions (inclusive, 0-indexed |api-indexing|).
+/// Gets |extmarks| in "traversal order" from a |charwise| region defined by
+/// buffer positions (inclusive, 0-indexed |api-indexing|).
 ///
 /// Region can be given as (row,col) tuples, or valid extmark ids (whose
 /// positions define the bounds). 0 and -1 are understood as (0,0) and (-1,-1)
@@ -234,6 +234,10 @@ ArrayOf(Integer) nvim_buf_get_extmark_by_id(Buffer buffer, Integer ns_id,
 /// the `overlap` option might be useful. Otherwise only the start position
 /// of an extmark will be considered.
 ///
+/// Note: legacy signs placed through the |:sign| commands are implemented
+/// as extmarks and will show up here. Their details array will contain a
+/// `sign_name` field.
+///
 /// Example:
 ///
 /// ```lua
@@ -434,7 +438,9 @@ Array nvim_buf_get_extmarks(Buffer buffer, Integer ns_id, Object start, Object e
 ///                   if text around the mark was deleted and then restored by undo.
 ///                   Defaults to true.
 ///               - invalidate : boolean that indicates whether to hide the
-///                   extmark if the entirety of its range is deleted. If
+///                   extmark if the entirety of its range is deleted. For
+///                   hidden marks, an "invalid" key is added to the "details"
+///                   array of |nvim_buf_get_extmarks()| and family. If
 ///                   "undo_restore" is false, the extmark is deleted instead.
 ///               - priority: a priority value for the highlight group or sign
 ///                   attribute. For example treesitter highlighting uses a
diff --git a/src/nvim/extmark.c b/src/nvim/extmark.c
index 0321a11b0f..4e4db93a7b 100644
--- a/src/nvim/extmark.c
+++ b/src/nvim/extmark.c
@@ -150,7 +150,11 @@ void extmark_del(buf_T *buf, MarkTreeIter *itr, MTKey key, bool restore)
   }
 
   if (mt_decor_any(key)) {
-    buf_decor_remove(buf, key.pos.row, key2.pos.row, mt_decor(key), true);
+    if (mt_invalid(key)) {
+      decor_free(mt_decor(key));
+    } else {
+      buf_decor_remove(buf, key.pos.row, key2.pos.row, mt_decor(key), true);
+    }
   }
 
   // TODO(bfredl): delete it from current undo header, opportunistically?
@@ -352,14 +356,12 @@ void extmark_splice_delete(buf_T *buf, int l_row, colnr_T l_col, int u_row, coln
 
     // Push mark to undo header
     if (only_copy || (uvp != NULL && op == kExtmarkUndo && !mt_no_undo(mark))) {
-      ExtmarkSavePos pos;
-      pos.mark = mt_lookup_key(mark);
-      pos.invalidated = invalidated;
-      pos.old_row = mark.pos.row;
-      pos.old_col = mark.pos.col;
-      pos.row = -1;
-      pos.col = -1;
-
+      ExtmarkSavePos pos = {
+        .mark = mt_lookup_key(mark),
+        .invalidated = invalidated,
+        .old_row = mark.pos.row,
+        .old_col = mark.pos.col
+      };
       undo.data.savepos = pos;
       undo.type = kExtmarkSavePos;
       kv_push(*uvp, undo);
@@ -393,22 +395,17 @@ void extmark_apply_undo(ExtmarkUndoObject undo_info, bool undo)
   } 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);
-      }
-      if (pos.invalidated) {
+      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);
       }
-      // Redo
-    } else {
-      if (pos.row >= 0) {
-        extmark_setraw(curbuf, pos.mark, pos.row, pos.col);
-      }
     }
+    // No Redo since kExtmarkSplice will move marks back
   } else if (undo_info.type == kExtmarkMove) {
     ExtmarkMove move = undo_info.data.move;
     if (undo) {
diff --git a/src/nvim/extmark.h b/src/nvim/extmark.h
index a68bbf33f2..b1ef5cf214 100644
--- a/src/nvim/extmark.h
+++ b/src/nvim/extmark.h
@@ -45,8 +45,6 @@ typedef struct {
   uint64_t mark;  // raw mark id of the marktree
   int old_row;
   colnr_T old_col;
-  int row;
-  colnr_T col;
   bool invalidated;
 } ExtmarkSavePos;
 
diff --git a/test/functional/api/extmark_spec.lua b/test/functional/api/extmark_spec.lua
index 54f4aaab03..19ad26fa86 100644
--- a/test/functional/api/extmark_spec.lua
+++ b/test/functional/api/extmark_spec.lua
@@ -1712,6 +1712,10 @@ describe('API/extmarks', function()
           aaa bbb ccc                         |*2
                                               |
     ]])
+    -- decor is not removed twice
+    command('d3')
+    api.nvim_buf_del_extmark(0, ns, 1)
+    command('silent undo')
     -- mark is deleted with undo_restore == false
     set_extmark(ns, 1, 0, 0, { invalidate = true, undo_restore = false, sign_text = 'S1' })
     set_extmark(ns, 2, 1, 0, { invalidate = true, undo_restore = false, sign_text = 'S2' })