From 1a524ca63e51092ab10febea40a6f018b6e85173 Mon Sep 17 00:00:00 2001 From: w0rp Date: Tue, 22 Aug 2017 21:19:36 +0100 Subject: [PATCH] #653 - Always set loclist or quickfix in a timer callback, which prevents errors E924, E925, and E926 --- autoload/ale/engine.vim | 2 +- autoload/ale/events.vim | 52 ---------- autoload/ale/list.vim | 32 ++++++- autoload/ale/util.vim | 30 ++++++ ftplugin/qf.vim | 4 - plugin/ale.vim | 1 - test/test_ale_init_au_groups.vader | 15 --- ...t_list_modification_error_cancelling.vader | 96 ------------------- test/test_set_list_timers.vader | 38 ++++++++ test/test_statusline.vader | 10 +- test/vimrc | 3 + 11 files changed, 107 insertions(+), 176 deletions(-) delete mode 100644 ftplugin/qf.vim delete mode 100644 test/test_list_modification_error_cancelling.vader create mode 100644 test/test_set_list_timers.vader diff --git a/autoload/ale/engine.vim b/autoload/ale/engine.vim index 40930038..2875d4ad 100644 --- a/autoload/ale/engine.vim +++ b/autoload/ale/engine.vim @@ -708,7 +708,7 @@ function! s:RemoveProblemsForDisabledLinters(buffer, linters) abort call filter( \ get(g:ale_buffer_info[a:buffer], 'loclist', []), - \ 'get(l:name_map, v:val.linter_name)', + \ 'get(l:name_map, get(v:val, ''linter_name''))', \) endfunction diff --git a/autoload/ale/events.vim b/autoload/ale/events.vim index 2ee7f86a..a3b74677 100644 --- a/autoload/ale/events.vim +++ b/autoload/ale/events.vim @@ -45,55 +45,3 @@ function! ale#events#FileChangedEvent(buffer) abort call s:LintOnEnter(a:buffer) endif endfunction - -" When changing quickfix or a loclist window while the window is open -" from autocmd events and while navigating from one buffer to another, Vim -" will complain saying that the window has closed or that the lists have -" changed. -" -" This timer callback just accepts those errors when they appear. -function! s:HitReturn(...) abort - if ale#util#Mode() is# 'r' - redir => l:output - silent mess - redir end - - if get(split(l:output, "\n"), -1, '') =~# '^E92[456]' - call ale#util#FeedKeys("\", 'n') - - " If we hit one of the errors and cleared it, then Vim didn't - " move to the position we wanted. Change the position to the one - " the user selected. - if exists('g:ale_list_window_selection') - let l:pos = getcurpos() - let [l:pos[1], l:pos[2]] = g:ale_list_window_selection - call setpos('.', l:pos) - endif - endif - endif - - " Always clear the last selection when trying to cancel the errors above - " here. This prevents us from remembering invalid positions. - unlet! g:ale_list_window_selection -endfunction - -function! ale#events#BufWinLeave() abort - call timer_start(0, function('s:HitReturn')) -endfunction - -" Grab the position for a problem from the loclist or quickfix windows -" when moving through selections. This selection will be remembered and -" set as the position when jumping to an error in another file. -function! ale#events#ParseLoclistWindowItemPosition() abort - " Parses lines like - " test.txt|72 col 5 error| ... - " test.txt|72| ... - let l:match = matchlist(getline('.'), '\v^[^|]+\|(\d+)( [^ ]+ )?(\d*)') - - if !empty(l:match) - let g:ale_list_window_selection = [ - \ l:match[1] + 0, - \ max([l:match[3] + 0, 1]), - \] - endif -endfunction diff --git a/autoload/ale/list.vim b/autoload/ale/list.vim index 25920ce6..7b2bf2cb 100644 --- a/autoload/ale/list.vim +++ b/autoload/ale/list.vim @@ -1,6 +1,10 @@ " Author: Bjorn Neergaard , modified by Yann fery " Description: Manages the loclist and quickfix lists +if !exists('s:timer_args') + let s:timer_args = {} +endif + " Return 1 if there is a buffer with buftype == 'quickfix' in bufffer list function! ale#list#IsQuickfixOpen() abort for l:buf in range(1, bufnr('$')) @@ -52,7 +56,7 @@ function! s:FixList(list) abort return l:new_list endfunction -function! ale#list#SetLists(buffer, loclist) abort +function! s:SetListsImpl(timer_id, buffer, loclist) abort let l:title = expand('#' . a:buffer . ':p') if g:ale_set_quickfix @@ -115,7 +119,19 @@ function! ale#list#SetLists(buffer, loclist) abort endif endfunction -function! ale#list#CloseWindowIfNeeded(buffer) abort +function! ale#list#SetLists(buffer, loclist) abort + if get(g:, 'ale_set_lists_synchronously') == 1 + call s:SetListsImpl(-1, a:buffer, a:loclist) + else + call ale#util#StartPartialTimer( + \ 0, + \ function('s:SetListsImpl'), + \ [a:buffer, a:loclist], + \) + endif +endfunction + +function! s:CloseWindowIfNeededImpl(timer_id, buffer) abort if ale#Var(a:buffer, 'keep_list_window_open') || !s:ShouldOpen(a:buffer) return endif @@ -134,3 +150,15 @@ function! ale#list#CloseWindowIfNeeded(buffer) abort catch /E444/ endtry endfunction + +function! ale#list#CloseWindowIfNeeded(buffer) abort + if get(g:, 'ale_set_lists_synchronously') == 1 + call s:CloseWindowIfNeededImpl(-1, a:buffer) + else + call ale#util#StartPartialTimer( + \ 0, + \ function('s:CloseWindowIfNeededImpl'), + \ [a:buffer], + \) + endif +endfunction diff --git a/autoload/ale/util.vim b/autoload/ale/util.vim index c46579b0..cf8d5bec 100644 --- a/autoload/ale/util.vim +++ b/autoload/ale/util.vim @@ -267,3 +267,33 @@ function! ale#util#Writefile(buffer, lines, filename) abort call writefile(l:corrected_lines, a:filename) " no-custom-checks endfunction + +if !exists('s:patial_timers') + let s:partial_timers = {} +endif + +function! s:ApplyPartialTimer(timer_id) abort + let [l:Callback, l:args] = remove(s:partial_timers, a:timer_id) + call call(l:Callback, [a:timer_id] + l:args) +endfunction + +" Given a delay, a callback, a List of arguments, start a timer with +" timer_start() and call the callback provided with [timer_id] + args. +" +" The timer must not be stopped with timer_stop(). +" Use ale#util#StopPartialTimer() instead, which can stop any timer, and will +" clear any arguments saved for executing callbacks later. +function! ale#util#StartPartialTimer(delay, callback, args) abort + let l:timer_id = timer_start(a:delay, function('s:ApplyPartialTimer')) + let s:partial_timers[l:timer_id] = [a:callback, a:args] + + return l:timer_id +endfunction + +function! ale#util#StopPartialTimer(timer_id) abort + call timer_stop(a:timer_id) + + if has_key(s:partial_timers, a:timer_id) + call remove(s:partial_timers, a:timer_id) + endif +endfunction diff --git a/ftplugin/qf.vim b/ftplugin/qf.vim deleted file mode 100644 index 18e2c81a..00000000 --- a/ftplugin/qf.vim +++ /dev/null @@ -1,4 +0,0 @@ -augroup ALEQuickfixCursorMovedEvent - autocmd! * - autocmd CursorMoved call ale#events#ParseLoclistWindowItemPosition() -augroup END diff --git a/plugin/ale.vim b/plugin/ale.vim index d33f3c5a..a9ab88a1 100644 --- a/plugin/ale.vim +++ b/plugin/ale.vim @@ -226,7 +226,6 @@ function! ALEInitAuGroups() abort autocmd BufWinEnter,BufRead * call ale#Queue(0, 'lint_file', str2nr(expand(''))) " Track when the file is changed outside of Vim. autocmd FileChangedShellPost * call ale#events#FileChangedEvent(str2nr(expand(''))) - autocmd BufWinLeave * call ale#events#BufWinLeave() endif augroup END diff --git a/test/test_ale_init_au_groups.vader b/test/test_ale_init_au_groups.vader index 3606402f..2685f50b 100644 --- a/test/test_ale_init_au_groups.vader +++ b/test/test_ale_init_au_groups.vader @@ -60,11 +60,6 @@ After: call ALEInitAuGroups() - " Clean up the quickfix group. - augroup ALEQuickfixCursorMovedEvent - autocmd! * - augroup END - Execute (g:ale_lint_on_text_changed = 0 should bind no events): let g:ale_lint_on_text_changed = 0 @@ -139,7 +134,6 @@ Execute (g:ale_lint_on_enter = 1 should bind the required events): \ 'BufEnter * call ale#events#EnterEvent(str2nr(expand('''')))', \ 'BufReadPost * call ale#Queue(0, ''lint_file'', str2nr(expand('''')))', \ 'BufWinEnter * call ale#Queue(0, ''lint_file'', str2nr(expand('''')))', - \ 'BufWinLeave * call ale#events#BufWinLeave()', \ 'FileChangedShellPost * call ale#events#FileChangedEvent(str2nr(expand('''')))', \], CheckAutocmd('ALERunOnEnterGroup') @@ -222,12 +216,3 @@ Execute(Disabling completion should remove autocmd events correctly): AssertEqual [], CheckAutocmd('ALECompletionGroup') AssertEqual 0, g:ale_completion_enabled - -Execute(The cursor events should be set up for the quickfix list): - runtime! ftplugin/qf.vim - - AssertEqual - \ [ - \ 'CursorMoved call ale#events#ParseLoclistWindowItemPosition()', - \ ], - \ CheckAutocmd('ALEQuickfixCursorMovedEvent') diff --git a/test/test_list_modification_error_cancelling.vader b/test/test_list_modification_error_cancelling.vader deleted file mode 100644 index e4aa3afa..00000000 --- a/test/test_list_modification_error_cancelling.vader +++ /dev/null @@ -1,96 +0,0 @@ -Before: - let b:fake_mode = 'r' - let b:feedkeys_calls = [] - - " Mock mode() and feedkeys() for the check - function! ale#util#Mode(...) abort - return b:fake_mode - endfunction - - function! ale#util#FeedKeys(...) abort - call add(b:feedkeys_calls, a:000) - endfunction - - function! CheckError(mode, message, expected_list) abort - let b:fake_mode = a:mode - - echom a:message - - call ale#events#BufWinLeave() - AssertEqual [], b:feedkeys_calls - - sleep 1ms - AssertEqual a:expected_list, b:feedkeys_calls - endfunction - -After: - unlet! b:fake_mode - unlet! b:feedkeys_calls - unlet! g:ale_list_window_selection - - delfunction CheckError - - runtime autoload/ale/util.vim - -Execute(The BufWinLeave event function should hide E924 errors): - " For some reason, this test fails the first time when running in NeoVim - " in Docker, so just execute this twice. - echom 'E924' - call ale#events#BufWinLeave() - sleep 1ms - let b:feedkeys_calls = [] - - call CheckError('r', 'E924', [["\", 'n']]) - -Execute(The BufWinLeave event function should hide E925 errors): - call CheckError('r', 'E925', [["\", 'n']]) - -Execute(The BufWinLeave event function should hide E926 errors): - call CheckError('r', 'E926', [["\", 'n']]) - -Execute(The BufWinLeave event function should ignore other errors): - call CheckError('r', 'E999', []) - -Execute(The BufWinLeave event function not send keys for other modes): - call CheckError('n', 'E924', []) - -Execute(The last window selection should always be cleared by the timer): - let g:ale_list_window_selection = [347, 2] - - " The message and mode shouldn't matter, we should still clear the variable. - echom 'xyz' - let b:fake_mode = 'n' - call ale#events#BufWinLeave() - sleep 1ms - - Assert !has_key(g:, 'ale_list_window_selection') - -Given qf(A quickfix list with some errors): - test.txt|23 col 9 warning| Some warning - test.txt|72 col 25 error| Some column error - test.txt|93 error| Some line error - -Execute(Line and column numbers should be parsed by the quickfix event function): - call setpos('.', [bufnr(''), 2, 1, 0]) - call ale#events#ParseLoclistWindowItemPosition() - AssertEqual [72, 25], g:ale_list_window_selection - -Execute(Just Line numbers should be parsed by the quickfix event function): - call setpos('.', [bufnr(''), 3, 1, 0]) - call ale#events#ParseLoclistWindowItemPosition() - AssertEqual [93, 1], g:ale_list_window_selection - -Given python(Some example Python file): - class FooBar: - def whatever(self): - self.do_something() - -Execute(We should jump to the window selection after cancelling the errors): - call setpos('.', [bufnr(''), 1, 1, 0]) - let g:ale_list_window_selection = [3, 9] - - echom 'E925' - call ale#events#BufWinLeave() - sleep 1ms - - AssertEqual [3, 9], getcurpos()[1:2] diff --git a/test/test_set_list_timers.vader b/test/test_set_list_timers.vader new file mode 100644 index 00000000..90aacb55 --- /dev/null +++ b/test/test_set_list_timers.vader @@ -0,0 +1,38 @@ +Before: + Save g:ale_set_lists_synchronously + Save g:ale_open_list + + let g:ale_set_lists_synchronously = 0 + +After: + Restore + + sleep 1ms + call setloclist(0, []) + lclose + +Execute(The SetLists function should work when run in a timer): + call ale#list#SetLists(bufnr(''), [ + \ {'bufnr': bufnr(''), 'lnum': 5, 'col': 5, 'text': 'x', 'type': 'E'}, + \]) + sleep 1ms + AssertEqual [{ + \ 'lnum': 5, + \ 'bufnr': bufnr(''), + \ 'col': 5, + \ 'text': 'x', + \ 'valid': 1, + \ 'vcol': 0, + \ 'nr': 0, + \ 'type': 'E', + \ 'pattern': '', + \}], getloclist(0) + +Execute(The CloseWindowIfNeeded function should work when run in a timer): + let g:ale_open_list = 1 + lopen + + call ale#list#CloseWindowIfNeeded(bufnr('')) + sleep 1ms + + Assert !ale#list#IsQuickfixOpen(), 'The window was not closed!' diff --git a/test/test_statusline.vader b/test/test_statusline.vader index c47df75a..7978a509 100644 --- a/test/test_statusline.vader +++ b/test/test_statusline.vader @@ -1,9 +1,11 @@ Before: - Save g:ale_statusline_format, g:ale_buffer_info + Save g:ale_statusline_format + Save g:ale_buffer_info + let g:ale_buffer_info = {} " A function for conveniently creating expected count objects. - function Counts(data) abort + function! Counts(data) abort let l:res = { \ '0': 0, \ '1': 0, @@ -28,6 +30,7 @@ Before: After: Restore + delfunction Counts Execute (Count should be 0 when data is empty): @@ -126,16 +129,13 @@ Execute (Status() should return the custom 'OK' string with the old format): AssertEqual 'OKIE', ale#statusline#Status() Execute(ale#statusline#Update shouldn't blow up when globals are undefined): - unlet! g:ale_buffer_info unlet! g:ale_statusline_format call ale#statusline#Update(1, []) Execute(ale#statusline#Count should return 0 counts when globals are undefined): - unlet! g:ale_buffer_info unlet! g:ale_statusline_format AssertEqual Counts({}), ale#statusline#Count(1) Execute(ale#statusline#Status should return 'OK' when globals are undefined): - unlet! g:ale_buffer_info unlet! g:ale_statusline_format AssertEqual 'OK', ale#statusline#Status() diff --git a/test/vimrc b/test/vimrc index 57af7e15..8dadb4f2 100644 --- a/test/vimrc +++ b/test/vimrc @@ -1,5 +1,8 @@ " vint: -ProhibitSetNoCompatible +" Make most tests just set lists synchronously when run in Docker. +let g:ale_set_lists_synchronously = 1 + " Load builtin plugins " We need this because run_vim.sh sets -i NONE set runtimepath=/home/vim,$VIM/vimfiles,$VIMRUNTIME,$VIM/vimfiles/after,/testplugin,/vader