From f064ba48f5893f25c1af9459b309a919a392db7c Mon Sep 17 00:00:00 2001 From: w0rp Date: Tue, 10 Apr 2018 21:05:22 +0100 Subject: [PATCH] Close #1494 - Prefer displaying higher severity problems for cursor messages, balloons, and highlights --- autoload/ale/sign.vim | 36 ++++------------ autoload/ale/util.vim | 54 +++++++++++++++++++++++ test/test_balloon_messages.vader | 9 ++++ test/test_cursor_warnings.vader | 12 ++++++ test/test_loclist_binary_search.vader | 17 ++++++++ test/test_loclist_jumping.vader | 20 ++++----- test/test_loclist_sorting.vader | 16 +++++++ test/test_quickfix_deduplication.vader | 60 +++++++++++++------------- 8 files changed, 156 insertions(+), 68 deletions(-) diff --git a/autoload/ale/sign.vim b/autoload/ale/sign.vim index 1c439bc8..d1139c03 100644 --- a/autoload/ale/sign.vim +++ b/autoload/ale/sign.vim @@ -60,55 +60,35 @@ execute 'sign define ALEInfoSign text=' . g:ale_sign_info \ . ' texthl=ALEInfoSign linehl=ALEInfoLine' sign define ALEDummySign -let s:error_priority = 1 -let s:warning_priority = 2 -let s:info_priority = 3 -let s:style_error_priority = 4 -let s:style_warning_priority = 5 - function! ale#sign#GetSignName(sublist) abort - let l:priority = s:style_warning_priority + let l:priority = g:ale#util#style_warning_priority " Determine the highest priority item for the line. for l:item in a:sublist - if l:item.type is# 'I' - let l:item_priority = s:info_priority - elseif l:item.type is# 'W' - if get(l:item, 'sub_type', '') is# 'style' - let l:item_priority = s:style_warning_priority - else - let l:item_priority = s:warning_priority - endif - else - if get(l:item, 'sub_type', '') is# 'style' - let l:item_priority = s:style_error_priority - else - let l:item_priority = s:error_priority - endif - endif + let l:item_priority = ale#util#GetItemPriority(l:item) - if l:item_priority < l:priority + if l:item_priority > l:priority let l:priority = l:item_priority endif endfor - if l:priority is# s:error_priority + if l:priority is# g:ale#util#error_priority return 'ALEErrorSign' endif - if l:priority is# s:warning_priority + if l:priority is# g:ale#util#warning_priority return 'ALEWarningSign' endif - if l:priority is# s:style_error_priority + if l:priority is# g:ale#util#style_error_priority return 'ALEStyleErrorSign' endif - if l:priority is# s:style_warning_priority + if l:priority is# g:ale#util#style_warning_priority return 'ALEStyleWarningSign' endif - if l:priority is# s:info_priority + if l:priority is# g:ale#util#info_priority return 'ALEInfoSign' endif diff --git a/autoload/ale/util.vim b/autoload/ale/util.vim index e0b31e2c..d160b34c 100644 --- a/autoload/ale/util.vim +++ b/autoload/ale/util.vim @@ -33,6 +33,32 @@ function! ale#util#GetFunction(string_or_ref) abort return a:string_or_ref endfunction +let g:ale#util#error_priority = 5 +let g:ale#util#warning_priority = 4 +let g:ale#util#info_priority = 3 +let g:ale#util#style_error_priority = 2 +let g:ale#util#style_warning_priority = 1 + +function! ale#util#GetItemPriority(item) abort + if a:item.type is# 'I' + return g:ale#util#info_priority + endif + + if a:item.type is# 'W' + if get(a:item, 'sub_type', '') is# 'style' + return g:ale#util#style_warning_priority + endif + + return g:ale#util#warning_priority + endif + + if get(a:item, 'sub_type', '') is# 'style' + return g:ale#util#style_error_priority + endif + + return g:ale#util#error_priority +endfunction + " Compare two loclist items for ALE, sorted by their buffers, filenames, and " line numbers and column numbers. function! ale#util#LocItemCompare(left, right) abort @@ -70,6 +96,23 @@ function! ale#util#LocItemCompare(left, right) abort return 1 endif + " When either of the items lacks a problem type, then the two items should + " be considered equal. This is important for loclist jumping. + if !has_key(a:left, 'type') || !has_key(a:right, 'type') + return 0 + endif + + let l:left_priority = ale#util#GetItemPriority(a:left) + let l:right_priority = ale#util#GetItemPriority(a:right) + + if l:left_priority < l:right_priority + return -1 + endif + + if l:left_priority > l:right_priority + return 1 + endif + return 0 endfunction @@ -139,6 +182,17 @@ function! ale#util#BinarySearch(loclist, buffer, line, column) abort let l:index += 1 endwhile + " Scan forwards to find the last item on the column for the item + " we found, which will have the most serious problem. + let l:item_column = a:loclist[l:index].col + + while l:index < l:max + \&& a:loclist[l:index + 1].bufnr == a:buffer + \&& a:loclist[l:index + 1].lnum == a:line + \&& a:loclist[l:index + 1].col == l:item_column + let l:index += 1 + endwhile + return l:index endif endwhile diff --git a/test/test_balloon_messages.vader b/test/test_balloon_messages.vader index 8f4415ae..d0724c21 100644 --- a/test/test_balloon_messages.vader +++ b/test/test_balloon_messages.vader @@ -7,10 +7,19 @@ Before: let g:ale_buffer_info[bufnr('')] = {'loclist': [ \ { + \ 'bufnr': bufnr('%'), + \ 'lnum': 1, + \ 'col': 10, + \ 'linter_name': 'eslint', + \ 'type': 'W', + \ 'text': 'Ignore me.', + \ }, + \ { \ 'bufnr': bufnr(''), \ 'lnum': 1, \ 'col': 10, \ 'text': 'Missing semicolon. (semi)', + \ 'type': 'E', \ }, \ { \ 'bufnr': bufnr(''), diff --git a/test/test_cursor_warnings.vader b/test/test_cursor_warnings.vader index 19592217..24652909 100644 --- a/test/test_cursor_warnings.vader +++ b/test/test_cursor_warnings.vader @@ -2,6 +2,7 @@ Before: Save g:ale_echo_msg_format Save g:ale_echo_cursor + " We should prefer the error message at column 10 instead of the warning. let g:ale_buffer_info = { \ bufnr('%'): { \ 'loclist': [ @@ -12,6 +13,17 @@ Before: \ 'vcol': 0, \ 'linter_name': 'eslint', \ 'nr': -1, + \ 'type': 'W', + \ 'code': 'semi', + \ 'text': 'Ignore me.', + \ }, + \ { + \ 'lnum': 1, + \ 'col': 10, + \ 'bufnr': bufnr('%'), + \ 'vcol': 0, + \ 'linter_name': 'eslint', + \ 'nr': -1, \ 'type': 'E', \ 'code': 'semi', \ 'text': 'Missing semicolon.', diff --git a/test/test_loclist_binary_search.vader b/test/test_loclist_binary_search.vader index 5558191c..219fb314 100644 --- a/test/test_loclist_binary_search.vader +++ b/test/test_loclist_binary_search.vader @@ -47,3 +47,20 @@ Execute(Searches should work with just one item): let g:loclist = [{'bufnr': 1, 'lnum': 3, 'col': 10}] AssertEqual 0, ale#util#BinarySearch(g:loclist, 1, 3, 2) + +Execute(Searches should return the last item on a single column): + let g:loclist = [ + \ {'bufnr': 1, 'lnum': 1, 'col': 10, 'type': 'W'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 10, 'type': 'E'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 11, 'type': 'W'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 11, 'type': 'E'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 20, 'type': 'W'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 20, 'type': 'E'}, + \] + + " We should return the index for the last item at some column to the right. + AssertEqual 1, ale#util#BinarySearch(g:loclist, 1, 1, 1) + " We should return the index for the last item at the column we are on. + AssertEqual 3, ale#util#BinarySearch(g:loclist, 1, 1, 11) + " We should prefer items to the left of the cursor, over the right. + AssertEqual 3, ale#util#BinarySearch(g:loclist, 1, 1, 19) diff --git a/test/test_loclist_jumping.vader b/test/test_loclist_jumping.vader index 5e18499e..da9a1f57 100644 --- a/test/test_loclist_jumping.vader +++ b/test/test_loclist_jumping.vader @@ -2,15 +2,15 @@ Before: let g:ale_buffer_info = { \ bufnr(''): { \ 'loclist': [ - \ {'bufnr': bufnr('') - 1, 'lnum': 3, 'col': 2}, - \ {'bufnr': bufnr(''), 'lnum': 1, 'col': 2}, - \ {'bufnr': bufnr(''), 'lnum': 1, 'col': 3}, - \ {'bufnr': bufnr(''), 'lnum': 2, 'col': 1}, - \ {'bufnr': bufnr(''), 'lnum': 2, 'col': 2}, - \ {'bufnr': bufnr(''), 'lnum': 2, 'col': 3}, - \ {'bufnr': bufnr(''), 'lnum': 2, 'col': 6}, - \ {'bufnr': bufnr(''), 'lnum': 2, 'col': 700}, - \ {'bufnr': bufnr('') + 1, 'lnum': 3, 'col': 2}, + \ {'type': 'E', 'bufnr': bufnr('') - 1, 'lnum': 3, 'col': 2}, + \ {'type': 'E', 'bufnr': bufnr(''), 'lnum': 1, 'col': 2}, + \ {'type': 'E', 'bufnr': bufnr(''), 'lnum': 1, 'col': 3}, + \ {'type': 'E', 'bufnr': bufnr(''), 'lnum': 2, 'col': 1}, + \ {'type': 'E', 'bufnr': bufnr(''), 'lnum': 2, 'col': 2}, + \ {'type': 'E', 'bufnr': bufnr(''), 'lnum': 2, 'col': 3}, + \ {'type': 'E', 'bufnr': bufnr(''), 'lnum': 2, 'col': 6}, + \ {'type': 'E', 'bufnr': bufnr(''), 'lnum': 2, 'col': 700}, + \ {'type': 'E', 'bufnr': bufnr('') + 1, 'lnum': 3, 'col': 2}, \ ], \ }, \} @@ -81,7 +81,7 @@ Execute(We should be able to jump when the error line is blank): " Add a blank line at the end. call setline(1, getline('.', '$') + ['']) " Add a problem on the blank line. - call add(g:ale_buffer_info[bufnr('%')].loclist, {'bufnr': bufnr(''), 'lnum': 3, 'col': 1}) + call add(g:ale_buffer_info[bufnr('%')].loclist, {'type': 'E', 'bufnr': bufnr(''), 'lnum': 3, 'col': 1}) AssertEqual 0, len(getline(3)) AssertEqual [2, 8], TestJump('before', 0, [3, 1]) diff --git a/test/test_loclist_sorting.vader b/test/test_loclist_sorting.vader index 157b2a25..376e743a 100644 --- a/test/test_loclist_sorting.vader +++ b/test/test_loclist_sorting.vader @@ -25,3 +25,19 @@ Execute(loclist item should be sorted): \ {'bufnr': 2, 'lnum': 1, 'col': 2}, \ {'bufnr': -1, 'filename': 'c', 'lnum': 3, 'col': 2}, \], 'ale#util#LocItemCompare') + +Execute(Items should be sorted in by their problem priority when they lie on the same column): + AssertEqual [ + \ {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'W', 'sub_type': 'style'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'E', 'sub_type': 'style'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'I'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'W'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'E'}, + \ ], + \ sort([ + \ {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'E'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'I'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'W'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'E', 'sub_type': 'style'}, + \ {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'W', 'sub_type': 'style'}, + \], 'ale#util#LocItemCompare') diff --git a/test/test_quickfix_deduplication.vader b/test/test_quickfix_deduplication.vader index 0dff3f2e..9cb8b931 100644 --- a/test/test_quickfix_deduplication.vader +++ b/test/test_quickfix_deduplication.vader @@ -9,42 +9,42 @@ Execute: " 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'}, + \ {'type': 'E', 'bufnr': 2, 'lnum': 1, 'col': 2, 'text': 'foo'}, + \ {'type': 'E', 'bufnr': 2, 'lnum': 1, 'col': 5, 'text': 'bar'}, + \ {'type': 'E', 'bufnr': -1, 'filename': 'c', 'lnum': 3, 'col': 2, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 1, 'lnum': 5, 'col': 4, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'foo'}, + \ {'type': 'E', 'bufnr': 1, 'lnum': 2, 'col': 10, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 1, 'lnum': 3, 'col': 2, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 1, 'lnum': 5, 'col': 5, 'text': 'x'}, + \ {'type': 'E', 'bufnr': -1, 'filename': 'b', 'lnum': 4, 'col': 2, 'text': 'x'}, + \ {'type': 'E', 'bufnr': -1, 'filename': 'b', 'lnum': 5, 'col': 2, 'text': 'x'}, + \ {'type': 'E', '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'}, + \ {'type': 'E', 'bufnr': 1, 'lnum': 2, 'col': 10, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 1, 'lnum': 5, 'col': 5, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 2, 'lnum': 1, 'col': 2, 'text': 'foo'}, + \ {'type': 'E', 'bufnr': 1, 'lnum': 3, 'col': 2, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 1, 'lnum': 5, 'col': 4, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 2, 'lnum': 1, 'col': 5, 'text': 'bar'}, + \ {'type': 'E', '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'}, + \ {'type': 'E', 'bufnr': -1, 'filename': 'b', 'lnum': 4, 'col': 2, 'text': 'x'}, + \ {'type': 'E', 'bufnr': -1, 'filename': 'b', 'lnum': 5, 'col': 2, 'text': 'x'}, + \ {'type': 'E', 'bufnr': -1, 'filename': 'c', 'lnum': 3, 'col': 2, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 1, 'lnum': 2, 'col': 10, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 1, 'lnum': 3, 'col': 2, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 1, 'lnum': 5, 'col': 4, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 1, 'lnum': 5, 'col': 5, 'text': 'x'}, + \ {'type': 'E', 'bufnr': 2, 'lnum': 1, 'col': 2, 'text': 'foo'}, + \ {'type': 'E', 'bufnr': 2, 'lnum': 1, 'col': 5, 'text': 'bar'}, + \ {'type': 'E', 'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'another error'}, + \ {'type': 'E', 'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'foo'}, + \ {'type': 'E', 'bufnr': 3, 'lnum': 1, 'col': 1, 'text': 'foo'}, \ ], \ ale#list#GetCombinedList()