fix(surround)!: make find, find_left, highlight not dot-repeatable

Resolve #1248

CHANGELOG.md

@@ -4,6 +4,10 @@
 
 FEATURE: add `'lowmedium'` and `'mediumhigh'` saturation levels.
 
+## mini.surround
+
+- BREAKING: created mappings for `find`, `find_left`, and `highlight` are now *not* dot-repeatable. Dot-repeat should repeat last text change but neither of those actions change text. Having them dot-repeatable breaks the common "move cursor -> press dot" workflow. Initially making them dot-repeatable was a "you can but you should not" type of mistake.
+
 ## mini.test
 
 - FEATURE: add `n_retry` test set property. When set, each case will be tried that at most that many times until first success (if any).

doc/mini-surround.txt

@@ -9,7 +9,7 @@ Fast and feature-rich surrounding. Can be configured to have experience
 similar to 'tpope/vim-surround' (see |MiniSurround-vim-surround-config|).
 
 Features:
-- Actions (all of them are dot-repeatable out of the box and respect
+- Actions (text editing actions are dot-repeatable out of the box and respect
   |[count]|) with configurable keymappings:
     - Add surrounding with `sa` (in visual mode or on motion).
     - Delete surrounding with `sd`.
@@ -690,7 +690,7 @@ Notes:
 - It creates new mappings only for actions involving surrounding search:
   delete, replace, find (right and left), highlight.
 - All new mappings behave the same way as if `config.search_method` is set
-  to certain search method. They are dot-repeatable, respect |[count]|, etc.
+  to certain search method. They preserve dot-repeat support, respect |[count]|.
 - Supply empty string to disable creation of corresponding set of mappings.
 
 Example with default values (`n` for `suffix_next`, `l` for `suffix_last`)

lua/mini/surround.lua

@@ -9,7 +9,7 @@
 --- similar to 'tpope/vim-surround' (see |MiniSurround-vim-surround-config|).
 ---
 --- Features:
---- - Actions (all of them are dot-repeatable out of the box and respect
+--- - Actions (text editing actions are dot-repeatable out of the box and respect
 ---   |[count]|) with configurable keymappings:
 ---     - Add surrounding with `sa` (in visual mode or on motion).
 ---     - Delete surrounding with `sd`.
@@ -659,7 +659,7 @@ end
 --- - It creates new mappings only for actions involving surrounding search:
 ---   delete, replace, find (right and left), highlight.
 --- - All new mappings behave the same way as if `config.search_method` is set
----   to certain search method. They are dot-repeatable, respect |[count]|, etc.
+---   to certain search method. They preserve dot-repeat support, respect |[count]|.
 --- - Supply empty string to disable creation of corresponding set of mappings.
 ---
 --- Example with default values (`n` for `suffix_next`, `l` for `suffix_last`)
@@ -854,7 +854,7 @@ end
 MiniSurround.find = function()
   -- Find surrounding region
   local surr = H.find_surrounding(H.get_surround_spec('input', true))
-  if surr == nil then return '<Esc>' end
+  if surr == nil then return end
 
   -- Make array of unique positions to cycle through
   local pos_array = H.surr_to_pos_array(surr)
@@ -873,7 +873,7 @@ end
 MiniSurround.highlight = function()
   -- Find surrounding region
   local surr = H.find_surrounding(H.get_surround_spec('input', true))
-  if surr == nil then return '<Esc>' end
+  if surr == nil then return end
 
   -- Highlight surrounding region
   local config = H.get_config()
@@ -1130,8 +1130,8 @@ H.builtin_surroundings = {
 -- Cache for dot-repeatability. This table is currently used with these keys:
 -- - 'input' - surround info for searching (in 'delete' and 'replace' start).
 -- - 'output' - surround info for adding (in 'add' and 'replace' end).
--- - 'direction' - direction in which `MiniSurround.find()` should go. Used to
---   enable same `operatorfunc` pattern for dot-repeatability.
+-- - 'direction' - direction in which `MiniSurround.find()` should go.
+--   Currently is not used for dot-repeat, but for easier mappings.
 -- - 'search_method' - search method.
 -- - 'msg_shown' - whether helper message was shown.
 H.cache = {}
@@ -1175,51 +1175,52 @@ H.apply_config = function(config)
   MiniSurround.config = config
 
   local expr_map = function(lhs, rhs, desc) H.map('n', lhs, rhs, { expr = true, desc = desc }) end
+  local map = function(lhs, rhs, desc) H.map('n', lhs, rhs, { desc = desc }) end
+
   --stylua: ignore start
   -- Make regular mappings
   local m = config.mappings
 
-  expr_map(m.add,       H.make_operator('add', nil, nil, true), 'Add surrounding')
-  expr_map(m.delete,    H.make_operator('delete'),              'Delete surrounding')
-  expr_map(m.replace,   H.make_operator('replace'),             'Replace surrounding')
-  expr_map(m.find,      H.make_operator('find', 'right'),       'Find right surrounding')
-  expr_map(m.find_left, H.make_operator('find', 'left'),        'Find left surrounding')
-  expr_map(m.highlight, H.make_operator('highlight'),           'Highlight surrounding')
+  expr_map(m.add,     H.make_operator('add', nil, true), 'Add surrounding')
+  expr_map(m.delete,  H.make_operator('delete'),         'Delete surrounding')
+  expr_map(m.replace, H.make_operator('replace'),        'Replace surrounding')
+
+  map(m.find,      H.make_action('find', 'right'), 'Find right surrounding')
+  map(m.find_left, H.make_action('find', 'left'),  'Find left surrounding')
+  map(m.highlight, H.make_action('highlight'),     'Highlight surrounding')
 
   H.map('n', m.update_n_lines, MiniSurround.update_n_lines, { desc = 'Update `MiniSurround.config.n_lines`' })
   H.map('x', m.add, [[:<C-u>lua MiniSurround.add('visual')<CR>]], { desc = 'Add surrounding to selection' })
 
   -- Make extended mappings
-  local suffix_map = function(lhs, suffix, rhs, desc)
+  local suffix_expr_map = function(lhs, suffix, rhs, desc)
     -- Don't create extended mapping if user chose not to create regular one
     if lhs == '' then return end
     expr_map(lhs .. suffix, rhs, desc)
   end
+  local suffix_map = function(lhs, suffix, rhs, desc)
+    if lhs == '' then return end
+    map(lhs .. suffix, rhs, desc)
+  end
 
   if m.suffix_last ~= '' then
-    local operator_prev = function(method, direction)
-      return H.make_operator(method, direction, 'prev')
-    end
-
     local suff = m.suffix_last
-    suffix_map(m.delete,    suff, operator_prev('delete'),        'Delete previous surrounding')
-    suffix_map(m.replace,   suff, operator_prev('replace'),       'Replace previous surrounding')
-    suffix_map(m.find,      suff, operator_prev('find', 'right'), 'Find previous right surrounding')
-    suffix_map(m.find_left, suff, operator_prev('find', 'left'),  'Find previous left surrounding')
-    suffix_map(m.highlight, suff, operator_prev('highlight'),     'Highlight previous surrounding')
+    suffix_expr_map(m.delete,  suff, H.make_operator('delete',  'prev'), 'Delete previous surrounding')
+    suffix_expr_map(m.replace, suff, H.make_operator('replace', 'prev'), 'Replace previous surrounding')
+
+    suffix_map(m.find,      suff, H.make_action('find', 'right',  'prev'), 'Find previous right surrounding')
+    suffix_map(m.find_left, suff, H.make_action('find', 'left',   'prev'), 'Find previous left surrounding')
+    suffix_map(m.highlight, suff, H.make_action('highlight', nil, 'prev'), 'Highlight previous surrounding')
   end
 
   if m.suffix_next ~= '' then
-    local operator_next = function(method, direction)
-      return H.make_operator(method, direction, 'next')
-    end
-
     local suff = m.suffix_next
-    suffix_map(m.delete,    suff, operator_next('delete'),        'Delete next surrounding')
-    suffix_map(m.replace,   suff, operator_next('replace'),       'Replace next surrounding')
-    suffix_map(m.find,      suff, operator_next('find', 'right'), 'Find next right surrounding')
-    suffix_map(m.find_left, suff, operator_next('find', 'left'),  'Find next left surrounding')
-    suffix_map(m.highlight, suff, operator_next('highlight'),     'Highlight next surrounding')
+    suffix_expr_map(m.delete,  suff, H.make_operator('delete',  'next'), 'Delete next surrounding')
+    suffix_expr_map(m.replace, suff, H.make_operator('replace', 'next'), 'Replace next surrounding')
+
+    suffix_map(m.find,      suff, H.make_action('find', 'right',  'next'), 'Find next right surrounding')
+    suffix_map(m.find_left, suff, H.make_action('find', 'left',   'next'), 'Find next left surrounding')
+    suffix_map(m.highlight, suff, H.make_action('highlight', nil, 'next'), 'Highlight next surrounding')
   end
   --stylua: ignore end
 end
@@ -1256,15 +1257,15 @@ H.validate_search_method = function(x, x_name)
 end
 
 -- Mappings -------------------------------------------------------------------
-H.make_operator = function(task, direction, search_method, ask_for_textobject)
+H.make_operator = function(task, search_method, ask_for_textobject)
   return function()
     if H.is_disabled() then
       -- Using `<Esc>` helps to stop moving cursor caused by current
       -- implementation detail of adding `' '` inside expression mapping
       return [[\<Esc>]]
     end
 
-    H.cache = { count = vim.v.count1, direction = direction, search_method = search_method }
+    H.cache = { count = vim.v.count1, search_method = search_method }
 
     vim.o.operatorfunc = 'v:lua.MiniSurround.' .. task
 
@@ -1277,6 +1278,14 @@ H.make_operator = function(task, direction, search_method, ask_for_textobject)
   end
 end
 
+H.make_action = function(task, direction, search_method)
+  return function()
+    if H.is_disabled() then return end
+    H.cache = { count = vim.v.count1, direction = direction, search_method = search_method }
+    return MiniSurround[task]()
+  end
+end
+
 -- Work with surrounding info -------------------------------------------------
 H.get_surround_spec = function(sur_type, use_cache)
   local res

tests/screenshots/tests-test_surround.lua---Highlight-surrounding---works-in-extended-mappings-003

@@ -1,13 +1,13 @@
 -|---------|-----
-1|(aa) (bb) (cc) 
+1|(aa) (bb) (cx) 
 2|~              
 3|~              
-4|<e] [+] 1,2 All
+4|<e] [+] 1,13   
 5|               
 
 -|---------|-----
-1|000001001000000
-2|222222222222222
-3|222222222222222
-4|333333333333333
-5|444444444444444
+1|000000000000000
+2|111111111111111
+3|111111111111111
+4|222222222222222
+5|333333333333333

tests/screenshots/tests-test_surround.lua---Highlight-surrounding---works-with-dot-repeat-005 → tests/screenshots/tests-test_surround.lua---Highlight-surrounding---works-without-dot-repeat-004

@@ -1,5 +1,5 @@
 -|---------|--
-1|(aaa) (bbb) 
+1|(axa) (bbb) 
 2|~           
 3|~           
 4|< [+] 1,3   

tests/test_surround.lua

@@ -1208,44 +1208,36 @@ T['Find surrounding'] = new_set()
 
 -- NOTE: most tests are done for `sf` ('find right') in hope that `sF` ('find
 -- left') is implemented similarly
-T['Find surrounding']['works with dot-repeat'] = function()
+T['Find surrounding']['works without dot-repeat'] = function()
   validate_find({ '(aaa)' }, { 1, 0 }, { { 1, 4 }, { 1, 0 }, { 1, 4 } }, type_keys, 'sf', ')')
   validate_find({ '(aaa)' }, { 1, 2 }, { { 1, 4 }, { 1, 0 }, { 1, 4 } }, type_keys, 'sf', ')')
   validate_find({ '(aaa)' }, { 1, 4 }, { { 1, 0 }, { 1, 4 }, { 1, 0 } }, type_keys, 'sf', ')')
 
-  -- Allows immediate dot-repeat
+  -- Does not override dot-repeat
   set_lines({ '(aaa)' })
+  set_cursor(1, 0)
+  type_keys('r]', 'u') -- dot-repeatable action
   set_cursor(1, 2)
   type_keys('sf', ')')
   type_keys('.')
-  eq(get_lines(), { '(aaa)' })
-  eq(get_cursor(), { 1, 0 })
-
-  -- Allows not immediate dot-repeat
-  set_lines({ 'aaa (bbb)' })
-  set_cursor(1, 5)
-  type_keys('.')
-  eq(get_cursor(), { 1, 8 })
+  eq(get_lines(), { '(aaa]' })
+  eq(get_cursor(), { 1, 4 })
 end
 
-T['Find surrounding']['works in left direction with dot-repeat'] = function()
+T['Find surrounding']['works in left direction without dot-repeat'] = function()
   validate_find({ '(aaa)' }, { 1, 0 }, { { 1, 4 }, { 1, 0 }, { 1, 4 } }, type_keys, 'sF', ')')
   validate_find({ '(aaa)' }, { 1, 4 }, { { 1, 0 }, { 1, 4 }, { 1, 0 } }, type_keys, 'sF', ')')
   validate_find({ '(aaa)' }, { 1, 2 }, { { 1, 0 }, { 1, 4 }, { 1, 0 } }, type_keys, 'sF', ')')
 
-  -- Allows immediate dot-repeat
+  -- Does not override dot-repeat
   set_lines({ '(aaa)' })
+  set_cursor(1, 0)
+  type_keys('r[', 'u') -- dot-repeatable action
   set_cursor(1, 2)
   type_keys('sF', ')')
   type_keys('.')
-  eq(get_lines(), { '(aaa)' })
-  eq(get_cursor(), { 1, 4 })
-
-  -- Allows not immediate dot-repeat
-  set_lines({ 'aaa (bbb)' })
-  set_cursor(1, 5)
-  type_keys('.')
-  eq(get_cursor(), { 1, 4 })
+  eq(get_lines(), { '[aaa)' })
+  eq(get_cursor(), { 1, 0 })
 end
 
 T['Find surrounding']['works with "non single character" surroundings'] = function()
@@ -1282,13 +1274,14 @@ T['Find surrounding']['works in extended mappings'] = function()
   validate_edit1d('(aa) (bb) (cc)', 11, '(aa) (bb) (cc)', 8, type_keys, 'sFl', ')')
   validate_edit1d('(aa) (bb) (cc)', 11, '(aa) (bb) (cc)', 3, type_keys, '2sFl', ')')
 
-  -- Dot-repeat
+  -- Does not override dot-repeat
   set_lines({ '(aa) (bb) (cc)' })
   set_cursor(1, 0)
+  type_keys('r[', 'u') -- dot-repeatable action
   type_keys('sfn', ')')
   type_keys('.')
-  eq(get_lines(), { '(aa) (bb) (cc)' })
-  eq(get_cursor(), { 1, 10 })
+  eq(get_lines(), { '(aa) [bb) (cc)' })
+  eq(get_cursor(), { 1, 5 })
 end
 
 T['Find surrounding']['respects `config.n_lines`'] = function()
@@ -1479,8 +1472,12 @@ local activate_highlighting = function()
   child.poke_eventloop()
 end
 
-T['Highlight surrounding']['works with dot-repeat'] = function()
+T['Highlight surrounding']['works without dot-repeat'] = function()
   local test_duration = child.lua_get('MiniSurround.config.highlight_duration')
+  set_lines({ ' ' })
+  set_cursor(1, 0)
+  type_keys('rx') -- dot-repeatable action
+
   set_lines({ '(aaa) (bbb)' })
   set_cursor(1, 2)
 
@@ -1496,24 +1493,21 @@ T['Highlight surrounding']['works with dot-repeat'] = function()
   sleep(2 * small_time + small_time)
   child.expect_screenshot()
 
-  -- Should highlight with dot-repeat
+  -- Does not override dot-repeat
   type_keys('.')
-  child.expect_screenshot()
-
-  -- Should stop highlighting
-  sleep(test_duration + small_time)
-  child.expect_screenshot()
 
-  -- Should allow not immediate dot-repeat
-  set_cursor(1, 8)
-  type_keys('.')
+  -- - No highlighting should be present
   child.expect_screenshot()
 end
 
 T['Highlight surrounding']['works in extended mappings'] = function()
   child.set_size(5, 15)
   local test_duration = child.lua_get('MiniSurround.config.highlight_duration')
+  set_lines({ ' ' })
+  set_cursor(1, 0)
+  type_keys('rx') -- dot-repeatable action
   set_lines({ '(aa) (bb) (cc)' })
+  set_cursor(1, 0)
 
   set_cursor(1, 1)
   type_keys('shn', ')')
@@ -1527,12 +1521,10 @@ T['Highlight surrounding']['works in extended mappings'] = function()
   child.expect_screenshot()
   sleep(test_duration + small_time)
 
-  -- Dot-repeat
-  set_cursor(1, 1)
-  type_keys('shn', ')')
-  sleep(test_duration + small_time)
+  -- Does not override dot-repeat
   type_keys('.')
-  child.poke_eventloop()
+
+  -- - No highlighting should be present
   child.expect_screenshot()
 end