#333 Save sign IDs back on loclist items, and make it possible to get line numbers again

4 files changed, 51 insertions, 48 deletions

diff --git a/autoload/ale/sign.vim b/autoload/ale/sign.vim
index e95de58f..1a39ba89 100644
--- a/autoload/ale/sign.vim
+++ b/autoload/ale/sign.vim
@@ -28,7 +28,7 @@ function! ale#sign#ReadSigns(buffer) abort
     return split(l:output, "\n")
 endfunction
 
-" Given a list of lines for sign output, return a list of sign IDs
+" Given a list of lines for sign output, return a List of pairs [line, id]
 function! ale#sign#ParseSigns(line_list) abort
     " Matches output like :
     " line=4  id=1  name=ALEErrorSign
@@ -36,20 +36,19 @@ function! ale#sign#ParseSigns(line_list) abort
     " 行=1  識別子=1000001  名前=ALEWarningSign
     " línea=12 id=1000001 nombre=ALEWarningSign
     " riga=1 id=1000001, nome=ALEWarningSign
-    let l:pattern = '^.*=\d*\s\+.*=\(\d\+\)\,\?\s\+.*=ALE\(Warning\|Error\|Dummy\)Sign'
+    let l:pattern = '^.*=\(\d\+\).*=\(\d\+\).*=ALE\(Error\|Warning\|Dummy\)Sign'
 
-
-    let l:id_list = []
+    let l:result = []
 
     for l:line in a:line_list
         let l:match = matchlist(l:line, l:pattern)
 
         if len(l:match) > 0
-            call add(l:id_list, l:match[1] + 0)
+            call add(l:result, [str2nr(l:match[1]), str2nr(l:match[2])])
         endif
     endfor
 
-    return l:id_list
+    return l:result
 endfunction
 
 function! ale#sign#FindCurrentSigns(buffer) abort
@@ -58,54 +57,46 @@ function! ale#sign#FindCurrentSigns(buffer) abort
     return ale#sign#ParseSigns(l:line_list)
 endfunction
 
-" Given a loclist, combine the loclist into a list of signs such that only
-" one sign appears per line. Error lines will take precedence.
-" The loclist will have been previously sorted.
-function! ale#sign#CombineSigns(loclist) abort
+" Given a loclist, group the List into with one List per line.
+function! s:GroupSigns(loclist) abort
     let l:signlist = []
+    let l:last_lnum = -1
 
     for l:obj in a:loclist
-        let l:should_append = 1
-
-        if l:obj.lnum < 1
-            " Skip warnings and errors at line 0, etc.
-            continue
+        " Create a new sub-List when we hit a new line.
+        if l:obj.lnum != l:last_lnum
+            call add(l:signlist, [])
         endif
 
-        if len(l:signlist) > 0 && l:signlist[-1].lnum == l:obj.lnum
-            " We can't add the same line twice, because signs must be
-            " unique per line.
-            let l:should_append = 0
+        call add(l:signlist[-1], l:obj)
+        let l:last_lnum = l:obj.lnum
+    endfor
+
+    return l:signlist
+endfunction
 
-            if l:signlist[-1].type ==# 'W' && l:obj.type ==# 'E'
-                " If we had a warning previously, but now have an error,
-                " we replace the object to set an error instead.
-                let l:signlist[-1] = l:obj
-            endif
+function! s:IsDummySignSet(current_id_list) abort
+    for [l:line, l:id] in a:current_id_list
+        if l:id == g:ale_sign_offset
+            return 1
         endif
 
-        if l:should_append
-            call add(l:signlist, l:obj)
+        if l:line > 1
+            return 0
         endif
     endfor
 
-    return l:signlist
+    return 0
 endfunction
 
 " This function will set the signs which show up on the left.
 function! ale#sign#SetSigns(buffer, loclist) abort
-    let l:signlist = ale#sign#CombineSigns(a:loclist)
+    let l:signlist = s:GroupSigns(a:loclist)
 
     " Find the current markers
     let l:current_id_list = ale#sign#FindCurrentSigns(a:buffer)
-    let l:dummy_sign_set = 0
-
     " Check if we set the dummy sign already.
-    for l:current_id in l:current_id_list
-        if l:current_id == g:ale_sign_offset
-            let l:dummy_sign_set = 1
-        endif
-    endfor
+    let l:dummy_sign_set = s:IsDummySignSet(l:current_id_list)
 
     " If we haven't already set a dummy sign, and we have some previous signs
     " or always want a dummy sign, then set one, to keep the sign column open.
@@ -119,7 +110,7 @@ function! ale#sign#SetSigns(buffer, loclist) abort
 
     " Now remove the previous signs. The dummy will hold the column open
     " while we add the new signs, if we had signs before.
-    for l:current_id in l:current_id_list
+    for [l:line, l:current_id] in l:current_id_list
         if l:current_id != g:ale_sign_offset
             exec 'sign unplace ' . l:current_id . ' buffer=' . a:buffer
         endif
@@ -127,11 +118,21 @@ function! ale#sign#SetSigns(buffer, loclist) abort
 
     " Add the new signs,
     for l:index in range(0, len(l:signlist) - 1)
