From 7481facd7373dda4667c401695e6d8000ef52362 Mon Sep 17 00:00:00 2001 From: w0rp Date: Sun, 23 Oct 2016 22:41:00 +0100 Subject: [PATCH] #107 Stop jobs when buffers close --- autoload/ale.vim | 5 ++- autoload/ale/cleanup.vim | 15 +++----- autoload/ale/engine.vim | 60 +++++++++++++++++++++--------- autoload/ale/sign.vim | 8 ++-- autoload/ale/statusline.vim | 40 +++++++++++++------- plugin/ale.vim | 4 +- test/test_cleanup.vader | 23 +++--------- test/test_linting_sets_signs.vader | 40 ++++++++++++++++++++ test/test_statusline.vader | 52 +++++++++++++++----------- 9 files changed, 161 insertions(+), 86 deletions(-) create mode 100644 test/test_linting_sets_signs.vader diff --git a/autoload/ale.vim b/autoload/ale.vim index 44cd5825..3e86c8f9 100644 --- a/autoload/ale.vim +++ b/autoload/ale.vim @@ -27,8 +27,11 @@ function! ale#Lint(...) abort let l:buffer = bufnr('%') let l:linters = ale#linter#Get(&filetype) + " Initialise the buffer information if needed. + call ale#engine#InitBufferInfo(l:buffer) + " Set a variable telling us to clear the loclist later. - let g:ale_buffer_should_reset_map[l:buffer] = 1 + let g:ale_buffer_info[l:buffer].should_reset = 1 for l:linter in l:linters " Check if a given linter has a program which can be executed. diff --git a/autoload/ale/cleanup.vim b/autoload/ale/cleanup.vim index 45eccb98..40a8a359 100644 --- a/autoload/ale/cleanup.vim +++ b/autoload/ale/cleanup.vim @@ -2,19 +2,16 @@ " Description: Utility functions related to cleaning state. function! ale#cleanup#Buffer(buffer) abort - if has_key(g:ale_buffer_count_map, a:buffer) - call remove(g:ale_buffer_count_map, a:buffer) - endif - if has_key(g:ale_buffer_loclist_map, a:buffer) call remove(g:ale_buffer_loclist_map, a:buffer) endif - if has_key(g:ale_buffer_should_reset_map, a:buffer) - call remove(g:ale_buffer_should_reset_map, a:buffer) - endif + if has_key(g:ale_buffer_info, a:buffer) + " When buffers are removed, clear all of the jobs. + for l:job in get(g:ale_buffer_info[a:buffer], 'job_list', []) + call ale#engine#ClearJob(l:job) + endfor - if has_key(g:ale_buffer_sign_dummy_map, a:buffer) - call remove(g:ale_buffer_sign_dummy_map, a:buffer) + call remove(g:ale_buffer_info, a:buffer) endif endfunction diff --git a/autoload/ale/engine.vim b/autoload/ale/engine.vim index 5cdcd7ce..3af60ab6 100644 --- a/autoload/ale/engine.vim +++ b/autoload/ale/engine.vim @@ -20,7 +20,17 @@ function! s:GetJobID(job) abort return ch_info(job_getchannel(a:job)).id endfunction -function! s:ClearJob(job) abort +function! ale#engine#InitBufferInfo(buffer) abort + if !has_key(g:ale_buffer_info, a:buffer) + let g:ale_buffer_info[a:buffer] = { + \ 'job_list': [], + \ 'should_reset': 1, + \ 'dummy_sign_set': 0, + \} + endif +endfunction + +function! ale#engine#ClearJob(job) abort let l:job_id = s:GetJobID(a:job) let l:linter = s:job_info_map[l:job_id].linter @@ -37,7 +47,26 @@ function! s:ClearJob(job) abort endif call remove(s:job_info_map, l:job_id) - call remove(l:linter, 'job') +endfunction + +function! s:StopPreviousJobs(buffer, linter) abort + let l:new_job_list = [] + + for l:job in g:ale_buffer_info[a:buffer].job_list + let l:job_id = s:GetJobID(l:job) + + if has_key(s:job_info_map, l:job_id) + \&& s:job_info_map[l:job_id].linter.name ==# a:linter.name + " Stop jobs which match the buffer and linter. + call ale#engine#ClearJob(l:job) + else + " Keep other jobs in the list. + call add(l:new_job_list, l:job) + endif + endfor + + " Update the list, removing the previously run job. + let g:ale_buffer_info[a:buffer].job_list = l:new_job_list endfunction function! s:GatherOutput(job, data) abort @@ -71,17 +100,11 @@ function! s:HandleExit(job) abort endif let l:job_info = s:job_info_map[l:job_id] - - call s:ClearJob(a:job) - let l:linter = l:job_info.linter let l:output = l:job_info.output let l:buffer = l:job_info.buffer - if !has_key(g:ale_buffer_should_reset_map, l:buffer) - " A job ended for a buffer which has been closed, so stop here. - return - endif + call s:StopPreviousJobs(l:buffer, l:linter) let l:linter_loclist = ale#util#GetFunction(l:linter.callback)(l:buffer, l:output) @@ -92,8 +115,10 @@ function! s:HandleExit(job) abort let l:item.linter_name = l:linter.name endfor - if g:ale_buffer_should_reset_map[l:buffer] - let g:ale_buffer_should_reset_map[l:buffer] = 0 + if g:ale_buffer_info[l:buffer].should_reset + " Set the flag for resetting the loclist to 0 again, so we won't + " empty the list later. + let g:ale_buffer_info[l:buffer].should_reset = 0 let g:ale_buffer_loclist_map[l:buffer] = [] endif @@ -150,10 +175,8 @@ function! s:FixLocList(buffer, loclist) abort endfunction function! ale#engine#Invoke(buffer, linter) abort - if has_key(a:linter, 'job') - " Stop previous jobs for the same linter. - call s:ClearJob(a:linter.job) - endif + " Stop previous jobs for the same linter. + call s:StopPreviousJobs(a:buffer, a:linter) if has_key(a:linter, 'command_callback') " If there is a callback for generating a command, call that instead. @@ -227,7 +250,8 @@ function! ale#engine#Invoke(buffer, linter) abort " Only proceed if the job is being run. if has('nvim') || (l:job !=# 'no process' && job_status(l:job) ==# 'run') - let a:linter.job = l:job + " Add the job to the list of jobs, so we can track them. + call add(g:ale_buffer_info[a:buffer].job_list, l:job) " Store the ID for the job in the map to read back again. let s:job_info_map[s:GetJobID(l:job)] = { @@ -270,8 +294,8 @@ function! ale#engine#WaitForJobs(deadline) abort let l:job_list = [] - for l:job_id in keys(s:job_info_map) - call add(l:job_list, s:job_info_map[l:job_id].linter.job) + for l:info in values(g:ale_buffer_info) + call extend(l:job_list, l:info.job_list) endfor let l:should_wait_more = 1 diff --git a/autoload/ale/sign.vim b/autoload/ale/sign.vim index 23666287..deec47f2 100644 --- a/autoload/ale/sign.vim +++ b/autoload/ale/sign.vim @@ -89,13 +89,13 @@ function! ale#sign#SetSigns(buffer, loclist) abort let l:signlist = ale#sign#CombineSigns(a:loclist) if len(l:signlist) > 0 || g:ale_sign_column_always - if !get(g:ale_buffer_sign_dummy_map, a:buffer, 0) + if !g:ale_buffer_info[a:buffer].dummy_sign_set " Insert a dummy sign if one is missing. execute 'sign place ' . g:ale_sign_offset \ . ' line=1 name=ALEDummySign buffer=' \ . a:buffer - let g:ale_buffer_sign_dummy_map[a:buffer] = 1 + let g:ale_buffer_info[a:buffer].dummy_sign_set = 1 endif endif @@ -121,10 +121,10 @@ function! ale#sign#SetSigns(buffer, loclist) abort endfor if !g:ale_sign_column_always && len(l:signlist) > 0 - if get(g:ale_buffer_sign_dummy_map, a:buffer, 0) + if g:ale_buffer_info[a:buffer].dummy_sign_set execute 'sign unplace ' . g:ale_sign_offset . ' buffer=' . a:buffer - let g:ale_buffer_sign_dummy_map[a:buffer] = 0 + let g:ale_buffer_info[a:buffer].dummy_sign_set = 0 endif endif endfunction diff --git a/autoload/ale/statusline.vim b/autoload/ale/statusline.vim index 8a31bc39..48b4d687 100644 --- a/autoload/ale/statusline.vim +++ b/autoload/ale/statusline.vim @@ -14,34 +14,48 @@ function! ale#statusline#Update(buffer, loclist) abort endif endfor - let g:ale_buffer_count_map[a:buffer] = [l:errors, l:warnings] + let g:ale_buffer_info[a:buffer].count = [l:errors, l:warnings] +endfunction + +" Set the error and warning counts, calling for an update only if needed. +" If counts cannot be set, return 0. +function! s:SetupCount(buffer) abort + if !has_key(g:ale_buffer_info, a:buffer) + " Linters have not been run for the buffer yet, so stop here. + return 0 + endif + + " Cache is cold, so manually ask for an update. + if !has_key(g:ale_buffer_info[a:buffer], 'count') + call ale#statusline#Update(a:buffer, get(g:ale_buffer_loclist_map, a:buffer, [])) + endif + + return 1 endfunction " Returns a tuple of errors and warnings for use in third-party integrations. function! ale#statusline#Count(buffer) abort - " Cache is cold, so manually ask for an update. - if !has_key(g:ale_buffer_count_map, a:buffer) - call ale#statusline#Update(a:buffer, get(g:ale_buffer_loclist_map, a:buffer, [])) + if !s:SetupCount(a:buffer) + return [0, 0] endif - return g:ale_buffer_count_map[a:buffer] + return g:ale_buffer_info[a:buffer].count endfunction " Returns a formatted string that can be integrated in the statusline. function! ale#statusline#Status() abort + let [l:error_format, l:warning_format, l:no_errors] = g:ale_statusline_format let l:buffer = bufnr('%') - " Cache is cold, so manually ask for an update. - if !has_key(g:ale_buffer_count_map, l:buffer) - call ale#statusline#Update(l:buffer, get(g:ale_buffer_loclist_map, l:buffer, [])) + if !s:SetupCount(l:buffer) + return l:no_errors endif + let [l:error_count, l:warning_count] = g:ale_buffer_info[l:buffer].count + " Build strings based on user formatting preferences. - let l:errors = g:ale_buffer_count_map[l:buffer][0] ? - \ printf(g:ale_statusline_format[0], g:ale_buffer_count_map[l:buffer][0]) : '' - let l:warnings = g:ale_buffer_count_map[l:buffer][1] ? - \ printf(g:ale_statusline_format[1], g:ale_buffer_count_map[l:buffer][1]) : '' - let l:no_errors = g:ale_statusline_format[2] + let l:errors = l:error_count ? printf(l:error_format, l:error_count) : '' + let l:warnings = l:warning_count ? printf(l:warning_format, l:warning_count) : '' " Different formats based on the combination of errors and warnings. if empty(l:errors) && empty(l:warnings) diff --git a/plugin/ale.vim b/plugin/ale.vim index d1c29902..7e88f688 100644 --- a/plugin/ale.vim +++ b/plugin/ale.vim @@ -24,10 +24,8 @@ endif " Globals -let g:ale_buffer_count_map = {} let g:ale_buffer_loclist_map = {} -let g:ale_buffer_should_reset_map = {} -let g:ale_buffer_sign_dummy_map = {} +let g:ale_buffer_info = {} " User Configuration diff --git a/test/test_cleanup.vader b/test/test_cleanup.vader index 4f6827f4..a700d6b5 100644 --- a/test/test_cleanup.vader +++ b/test/test_cleanup.vader @@ -1,32 +1,21 @@ Before: let g:buffer = bufnr('%') - let g:ale_buffer_count_map = { - \ g:buffer: [1, 1], - \ 10347: [1, 1], - \} let g:ale_buffer_loclist_map = { \ g:buffer : [], \ 10347: [], \} - let g:ale_buffer_should_reset_map = { - \ g:buffer : 1, - \ 10347: 1, - \} - let g:ale_buffer_sign_dummy_map = { - \ g:buffer : 1, - \ 10347: 1, + + let g:ale_buffer_info = { + \ g:buffer : {}, + \ 10347: {}, \} After: unlet! g:buffer - let g:ale_buffer_count_map = {} let g:ale_buffer_loclist_map = {} - let g:ale_buffer_should_reset_map = {} - let g:ale_buffer_sign_dummy_map = {} + let g:ale_buffer_info = {} Execute('ALE globals should be cleared when the buffer is closed.'): :q! - AssertEqual {10347: [1, 1]}, g:ale_buffer_count_map AssertEqual {10347: []}, g:ale_buffer_loclist_map - AssertEqual {10347: 1}, g:ale_buffer_should_reset_map - AssertEqual {10347: 1}, g:ale_buffer_sign_dummy_map + AssertEqual {10347: {}}, g:ale_buffer_info diff --git a/test/test_linting_sets_signs.vader b/test/test_linting_sets_signs.vader new file mode 100644 index 00000000..1f323fe8 --- /dev/null +++ b/test/test_linting_sets_signs.vader @@ -0,0 +1,40 @@ +Given javascript (Some JavaScript with problems): + var y = 3+3; + var y = 3 + +Before: + sign unplace * + let g:actual_sign_list = [] + let g:expected_sign_list = [ + \ ['1', 'ALEWarningSign'], + \ ['2', 'ALEErrorSign'], + \] + + function! g:CollectSigns() + redir => l:output + silent exec 'sign place' + redir END + + for l:line in split(l:output, "\n") + let l:match = matchlist(l:line, 'line=\(\d\+\).*name=\(ALE[a-zA-Z]\+\)') + + if len(l:match) > 0 + call add(g:actual_sign_list, [l:match[1], l:match[2]]) + endif + endfor + endfunction + +After: + sign unplace * + let g:ale_buffer_loclist_map = {} + let g:ale_buffer_info = {} + delfunction g:CollectSigns + unlet g:actual_sign_list + unlet g:expected_sign_list + +Execute(The signs should be updated after linting is done): + call ale#Lint() + call ale#engine#WaitForJobs(2000) + call g:CollectSigns() + + AssertEqual g:expected_sign_list, g:actual_sign_list diff --git a/test/test_statusline.vader b/test/test_statusline.vader index da4c693f..43245e97 100644 --- a/test/test_statusline.vader +++ b/test/test_statusline.vader @@ -1,49 +1,59 @@ Before: let g:ale_buffer_loclist_map = {} + let g:ale_statusline_format = ['%sE', '%sW', 'OKIE'] + +After: + let g:ale_buffer_loclist_map = {} + let g:ale_buffer_info = {} Execute (Count should be 0 when data is empty): + let g:ale_buffer_info = {} AssertEqual [0, 0], ale#statusline#Count(bufnr('%')) -Before: - let g:ale_buffer_count_map = {'44': [1, 2]} - Execute (Count should read data from the cache): + let g:ale_buffer_info = {'44': {'count': [1, 2]}} AssertEqual [1, 2], ale#statusline#Count(44) -Execute (Update the cache with new data): +Execute (The count should be correct after an update): + let g:ale_buffer_info = {'44': {}} call ale#statusline#Update(44, []) - -Then (The cache should reflect the new data): AssertEqual [0, 0], ale#statusline#Count(44) -Before: - let g:ale_buffer_loclist_map = {'1': [{'lnum': 1, 'bufnr': 1, 'vcol': 0, 'linter_name': 'testlinter', 'nr': -1, 'type': 'E', 'col': 1, 'text': 'Test Error'}]} - Execute (Count should be match the loclist): - AssertEqual [1, 0], ale#statusline#Count(1) + let g:ale_buffer_info = {bufnr('%'): {}} + let g:ale_buffer_loclist_map = {bufnr('%'): [ + \ { + \ 'lnum': 1, + \ 'bufnr': 1, + \ 'vcol': 0, + \ 'linter_name': 'testlinter', + \ 'nr': -1, + \ 'type': 'E', + \ 'col': 1, + \ 'text': 'Test Error', + \ }, + \]} + AssertEqual [1, 0], ale#statusline#Count(bufnr('%')) Execute (Output should be empty for non-existant buffer): AssertEqual [0, 0], ale#statusline#Count(9001) -Before: - let g:ale_statusline_format = ['%sE', '%sW', 'OKIE'] - -Execute (Given some errors): +Execute (Statusline is formatted to the users preference for just errors): + let g:ale_buffer_info = {bufnr('%'): {}} call ale#statusline#Update(bufnr('%'), [{'type': 'E'}, {'type': 'E'}]) -Then (Statusline is formatted to the users preference): AssertEqual '2E', ale#statusline#Status() -Execute (Given some warnings): +Execute (Statusline is formatted to the users preference for just warnings): + let g:ale_buffer_info = {bufnr('%'): {}} call ale#statusline#Update(bufnr('%'), [{'type': 'W'}, {'type': 'W'}, {'type': 'W'}]) -Then (Statusline is formatted to the users preference): AssertEqual '3W', ale#statusline#Status() -Execute (Given some warnings, and errors): +Execute (Statusline is formatted to the users preference for errors and warnings): + let g:ale_buffer_info = {bufnr('%'): {}} call ale#statusline#Update(bufnr('%'), [{'type': 'E'}, {'type': 'W'}, {'type': 'W'}]) -Then (Statusline is formatted to the users preference): AssertEqual '1E 2W', ale#statusline#Status() -Execute (Given a lack of data): +Execute (Statusline is formatted to the users preference for no errors or warnings): + let g:ale_buffer_info = {bufnr('%'): {}} call ale#statusline#Update(bufnr('%'), []) -Then (Statusline is formatted to the users preference): AssertEqual 'OKIE', ale#statusline#Status()