From 790c614b7a2360446aee6e003e3e834d21b2f04b Mon Sep 17 00:00:00 2001 From: w0rp Date: Tue, 14 Mar 2017 23:04:25 +0000 Subject: [PATCH] #333 Update line numbers for loclist items when current sign_id values are set --- autoload/ale/engine.vim | 9 +- autoload/ale/sign.vim | 124 ++++++++++++++++++++-------- test/sign/test_sign_parsing.vader | 23 ++++-- test/sign/test_sign_placement.vader | 69 +++++++++++++--- test/test_ale_toggle.vader | 4 +- 5 files changed, 173 insertions(+), 56 deletions(-) diff --git a/autoload/ale/engine.vim b/autoload/ale/engine.vim index 63362785..31591ded 100644 --- a/autoload/ale/engine.vim +++ b/autoload/ale/engine.vim @@ -254,14 +254,15 @@ function! s:HandleExit(job) abort endfunction function! ale#engine#SetResults(buffer, loclist) abort - if g:ale_set_quickfix || g:ale_set_loclist - call ale#list#SetLists(a:buffer, a:loclist) - endif - + " Set signs first. This could potentially fix some line numbers. if g:ale_set_signs call ale#sign#SetSigns(a:buffer, a:loclist) endif + if g:ale_set_quickfix || g:ale_set_loclist + call ale#list#SetLists(a:buffer, a:loclist) + endif + if exists('*ale#statusline#Update') " Don't load/run if not already loaded. call ale#statusline#Update(a:buffer, a:loclist) diff --git a/autoload/ale/sign.vim b/autoload/ale/sign.vim index fc12e158..0c4488b5 100644 --- a/autoload/ale/sign.vim +++ b/autoload/ale/sign.vim @@ -37,14 +37,17 @@ function! ale#sign#ParseSigns(line_list) abort " línea=12 id=1000001 nombre=ALEWarningSign " riga=1 id=1000001, nome=ALEWarningSign let l:pattern = '^.*=\(\d\+\).*=\(\d\+\).*=ALE\(Error\|Warning\|Dummy\)Sign' - 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:result, [str2nr(l:match[1]), str2nr(l:match[2])]) + call add(l:result, [ + \ str2nr(l:match[1]), + \ str2nr(l:match[2]), + \ 'ALE' . l:match[3] . 'Sign', + \]) endif endfor @@ -58,25 +61,25 @@ function! ale#sign#FindCurrentSigns(buffer) abort endfunction " Given a loclist, group the List into with one List per line. -function! s:GroupSigns(loclist) abort - let l:signlist = [] +function! s:GroupLoclistItems(loclist) abort + let l:grouped_items = [] let l:last_lnum = -1 for l:obj in a:loclist " Create a new sub-List when we hit a new line. if l:obj.lnum != l:last_lnum - call add(l:signlist, []) + call add(l:grouped_items, []) endif - call add(l:signlist[-1], l:obj) + call add(l:grouped_items[-1], l:obj) let l:last_lnum = l:obj.lnum endfor - return l:signlist + return l:grouped_items endfunction function! s:IsDummySignSet(current_id_list) abort - for [l:line, l:id] in a:current_id_list + for [l:line, l:id, l:name] in a:current_id_list if l:id == g:ale_sign_offset return 1 endif @@ -89,37 +92,27 @@ function! s:IsDummySignSet(current_id_list) abort 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 = s:GroupSigns(a:loclist) - - " Find the current markers - let l:current_id_list = ale#sign#FindCurrentSigns(a:buffer) - " Check if we set the dummy sign already. - let l:dummy_sign_set = s:IsDummySignSet(l:current_id_list) +function! s:SetDummySignIfNeeded(buffer, current_sign_list, new_signs) abort + let l:is_dummy_sign_set = s:IsDummySignSet(a:current_sign_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. - if !l:dummy_sign_set && (len(l:signlist) > 0 || g:ale_sign_column_always) + if !l:is_dummy_sign_set && (a:new_signs || g:ale_sign_column_always) execute 'sign place ' . g:ale_sign_offset \ . ' line=1 name=ALEDummySign buffer=' \ . a:buffer - let l:dummy_sign_set = 1 + let l:is_dummy_sign_set = 1 endif - " 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: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 - endfor + return l:is_dummy_sign_set +endfunction +function! s:PlaceNewSigns(buffer, grouped_items) abort " Add the new signs, - for l:index in range(0, len(l:signlist) - 1) + for l:index in range(0, len(a:grouped_items) - 1) let l:sign_id = l:index + g:ale_sign_offset + 1 - let l:sublist = l:signlist[l:index] + let l:sublist = a:grouped_items[l:index] let l:type = !empty(filter(copy(l:sublist), 'v:val.type ==# ''E''')) \ ? 'ALEErrorSign' \ : 'ALEWarningSign' @@ -130,18 +123,81 @@ function! ale#sign#SetSigns(buffer, loclist) abort 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 - - exec l:sign_line + execute 'sign place ' . l:sign_id + \ . ' line=' . l:sublist[0].lnum + \ . ' name=' . l:type + \ . ' buffer=' . a:buffer endfor +endfunction + +" Get items grouped by any current sign IDs they might have. +function! s:GetItemsWithSignIDs(loclist) abort + let l:items_by_sign_id = {} + + for l:item in a:loclist + if has_key(l:item, 'sign_id') + if !has_key(l:items_by_sign_id, l:item.sign_id) + let l:items_by_sign_id[l:item.sign_id] = [] + endif + + call add(l:items_by_sign_id[l:item.sign_id], l:item) + endif + endfor + + return l:items_by_sign_id +endfunction + +" Given some current signs and a loclist, look for items with sign IDs, +" and change the line numbers for loclist items to match the signs. +function! s:UpdateLineNumbers(current_sign_list, loclist) abort + let l:items_by_sign_id = s:GetItemsWithSignIDs(a:loclist) + + " Do nothing if there's nothing to work with. + if empty(l:items_by_sign_id) + return + endif + + for [l:line, l:sign_id, l:name] in a:current_sign_list + for l:obj in get(l:items_by_sign_id, l:sign_id, []) + let l:obj.lnum = l:line + endfor + endfor + + " Sort items again. + call sort(a:loclist, 'ale#util#LocItemCompare') +endfunction + +" This function will set the signs which show up on the left. +function! ale#sign#SetSigns(buffer, loclist) abort + " Find the current markers + let l:current_sign_list = ale#sign#FindCurrentSigns(a:buffer) + + call s:UpdateLineNumbers(l:current_sign_list, a:loclist) + + let l:grouped_items = s:GroupLoclistItems(a:loclist) + + " Set the dummy sign if we need to. + " This keeps the sign gutter open while we remove things, etc. + let l:is_dummy_sign_set = s:SetDummySignIfNeeded( + \ a:buffer, + \ l:current_sign_list, + \ !empty(l:grouped_items), + \) + + " 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:line, l:sign_id, l:name] in l:current_sign_list + if l:sign_id != g:ale_sign_offset + exec 'sign unplace ' . l:sign_id . ' buffer=' . a:buffer + endif + endfor + + call s:PlaceNewSigns(a:buffer, l:grouped_items) " Remove the dummy sign now we've updated the signs, unless we want " to keep it, which will keep the sign column open even when there are " no warnings or errors. - if l:dummy_sign_set && !g:ale_sign_column_always + if l:is_dummy_sign_set && !g:ale_sign_column_always execute 'sign unplace ' . g:ale_sign_offset . ' buffer=' . a:buffer endif endfunction diff --git a/test/sign/test_sign_parsing.vader b/test/sign/test_sign_parsing.vader index 916fb570..ee112a1b 100644 --- a/test/sign/test_sign_parsing.vader +++ b/test/sign/test_sign_parsing.vader @@ -1,14 +1,27 @@ Execute (Parsing English signs should work): - AssertEqual [[9, 1000001]], ale#sign#ParseSigns(['Signs for app.js:', ' line=9 id=1000001 name=ALEWarningSign']) + AssertEqual + \ [[9, 1000001, 'ALEWarningSign']], + \ ale#sign#ParseSigns([ + \ 'Signs for app.js:', + \ ' line=9 id=1000001 name=ALEWarningSign', + \ ]) Execute (Parsing Russian signs should work): - AssertEqual [[1, 1000001]], ale#sign#ParseSigns([' строка=1 id=1000001 имя=ALEErrorSign']) + AssertEqual + \ [[1, 1000001, 'ALEErrorSign']], + \ ale#sign#ParseSigns([' строка=1 id=1000001 имя=ALEErrorSign']) Execute (Parsing Japanese signs should work): - AssertEqual [[1, 1000001]], ale#sign#ParseSigns([' 行=1 識別子=1000001 名前=ALEWarningSign']) + AssertEqual + \ [[1, 1000001, 'ALEWarningSign']], + \ ale#sign#ParseSigns([' 行=1 識別子=1000001 名前=ALEWarningSign']) Execute (Parsing Spanish signs should work): - AssertEqual [[12, 1000001]], ale#sign#ParseSigns([' línea=12 id=1000001 nombre=ALEWarningSign']) + AssertEqual + \ [[12, 1000001, 'ALEWarningSign']], + \ ale#sign#ParseSigns([' línea=12 id=1000001 nombre=ALEWarningSign']) Execute (Parsing Italian signs should work): - AssertEqual [[1, 1000001]], ale#sign#ParseSigns([' riga=1 id=1000001, nome=ALEWarningSign']) + AssertEqual + \ [[1, 1000001, 'ALEWarningSign']], + \ ale#sign#ParseSigns([' riga=1 id=1000001, nome=ALEWarningSign']) diff --git a/test/sign/test_sign_placement.vader b/test/sign/test_sign_placement.vader index 6fa72c86..396ef5c4 100644 --- a/test/sign/test_sign_placement.vader +++ b/test/sign/test_sign_placement.vader @@ -46,6 +46,17 @@ Before: \] endfunction + function! ParseSigns() + redir => l:output + silent sign place + redir END + + return map( + \ split(l:output, '\n')[2:], + \ 'matchlist(v:val, ''^.*=\(\d\+\).*=\(\d\+\).*=\(.*\)$'')[1:3]', + \) + endfunction + call ale#linter#Define('testft', { \ 'name': 'x', \ 'executable': 'echo', @@ -54,9 +65,11 @@ Before: \}) After: - call ale#linter#Reset() + unlet! g:loclist delfunction GenerateResults - unlet! g:output + delfunction ParseSigns + call ale#linter#Reset() + sign unplace * Given testft(A file with warnings/errors): foo @@ -65,14 +78,10 @@ Given testft(A file with warnings/errors): fourth line fifth line -Execute: +Execute(The current signs should be set for running a job): call ale#Lint() call ale#engine#WaitForJobs(2000) - redir => g:output - silent sign place - redir END - AssertEqual \ [ \ ['1', '1000001', 'ALEErrorSign'], @@ -81,7 +90,45 @@ Execute: \ ['4', '1000004', 'ALEErrorSign'], \ ['5', '1000005', 'ALEErrorSign'], \ ], - \ map( - \ split(g:output, '\n')[2:], - \ 'matchlist(v:val, "[^=]*=\\(\\d\\+\\)[^=]*=\\(\\d\\+\\).*\\(ALE.*\\)$")[1:3]' - \ ) + \ ParseSigns() + + +Execute(Loclist items with sign_id values should be kept): + exec 'sign place 1000347 line=15 name=ALEErrorSign buffer=' . bufnr('%') + exec 'sign place 1000348 line=16 name=ALEWarningSign buffer=' . bufnr('%') + + let g:loclist = [ + \ {'lnum': 1, 'col': 1, 'type': 'E', 'text': 'a', 'sign_id': 1000347}, + \ {'lnum': 2, 'col': 1, 'type': 'W', 'text': 'b', 'sign_id': 1000348}, + \ {'lnum': 3, 'col': 1, 'type': 'E', 'text': 'c'}, + \ {'lnum': 4, 'col': 1, 'type': 'W', 'text': 'd'}, + \ {'lnum': 15, 'col': 2, 'type': 'W', 'text': 'e'}, + \ {'lnum': 16, 'col': 2, 'type': 'E', 'text': 'f'}, + \] + + call ale#sign#SetSigns(bufnr('%'), g:loclist) + + " Line numbers should be changed, sign_id values should be replaced, + " and items should be sorted again. + AssertEqual + \ [ + \ {'lnum': 3, 'col': 1, 'type': 'E', 'text': 'c', 'sign_id': 1000001}, + \ {'lnum': 4, 'col': 1, 'type': 'W', 'text': 'd', 'sign_id': 1000002}, + \ {'lnum': 15, 'col': 1, 'type': 'E', 'text': 'a', 'sign_id': 1000003}, + \ {'lnum': 15, 'col': 2, 'type': 'W', 'text': 'e', 'sign_id': 1000003}, + \ {'lnum': 16, 'col': 1, 'type': 'W', 'text': 'b', 'sign_id': 1000004}, + \ {'lnum': 16, 'col': 2, 'type': 'E', 'text': 'f', 'sign_id': 1000004}, + \ ], + \ g:loclist + + " Items should be grouped again. We should see error signs, where there + " were warnings before, and errors where there were errors and where we + " now have new warnings. + AssertEqual + \ [ + \ ['3', '1000001', 'ALEErrorSign'], + \ ['4', '1000002', 'ALEWarningSign'], + \ ['15', '1000003', 'ALEErrorSign'], + \ ['16', '1000004', 'ALEErrorSign'], + \ ], + \ ParseSigns() diff --git a/test/test_ale_toggle.vader b/test/test_ale_toggle.vader index 90dba401..2cd0110d 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 [[2, 1000001]], ale#sign#FindCurrentSigns(bufnr('%')) + AssertEqual [[2, 1000001, 'ALEErrorSign']], 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 [[2, 1000001]], ale#sign#FindCurrentSigns(bufnr('%')) + AssertEqual [[2, 1000001, 'ALEErrorSign']], ale#sign#FindCurrentSigns(bufnr('%')) AssertEqual \ [{'group': 'ALEError', 'pos1': [2, 3, 1]}], \ map(getmatches(), '{''group'': v:val.group, ''pos1'': v:val.pos1}')