Fix ambi_pair both sides match in '' phase search

Resolve the higher-level shortcoming of `sub_pair`: a pair consists of
"begin items" and "end items" groups, but have no indication that they
are serving the same purpose.

Add a auto-increment `pair_id` to both side of groups.

- Raise up the scope of `search_ctx` from `phased_search()` to
  `#search()`, let the context lives across phases.

- When an end_item matched, remember the pair_id in context.
  Then for any `sub_pair` group, we can check if the opposite has
  already matched by the pair_id memo.

Author: bootleq

autoload/cycle.vim

@@ -10,6 +10,7 @@
 "   sub_pair
 "   sub_pairs
 "   ambi_pair
+"   pair_id
 "   end_with
 "   begin_with
 "   matcher
@@ -25,6 +26,7 @@
 "   subp
 
 let s:tick = 0
+let s:pair_id = 0
 
 " }}} Constants
 
@@ -140,6 +142,7 @@ function! cycle#search(class_name, ...) "{{{
   let direction = get(options, 'direction', 1)
   let l:count = get(options, 'count', 1)
   let matches = []
+  let search_ctx = {}
   let cword = cycle#text#new_cword()
   let cchar = cycle#text#new_cchar()
 
@@ -166,7 +169,7 @@ function! cycle#search(class_name, ...) "{{{
       endif
     endif
 
-    let phase_matches = s:phased_search(phase, groups, direction, l:count)
+    let phase_matches = s:phased_search(phase, groups, direction, l:count, search_ctx)
 
     if !empty(phase_matches)
       call extend(matches, phase_matches)
@@ -177,9 +180,8 @@ function! cycle#search(class_name, ...) "{{{
 endfunction "}}}
 
 
-function! s:phased_search(class_name, groups, direction, count) "{{{
+function! s:phased_search(class_name, groups, direction, count, search_ctx) "{{{
   let matches = []
-  let search_ctx = {}
 
   for group in a:groups
     if get(group, '_phase_matched', 0)
@@ -203,7 +205,7 @@ function! s:phased_search(class_name, groups, direction, count) "{{{
       break
     endif
 
-    let [index, ctext] = s:group_search(group, a:class_name, search_ctx)
+    let [index, ctext] = s:group_search(group, a:class_name, a:search_ctx)
     if index >= 0
       if a:count == '*'
         " Grab all group items for CycleSelect
@@ -483,8 +485,9 @@ function! cycle#parse_group(group_attrs) "{{{
     endif
     " Note that the "end_items" (has `begin_with`) must go first, `ambi_pair`
     " relies on this order to make orphaned behave as the begin part.
-    let end_group_attrs = [end_items, extend(deepcopy(options), {'begin_with': begin_items})]
-    let begin_group_attrs = [begin_items, extend(deepcopy(options), {'end_with': end_items})]
+    let s:pair_id += 1
+    let end_group_attrs = [end_items, extend(deepcopy(options), {'begin_with': begin_items, 'pair_id': s:pair_id})]
+    let begin_group_attrs = [begin_items, extend(deepcopy(options), {'end_with': end_items, 'pair_id': s:pair_id})]
     return [cycle#parse_group(end_group_attrs),
           \ cycle#parse_group(begin_group_attrs)]
   endif

autoload/cycle/matcher/default.vim

@@ -10,6 +10,14 @@ function! cycle#matcher#default#test(group, class_name, ctx) abort "{{{
   let index = -1
   let ctext = cycle#text#new_ctext(a:class_name)
 
+  " Skip a sub_pair group if its opposite group has already found
+  if get(options, 'sub_pair', 0)
+    let found_pairs = get(a:ctx, 'found_pairs', [])
+    if index(found_pairs, get(options, 'pair_id')) > -1
+      return [index, ctext]
+    endif
+  endif
+
   for item in a:group.items
     if get(options, 'match_word') && a:class_name != 'w'
       continue
@@ -44,14 +52,16 @@ function! cycle#matcher#default#test(group, class_name, ctx) abort "{{{
 
       let ambi_pair = get(options, 'ambi_pair')
       if !empty(ambi_pair) && index(ambi_pair, text) > -1
-        " For begin pair, if there was no end pair found, accept it as orphan.
-        let accept_orphan = !empty(get(options, 'end_with')) && index(get(a:ctx, 'ambi_pair_found', []), text) < 0
+        " For begin pair, if there was no end pair found, we can accept it as orphan.
+        " This relies on the assumption that "end_items" were defined first.
+        let accept_orphan = !empty(get(options, 'end_with'))
 
         if cycle#matcher#default#ambi_pair#test(text, options)
-          let founds = get(a:ctx, 'ambi_pair_found', [])
-          let founds = uniq(founds + [text])
-          call extend(a:ctx, {'ambi_pair_found': founds}, 'force')
+          let founds = get(a:ctx, 'found_pairs', [])
+          let founds = uniq(founds + [get(options, 'pair_id', -1)])
+          call extend(a:ctx, {'found_pairs': founds}, 'force')
         elseif !accept_orphan
+          " Still accept it
           continue
         endif
       endif

doc/cycle.txt

@@ -389,6 +389,7 @@ By default no options are set.
 "before_sub"        (List) ~
 "after_sub"         (List) ~
 "ambi_pair"         (List) ~
+"pair_id"           (Number) ~
 "restrict_cursor"   (Number) ~
 
   Internal usage only, don't set them.

test/group_definition.vimspec

@@ -87,9 +87,24 @@ Describe Group definition
       Assert Equals(group, #{items: ['Yes', 'No'], options: #{match_case: 1}})
 
       let groups = cycle#parse_group([['a:b', 'c:d'], 'sub_pairs'])
+      let [g1, g2] = groups
+
       Assert IsList(groups)
-      Assert Equals(groups[0], #{items: ['b', 'd'], options: #{sub_pair: 1, begin_with: ['a', 'c']}})
-      Assert Equals(groups[1], #{items: ['a', 'c'], options: #{sub_pair: 1, end_with: ['b', 'd']}})
+      Assert Includes(g1, ['items', 'options'])
+      Assert Includes(g2, ['items', 'options'])
+      Assert Equals(g1['options']['sub_pair'], 1)
+      Assert Equals(g2['options']['sub_pair'], 1)
+
+      Assert Includes(g1['options'], ['sub_pair', 'pair_id', 'begin_with'])
+      Assert Includes(g2['options'], ['sub_pair', 'pair_id', 'end_with'])
+
+      Assert Equals(g1['items'], ['b', 'd'])
+      Assert Equals(g1['options']['begin_with'], ['a', 'c'])
+
+      Assert Equals(g2['items'], ['a', 'c'])
+      Assert Equals(g2['options']['end_with'], ['b', 'd'])
+
+      Assert Equals(g1['options']['pair_id'], g2['options']['pair_id'])
     End
 
     It takes one-time groups

test/sub_pairs.vimspec

@@ -174,14 +174,11 @@ Describe Sub pairs
 
       It from end, sub begin (potentially conflict in '' phase)
         let line = '" foo "'
-        call setline(1, '" foo "')
+        call setline(1, line)
         normal! $
 
         execute 'normal \a'
-        Assert Equals(getline(1), line, 'no change')
-        Assert Exists('g:cycle_test_conflict.items')
-        Assert Equals(g:cycle_test_conflict.items, ['`', '`'])
-        " FIXME: this is not desired behavior
+        Assert Equals(getline(1), '` foo `')
       End
 
       It subs orphaned item even the opposite not found