From 5c839c482573263b4d3cb2bc4bc1719d49e14e22 Mon Sep 17 00:00:00 2001 From: w0rp Date: Sat, 19 Aug 2017 14:28:51 +0100 Subject: [PATCH] #653 Collect items for quickfix from all buffers, and de-duplicate them. Set filename items in quickfix and loclist. --- autoload/ale/list.vim | 64 +++++++++++++++++-- autoload/ale/util.vim | 10 +++ test/test_list_opening.vader | 25 ++++++-- test/test_list_titles.vader | 6 ++ test/test_loclist_sorting.vader | 56 ++++++++-------- test/test_quickfix_deduplication.vader | 50 +++++++++++++++ ..._setting_loclist_from_another_buffer.vader | 11 +++- 7 files changed, 180 insertions(+), 42 deletions(-) create mode 100644 test/test_quickfix_deduplication.vader diff --git a/autoload/ale/list.vim b/autoload/ale/list.vim index e01f9e6..6fe17d3 100644 --- a/autoload/ale/list.vim +++ b/autoload/ale/list.vim @@ -20,14 +20,67 @@ function! s:ShouldOpen(buffer) abort return l:val is 1 || (l:val is# 'on_save' && l:saved) endfunction +" A comparison function for de-duplicating loclist items for quickfix. +function! ale#list#TextLocItemCompare(left, right) abort + let l:cmp_val = ale#util#LocItemCompare(a:left, a:right) + + if l:cmp_val + return l:cmp_val + endif + + if a:left.text < a:right.text + return -1 + endif + + if a:left.text > a:right.text + return 1 + endif + + return 0 +endfunction + +function! ale#list#GetCombinedList() abort + let l:list = [] + + for l:info in values(g:ale_buffer_info) + call extend(l:list, l:info.loclist) + endfor + + call sort(l:list, function('ale#list#TextLocItemCompare')) + call uniq(l:list, function('ale#list#TextLocItemCompare')) + + return l:list +endfunction + +function! s:FixList(list) abort + let l:new_list = [] + + for l:item in a:list + if l:item.bufnr == -1 + " If the buffer number is invalid, remove it. + let l:fixed_item = copy(l:item) + call remove(l:fixed_item, 'bufnr') + else + " Don't copy the Dictionary if we do not need to. + let l:fixed_item = l:item + endif + + call add(l:new_list, l:fixed_item) + endfor + + return l:new_list +endfunction + function! ale#list#SetLists(buffer, loclist) abort let l:title = expand('#' . a:buffer . ':p') if g:ale_set_quickfix + let l:quickfix_list = ale#list#GetCombinedList() + if has('nvim') - call setqflist(a:loclist, ' ', l:title) + call setqflist(s:FixList(l:quickfix_list), ' ', l:title) else - call setqflist(a:loclist) + call setqflist(s:FixList(l:quickfix_list)) call setqflist([], 'r', {'title': l:title}) endif elseif g:ale_set_loclist @@ -37,9 +90,9 @@ function! ale#list#SetLists(buffer, loclist) abort let l:win_id = exists('*bufwinid') ? bufwinid(str2nr(a:buffer)) : 0 if has('nvim') - call setloclist(l:win_id, a:loclist, ' ', l:title) + call setloclist(l:win_id, s:FixList(a:loclist), ' ', l:title) else - call setloclist(l:win_id, a:loclist) + call setloclist(l:win_id, s:FixList(a:loclist)) call setloclist(l:win_id, [], 'r', {'title': l:title}) endif endif @@ -47,6 +100,9 @@ function! ale#list#SetLists(buffer, loclist) abort let l:keep_open = ale#Var(a:buffer, 'keep_list_window_open') " Open a window to show the problems if we need to. + " + " We'll check if the current buffer's List is not empty here, so the + " window will only be opened if the current buffer has problems. if s:ShouldOpen(a:buffer) && (l:keep_open || !empty(a:loclist)) let l:winnr = winnr() let l:mode = mode() diff --git a/autoload/ale/util.vim b/autoload/ale/util.vim index 8eda8a9..969a19d 100644 --- a/autoload/ale/util.vim +++ b/autoload/ale/util.vim @@ -30,6 +30,16 @@ function! ale#util#LocItemCompare(left, right) abort return 1 endif + if a:left.bufnr == -1 + if a:left.filename < a:right.filename + return -1 + endif + + if a:left.filename > a:right.filename + return 1 + endif + endif + if a:left.lnum < a:right.lnum return -1 endif diff --git a/test/test_list_opening.vader b/test/test_list_opening.vader index 024e55a..7d386d8 100644 --- a/test/test_list_opening.vader +++ b/test/test_list_opening.vader @@ -5,6 +5,7 @@ Before: Save g:ale_open_list Save g:ale_keep_list_window_open Save g:ale_list_window_size + Save g:ale_buffer_info let g:ale_set_loclist = 1 let g:ale_set_quickfix = 0 @@ -13,11 +14,12 @@ Before: let g:ale_list_window_size = 10 let g:loclist = [ - \ {'lnum': 5, 'col': 5}, - \ {'lnum': 5, 'col': 4}, - \ {'lnum': 2, 'col': 10}, - \ {'lnum': 3, 'col': 2}, + \ {'bufnr': bufnr(''), 'lnum': 5, 'col': 5, 'text': 'x'}, + \ {'bufnr': bufnr(''), 'lnum': 5, 'col': 4, 'text': 'x'}, + \ {'bufnr': bufnr(''), 'lnum': 2, 'col': 10, 'text': 'x'}, + \ {'bufnr': bufnr(''), 'lnum': 3, 'col': 2, 'text': 'x'}, \] + let g:ale_buffer_info = {bufnr(''): {'loclist': g:loclist}} function GetQuickfixHeight() abort for l:win in range(1, winnr('$')) @@ -121,6 +123,10 @@ Execute(The quickfix window should open for the quickfix list): let g:ale_set_quickfix = 1 let g:ale_open_list = 1 + let g:ale_buffer_info[bufnr('') + 1] = { + \ 'loclist': [{'bufnr': -1, 'filename': '/foo/bar', 'lnum': 5, 'col': 5, 'text': 'x'}], + \} + " It should not open for an empty list. call ale#list#SetLists(bufnr('%'), []) call ale#list#CloseWindowIfNeeded(bufnr('')) @@ -131,10 +137,17 @@ Execute(The quickfix window should open for the quickfix list): call ale#list#CloseWindowIfNeeded(bufnr('')) Assert ale#list#IsQuickfixOpen(), 'The quickfix window was closed when the list was not empty' - " Clear the list and it should close again. + " Clear this List. The window should stay open, as there are other items. + let g:ale_buffer_info[bufnr('')].loclist = [] call ale#list#SetLists(bufnr('%'), []) call ale#list#CloseWindowIfNeeded(bufnr('')) - Assert !ale#list#IsQuickfixOpen(), 'The quickfix window was not closed when the list was empty' + Assert ale#list#IsQuickfixOpen(), 'The quickfix window closed even though there are items in another buffer' + + " Clear the other List now. Now the window should close. + call remove(g:ale_buffer_info, bufnr('') + 1) + call ale#list#SetLists(bufnr('%'), []) + call ale#list#CloseWindowIfNeeded(bufnr('')) + Assert !ale#list#IsQuickfixOpen(), 'The quickfix window was not closed' Execute(The quickfix window should stay open for the quickfix list): let g:ale_set_quickfix = 1 diff --git a/test/test_list_titles.vader b/test/test_list_titles.vader index fe28629..74cb4bc 100644 --- a/test/test_list_titles.vader +++ b/test/test_list_titles.vader @@ -1,7 +1,9 @@ Before: Save g:ale_set_loclist Save g:ale_set_quickfix + Save g:ale_buffer_info + let g:ale_buffer_info = {} let g:ale_set_loclist = 0 let g:ale_set_quickfix = 0 @@ -43,6 +45,10 @@ Execute(The quickfix titles should be set appropriately): let g:ale_set_quickfix = 1 + let g:ale_buffer_info[bufnr('')] = { + \ 'loclist': [{'bufnr': bufnr(''), 'lnum': 5, 'col': 5, 'text': 'x', 'type': 'E'}], + \} + call ale#list#SetLists(bufnr(''), [ \ {'bufnr': bufnr(''), 'lnum': 5, 'col': 5, 'text': 'x', 'type': 'E'}, \]) diff --git a/test/test_loclist_sorting.vader b/test/test_loclist_sorting.vader index c095bdd..157b2a2 100644 --- a/test/test_loclist_sorting.vader +++ b/test/test_loclist_sorting.vader @@ -1,31 +1,27 @@ -Before: - let g:loclist = [ - \ {'bufnr': 3, 'lnum': 1, 'col': 1}, - \ {'bufnr': 1, 'lnum': 5, 'col': 5}, - \ {'bufnr': 2, 'lnum': 5, 'col': 5}, - \ {'bufnr': 1, 'lnum': 5, 'col': 4}, - \ {'bufnr': 1, 'lnum': 2, 'col': 10}, - \ {'bufnr': 2, 'lnum': 1, 'col': 5}, - \ {'bufnr': 1, 'lnum': 3, 'col': 2}, - \ {'bufnr': 2, 'lnum': 1, 'col': 2}, - \ {'bufnr': -1, 'lnum': 3, 'col': 2}, - \] - -Execute (Sort loclist with comparison function): - call sort(g:loclist, 'ale#util#LocItemCompare') - -Then (loclist item should be sorted): +Execute(loclist item should be sorted): AssertEqual [ - \ {'bufnr': -1, 'lnum': 3, 'col': 2}, - \ {'bufnr': 1, 'lnum': 2, 'col': 10}, - \ {'bufnr': 1, 'lnum': 3, 'col': 2}, - \ {'bufnr': 1, 'lnum': 5, 'col': 4}, - \ {'bufnr': 1, 'lnum': 5, 'col': 5}, - \ {'bufnr': 2, 'lnum': 1, 'col': 2}, - \ {'bufnr': 2, 'lnum': 1, 'col': 5}, - \ {'bufnr': 2, 'lnum': 5, 'col': 5}, - \ {'bufnr': 3, 'lnum': 1, 'col': 1}, - \], g:loclist - -After: - unlet g:loclist + \ {'bufnr': -1, 'filename': 'b', 'lnum': 4, 'col': 2}, + \ {'bufnr': -1, 'filename': 'b', 'lnum': 5, 'col': 2}, + \ {'bufnr': -1, 'filename': 'c', 'lnum': 3, 'col': 2}, + \ {'bufnr': 1, 'lnum': 2, 'col': 10}, + \ {'bufnr': 1, 'lnum': 3, 'col': 2}, + \ {'bufnr': 1, 'lnum': 5, 'col': 4}, + \ {'bufnr': 1, 'lnum': 5, 'col': 5}, + \ {'bufnr': 2, 'lnum': 1, 'col': 2}, + \ {'bufnr': 2, 'lnum': 1, 'col': 5}, + \ {'bufnr': 2, 'lnum': 5, 'col': 5}, + \ {'bufnr': 3, 'lnum': 1, 'col': 1}, + \ ], + \ sort([ + \ {'bufnr': 3, 'lnum': 1, 'col': 1}, + \ {'bufnr': -1, 'filename': 'b', 'lnum': 5, 'col': 2}, + \ {'bufnr': 1, 'lnum': 5, 'col': 5}, + \ {'bufnr': 2, 'lnum': 5, 'col': 5}, + \ {'bufnr': -1, 'filename': 'b', 'lnum': 4, 'col': 2}, + \ {'bufnr': 1, 'lnum': 5, 'col': 4}, + \ {'bufnr': 1, 'lnum': 2, 'col': 10}, + \ {'bufnr': 2, 'lnum': 1, 'col': 5}, + \ {'bufnr': 1, 'lnum': 3, 'col': 2}, + \ {'bufnr': 2, 'lnum': 1, 'col': 2}, + \ {'bufnr': -1, 'filename': 'c', 'lnum': 3, 'col': 2}, + \], 'ale#util#LocItemCompare') diff --git a/test/test_quickfix_deduplication.vader b/test/test_quickfix_deduplication.vader new file mode 100644 index 0000000..0dff3f2 --- /dev/null +++ b/test/test_quickfix_deduplication.vader @@ -0,0 +1,50 @@ +Before: + Save g:ale_buffer_info + +After: + Restore + +Execute: + " Results from multiple buffers should be gathered together. + " Equal problems should be de-duplicated. + let g:ale_buffer_info = { + \ '1': {'loclist': [ + \ {'bufnr': 2, 'lnum': 1, 'col': 2, 'text': 'foo'}, + \ {'bufnr': 2, 'lnum': 1, 'col': 5, 'text': 'bar'}, + \ {'bufnr': -1, 'filename': 'c', 'lnum': 3, 'col': 2, 'text': 'x'}, + \ {'bufnr': 1, 'lnum': 5, 'col': 4, 'text': 'x'}, + \ {'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'foo'}, + \ {'bufnr': 1, 'lnum': 2, 'col': 10, 'text': 'x'}, + \ {'bufnr': 1, 'lnum': 3, 'col': 2, 'text': 'x'}, + \ {'bufnr': 1, 'lnum': 5, 'col': 5, 'text': 'x'}, + \ {'bufnr': -1, 'filename': 'b', 'lnum': 4, 'col': 2, 'text': 'x'}, + \ {'bufnr': -1, 'filename': 'b', 'lnum': 5, 'col': 2, 'text': 'x'}, + \ {'bufnr': 3, 'lnum': 1, 'col': 1, 'text': 'foo'}, + \ ]}, + \ '2': {'loclist': [ + \ {'bufnr': 1, 'lnum': 2, 'col': 10, 'text': 'x'}, + \ {'bufnr': 1, 'lnum': 5, 'col': 5, 'text': 'x'}, + \ {'bufnr': 2, 'lnum': 1, 'col': 2, 'text': 'foo'}, + \ {'bufnr': 1, 'lnum': 3, 'col': 2, 'text': 'x'}, + \ {'bufnr': 1, 'lnum': 5, 'col': 4, 'text': 'x'}, + \ {'bufnr': 2, 'lnum': 1, 'col': 5, 'text': 'bar'}, + \ {'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'another error'}, + \ ]}, + \} + + AssertEqual + \ [ + \ {'bufnr': -1, 'filename': 'b', 'lnum': 4, 'col': 2, 'text': 'x'}, + \ {'bufnr': -1, 'filename': 'b', 'lnum': 5, 'col': 2, 'text': 'x'}, + \ {'bufnr': -1, 'filename': 'c', 'lnum': 3, 'col': 2, 'text': 'x'}, + \ {'bufnr': 1, 'lnum': 2, 'col': 10, 'text': 'x'}, + \ {'bufnr': 1, 'lnum': 3, 'col': 2, 'text': 'x'}, + \ {'bufnr': 1, 'lnum': 5, 'col': 4, 'text': 'x'}, + \ {'bufnr': 1, 'lnum': 5, 'col': 5, 'text': 'x'}, + \ {'bufnr': 2, 'lnum': 1, 'col': 2, 'text': 'foo'}, + \ {'bufnr': 2, 'lnum': 1, 'col': 5, 'text': 'bar'}, + \ {'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'another error'}, + \ {'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'foo'}, + \ {'bufnr': 3, 'lnum': 1, 'col': 1, 'text': 'foo'}, + \ ], + \ ale#list#GetCombinedList() diff --git a/test/test_setting_loclist_from_another_buffer.vader b/test/test_setting_loclist_from_another_buffer.vader index ae53de1..10a44cc 100644 --- a/test/test_setting_loclist_from_another_buffer.vader +++ b/test/test_setting_loclist_from_another_buffer.vader @@ -1,7 +1,11 @@ Before: Save g:ale_buffer_info - let g:ale_buffer_info = {} + let g:ale_buffer_info = { + \ bufnr(''): { + \ 'loclist': [{'bufnr': bufnr(''), 'lnum': 4, 'col': 1, 'text': 'foo'}] + \ }, + \} let g:original_buffer = bufnr('%') noautocmd new @@ -12,7 +16,10 @@ After: unlet! g:original_buffer Execute(Errors should be set in the loclist for the original buffer, not the new one): - call ale#list#SetLists(g:original_buffer, [{'lnum': 4, 'text': 'foo'}]) + call ale#list#SetLists( + \ g:original_buffer, + \ g:ale_buffer_info[(g:original_buffer)].loclist, + \ ) AssertEqual [], getloclist(0) AssertEqual 1, len(getloclist(bufwinid(g:original_buffer)))