-        let l:sign = l:signlist[l:index]
-        let l:type = l:sign['type'] ==# 'W' ? 'ALEWarningSign' : 'ALEErrorSign'
-
-        let l:sign_line = 'sign place ' . (l:index + g:ale_sign_offset + 1)
-            \. ' line=' . l:sign['lnum']
+        let l:sign_id = l:index + g:ale_sign_offset + 1
+        let l:sublist = l:signlist[l:index]
+        let l:type = !empty(filter(copy(l:sublist), 'v:val.type ==# ''E'''))
+        \   ? 'ALEErrorSign'
+        \   : 'ALEWarningSign'
+
+        " Save the sign IDs we are setting back on our loclist objects.
+        " These IDs can be used later for changing line numbers of items
+        " we keep, based on what Vim adjusts automatically.
+        for l:obj in l:sublist
+            let l:obj.sign_id = l:sign_id
+        endfor
+
+        let l:sign_line = 'sign place ' . l:sign_id
+            \. ' line=' . l:sublist[0].lnum
             \. ' name=' . l:type
             \. ' buffer=' . a:buffer
 
diff --git a/test/sign/test_sign_parsing.vader b/test/sign/test_sign_parsing.vader
index a0ed26e4..916fb570 100644
--- a/test/sign/test_sign_parsing.vader
+++ b/test/sign/test_sign_parsing.vader
@@ -1,14 +1,14 @@
 Execute (Parsing English signs should work):
-  AssertEqual [1000001], ale#sign#ParseSigns(['Signs for app.js:', '    line=9  id=1000001  name=ALEWarningSign'])
+  AssertEqual [[9, 1000001]], ale#sign#ParseSigns(['Signs for app.js:', '    line=9  id=1000001  name=ALEWarningSign'])
 
 Execute (Parsing Russian signs should work):
-  AssertEqual [1000001], ale#sign#ParseSigns(['    строка=1  id=1000001  имя=ALEErrorSign'])
+  AssertEqual [[1, 1000001]], ale#sign#ParseSigns(['    строка=1  id=1000001  имя=ALEErrorSign'])
 
 Execute (Parsing Japanese signs should work):
-  AssertEqual [1000001], ale#sign#ParseSigns(['    行=1  識別子=1000001  名前=ALEWarningSign'])
+  AssertEqual [[1, 1000001]], ale#sign#ParseSigns(['    行=1  識別子=1000001  名前=ALEWarningSign'])
 
 Execute (Parsing Spanish signs should work):
-  AssertEqual [1000001], ale#sign#ParseSigns(['    línea=12 id=1000001 nombre=ALEWarningSign'])
+  AssertEqual [[12, 1000001]], ale#sign#ParseSigns(['    línea=12 id=1000001 nombre=ALEWarningSign'])
 
 Execute (Parsing Italian signs should work):
-  AssertEqual [1000001], ale#sign#ParseSigns(['    riga=1 id=1000001, nome=ALEWarningSign'])
+  AssertEqual [[1, 1000001]], ale#sign#ParseSigns(['    riga=1 id=1000001, nome=ALEWarningSign'])
diff --git a/test/test_ale_toggle.vader b/test/test_ale_toggle.vader
index 61557e8a..90dba401 100644
--- a/test/test_ale_toggle.vader
+++ b/test/test_ale_toggle.vader
@@ -85,7 +85,7 @@ Execute(ALEToggle should reset everything and then run again):
 
   " First check that everything is there...
   AssertEqual g:expected_loclist, getloclist(0)
-  AssertEqual [1000001], ale#sign#FindCurrentSigns(bufnr('%'))
+  AssertEqual [[2, 1000001]], ale#sign#FindCurrentSigns(bufnr('%'))
   AssertEqual
   \ [{'group': 'ALEError', 'pos1': [2, 3, 1]}],
   \ map(getmatches(), '{''group'': v:val.group, ''pos1'': v:val.pos1}')
@@ -105,7 +105,7 @@ Execute(ALEToggle should reset everything and then run again):
   call ale#engine#WaitForJobs(2000)
 
   AssertEqual g:expected_loclist, getloclist(0)
-  AssertEqual [1000001], ale#sign#FindCurrentSigns(bufnr('%'))
+  AssertEqual [[2, 1000001]], ale#sign#FindCurrentSigns(bufnr('%'))
   AssertEqual
   \ [{'group': 'ALEError', 'pos1': [2, 3, 1]}],
   \ map(getmatches(), '{''group'': v:val.group, ''pos1'': v:val.pos1}')
diff --git a/test/test_linting_updates_loclist.vader b/test/test_linting_updates_loclist.vader
index 06f11d8b..2e106d05 100644
--- a/test/test_linting_updates_loclist.vader
+++ b/test/test_linting_updates_loclist.vader
@@ -13,6 +13,7 @@ Before:
   \   'type': 'W',
   \   'col': 10,
   \   'text': 'Infix operators must be spaced. [Warning/space-infix-ops]',
+  \   'sign_id': 1000001,
   \ },
   \ {
   \   'lnum': 2,
@@ -23,6 +24,7 @@ Before:
   \   'type': 'E',
   \   'col': 10,
   \   'text': 'Missing semicolon. [Error/semi]',
+  \   'sign_id': 1000002,
   \ }
   \]