From 5de445c041432b602c590a175809d89837cdf2b8 Mon Sep 17 00:00:00 2001 From: w0rp Date: Thu, 9 Feb 2017 23:32:57 +0000 Subject: [PATCH] Fix #315 Implement the read_buffer option --- autoload/ale/engine.vim | 64 +++++---- autoload/ale/linter.vim | 15 +++ doc/ale.txt | 18 ++- test/test_engine_invocation.vader | 139 ++++++++++++++++++++ test/test_linter_defintion_processing.vader | 49 +++++++ test/test_linter_retrieval.vader | 6 +- 6 files changed, 262 insertions(+), 29 deletions(-) create mode 100644 test/test_engine_invocation.vader diff --git a/autoload/ale/engine.vim b/autoload/ale/engine.vim index 34e5ad95..59c6de77 100644 --- a/autoload/ale/engine.vim +++ b/autoload/ale/engine.vim @@ -214,13 +214,13 @@ function! s:FixLocList(buffer, loclist) abort endfor endfunction -function! s:RunJob(command, generic_job_options) abort - let l:buffer = a:generic_job_options.buffer - let l:linter = a:generic_job_options.linter - let l:output_stream = a:generic_job_options.output_stream - let l:next_chain_index = a:generic_job_options.next_chain_index - let l:read_buffer = a:generic_job_options.read_buffer - let l:command = a:command +function! s:RunJob(options) abort + let l:command = a:options.command + let l:buffer = a:options.buffer + let l:linter = a:options.linter + let l:output_stream = a:options.output_stream + let l:next_chain_index = a:options.next_chain_index + let l:read_buffer = a:options.read_buffer if l:command =~# '%s' " If there is a '%s' in the command string, replace it with the name @@ -321,8 +321,11 @@ function! s:RunJob(command, generic_job_options) abort endif endfunction -function! s:InvokeChain(buffer, linter, chain_index, input) abort +" Determine which commands to run for a link in a command chain, or +" just a regular command. +function! ale#engine#ProcessChain(buffer, linter, chain_index, input) abort let l:output_stream = get(a:linter, 'output_stream', 'stdout') + let l:read_buffer = a:linter.read_buffer let l:chain_index = a:chain_index let l:input = a:input @@ -332,11 +335,6 @@ function! s:InvokeChain(buffer, linter, chain_index, input) abort " so that many programs can be run in a sequence. let l:chain_item = a:linter.command_chain[l:chain_index] - " The chain item can override the output_stream option. - if has_key(l:chain_item, 'output_stream') - let l:output_stream = l:chain_item.output_stream - endif - if l:chain_index == 0 " The first callback in the chain takes only a buffer number. let l:command = ale#util#GetFunction(l:chain_item.callback)( @@ -352,6 +350,21 @@ function! s:InvokeChain(buffer, linter, chain_index, input) abort if !empty(l:command) " We hit a command to run, so we'll execute that + + " The chain item can override the output_stream option. + if has_key(l:chain_item, 'output_stream') + let l:output_stream = l:chain_item.output_stream + endif + + " The chain item can override the read_buffer option. + if has_key(l:chain_item, 'read_buffer') + let l:read_buffer = l:chain_item.read_buffer + elseif l:chain_index != len(a:linter.command_chain) - 1 + " Don't read the buffer for commands besides the last one + " in the chain by default. + let l:read_buffer = 0 + endif + break endif @@ -361,11 +374,6 @@ function! s:InvokeChain(buffer, linter, chain_index, input) abort let l:input = [] let l:chain_index += 1 endwhile - - if empty(l:command) - " Don't run any jobs if the last command is an empty string. - return - endif elseif has_key(a:linter, 'command_callback') " If there is a callback for generating a command, call that instead. let l:command = ale#util#GetFunction(a:linter.command_callback)(a:buffer) @@ -373,15 +381,27 @@ function! s:InvokeChain(buffer, linter, chain_index, input) abort let l:command = a:linter.command endif - let l:is_last_job = l:chain_index >= len(get(a:linter, 'command_chain', [])) - 1 + if empty(l:command) + " Don't run any jobs if the command is an empty string. + return {} + endif - call s:RunJob(l:command, { + return { + \ 'command': l:command, \ 'buffer': a:buffer, \ 'linter': a:linter, \ 'output_stream': l:output_stream, \ 'next_chain_index': l:chain_index + 1, - \ 'read_buffer': l:is_last_job, - \}) + \ 'read_buffer': l:read_buffer, + \} +endfunction + +function! s:InvokeChain(buffer, linter, chain_index, input) abort + let l:options = ale#engine#ProcessChain(a:buffer, a:linter, a:chain_index, a:input) + + if !empty(l:options) + call s:RunJob(l:options) + endif endfunction function! ale#engine#Invoke(buffer, linter) abort diff --git a/autoload/ale/linter.vim b/autoload/ale/linter.vim index be173588..260c145b 100644 --- a/autoload/ale/linter.vim +++ b/autoload/ale/linter.vim @@ -33,6 +33,10 @@ function! s:IsCallback(value) abort return type(a:value) == type('') || type(a:value) == type(function('type')) endfunction +function! s:IsBoolean(value) abort + return type(a:value) == type(0) && (a:value == 0 || a:value == 1) +endfunction + function! ale#linter#PreProcess(linter) abort if type(a:linter) != type({}) throw 'The linter object must be a Dictionary' @@ -95,6 +99,10 @@ function! ale#linter#PreProcess(linter) abort endif endif + if has_key(l:link, 'read_buffer') && !s:IsBoolean(l:link.read_buffer) + throw l:err_prefix . 'value for `read_buffer` must be `0` or `1`' + endif + let l:link_index += 1 endfor elseif has_key(a:linter, 'command_callback') @@ -130,6 +138,13 @@ function! ale#linter#PreProcess(linter) abort throw "`output_stream` must be 'stdout', 'stderr', or 'both'" endif + " An option indicating that the buffer should be read. + let l:obj.read_buffer = get(a:linter, 'read_buffer', 1) + + if !s:IsBoolean(l:obj.read_buffer) + throw '`read_buffer` must be `0` or `1`' + endif + return l:obj endfunction diff --git a/doc/ale.txt b/doc/ale.txt index 348be809..a4d476c3 100644 --- a/doc/ale.txt +++ b/doc/ale.txt @@ -1082,10 +1082,14 @@ ale#linter#Define(filetype, linter) *ale#linter#Define()* See the `output_stream` description for more information. - The Vim buffer being checked for linter will only - be sent to the final command in the chain. Previous - commands in the chain will receive no input from - stdin. + Commands in the chain all behave as if `read_buffer` + is set to `0` by default, except for the last command + in the chain, which uses the value set for + `read_buffer` in the root |Dictionary|. Each command + in the chain can also provide a `read_buffer` key + to override these values. + See the `read_buffer` description for more + information. `output_stream` A |String| for the output stream the lines of output should be read from for the command which is run. The @@ -1096,6 +1100,12 @@ ale#linter#Define(filetype, linter) *ale#linter#Define()* instead of stdout. The option `'both'` will read from both stder and stdout at the same time. + `read_buffer` A |Number| (`0` or `1`) indicating whether a command + should read the Vim buffer as input via stdin. This + option is set to `1` by default, and can be disabled + if a command manually reads from a temporary file + instead, etc. + Only one of `command`, `command_callback`, or `command_chain` should be specified. `command_callback` is generally recommended when a command string needs to be generated dynamically, or any global options are used. diff --git a/test/test_engine_invocation.vader b/test/test_engine_invocation.vader new file mode 100644 index 00000000..c56895d3 --- /dev/null +++ b/test/test_engine_invocation.vader @@ -0,0 +1,139 @@ +Before: + function! CollectResults(buffer, output) + return [] + endfunction + + function! FirstChainFunction(buffer) + return 'first' + endfunction + + function! SecondChainFunction(buffer, output) + " We'll skip this command + return '' + endfunction + + function! ThirdChainFunction(buffer, output) + return 'third' + endfunction + + function! FourthChainFunction(buffer, output) + return 'fourth' + endfunction + + let g:linter = { + \ 'name': 'testlinter', + \ 'callback': 'CollectResults', + \ 'executable': 'echo', + \ 'command_chain': [ + \ {'callback': 'FirstChainFunction'}, + \ {'callback': 'SecondChainFunction'}, + \ {'callback': 'ThirdChainFunction'}, + \ {'callback': 'FourthChainFunction'}, + \ ], + \ 'read_buffer': 1, + \} + + function! ProcessIndex(chain_index) + return ale#engine#ProcessChain(347, g:linter, a:chain_index, []) + endfunction + +After: + delfunction CollectResults + delfunction FirstChainFunction + delfunction SecondChainFunction + delfunction ThirdChainFunction + delfunction ProcessIndex + unlet! g:linter + unlet! g:result + +Execute(Engine invocation should return the command for the first item correctly): + let g:result = ProcessIndex(0) + + AssertEqual 'first', g:result.command + AssertEqual 1, g:result.next_chain_index + +Execute(Engine invocation should return the command for the second item correctly): + let g:result = ProcessIndex(1) + + AssertEqual 'third', g:result.command + AssertEqual 3, g:result.next_chain_index + +Execute(Engine invocation should return the command for the fourth item correctly): + let g:result = ProcessIndex(3) + + AssertEqual 'fourth', g:result.command + AssertEqual 4, g:result.next_chain_index + +Execute(Engine invocation should return the command for a single callback correctly): + unlet g:linter.command_chain + let g:linter.command_callback = 'FirstChainFunction' + + let g:result = ProcessIndex(0) + + AssertEqual 'first', g:result.command + +Execute(Engine invocation should return the command for a command string correctly): + unlet g:linter.command_chain + let g:linter.command = 'foo bar' + + let g:result = ProcessIndex(0) + + AssertEqual 'foo bar', g:result.command + +Execute(Engine invocation should process read_buffer correctly for simple commands): + unlet g:linter.command_chain + let g:linter.command = 'foo bar' + let g:linter.read_buffer = 0 + + let g:result = ProcessIndex(0) + + AssertEqual 'foo bar', g:result.command + AssertEqual 0, g:result.read_buffer + + let g:linter.command_callback = 'FirstChainFunction' + unlet g:linter.command + + let g:result = ProcessIndex(0) + + AssertEqual 'first', g:result.command + AssertEqual 0, g:result.read_buffer + +Execute(Engine invocation should allow read_buffer to be enabled for a command in the middle of a chain): + let g:linter.command_chain[2].read_buffer = 1 + + let g:result = ProcessIndex(2) + + AssertEqual g:result.command, 'third' + AssertEqual g:result.read_buffer, 1 + +Execute(Engine invocation should allow read_buffer to be disabled for the end of a chain): + let g:linter.command_chain[3].read_buffer = 0 + + let g:result = ProcessIndex(3) + + AssertEqual g:result.command, 'fourth' + AssertEqual g:result.read_buffer, 0 + +Execute(Engine invocation should not use read_buffer from earlier items in a chain): + let g:linter.command_chain[1].read_buffer = 1 + + let g:result = ProcessIndex(1) + + AssertEqual g:result.command, 'third' + AssertEqual g:result.read_buffer, 0 + +Execute(Engine invocation should allow the output_stream setting to be changed in the middle of a chain): + let g:linter.command_chain[2].output_stream = 'both' + + let g:result = ProcessIndex(2) + + AssertEqual g:result.command, 'third' + AssertEqual g:result.output_stream, 'both' + +Execute(Engine invocation should not use output_stream from earlier items in a chain): + let g:linter.command_chain[1].output_stream = 'both' + + let g:result = ProcessIndex(1) + + AssertEqual g:result.command, 'third' + AssertEqual g:result.output_stream, 'stdout' diff --git a/test/test_linter_defintion_processing.vader b/test/test_linter_defintion_processing.vader index 969a8932..3acc194e 100644 --- a/test/test_linter_defintion_processing.vader +++ b/test/test_linter_defintion_processing.vader @@ -234,3 +234,52 @@ Execute(PreProcess should complain when conflicting command options are used): AssertThrows call ale#linter#PreProcess(g:linter) AssertEqual 'Only one of `command`, `command_callback`, or `command_chain` should be set', g:vader_exception + +Execute(PreProcess should process the read_buffer option correctly): + let g:linter = { + \ 'name': 'x', + \ 'callback': 'x', + \ 'executable': 'x', + \ 'command_chain': [{'callback': 'foo'}, {'callback': 'bar'}], + \ 'read_buffer': '0', + \} + + AssertThrows call ale#linter#PreProcess(g:linter) + AssertEqual '`read_buffer` must be `0` or `1`', g:vader_exception + + let g:linter.read_buffer = 0 + + call ale#linter#PreProcess(g:linter) + + let g:linter.read_buffer = 1 + + call ale#linter#PreProcess(g:linter) + + unlet g:linter.read_buffer + let g:linter.command_chain[0].read_buffer = '0' + + AssertThrows call ale#linter#PreProcess(g:linter) + AssertEqual 'The `command_chain` item 0 value for `read_buffer` must be `0` or `1`', g:vader_exception + + let g:linter.command_chain[0].read_buffer = 0 + + call ale#linter#PreProcess(g:linter) + + let g:linter.command_chain[1].read_buffer = '0' + + AssertThrows call ale#linter#PreProcess(g:linter) + AssertEqual 'The `command_chain` item 1 value for `read_buffer` must be `0` or `1`', g:vader_exception + + let g:linter.command_chain[1].read_buffer = 1 + + call ale#linter#PreProcess(g:linter) + +Execute(PreProcess should set a default value for read_buffer): + let g:linter = { + \ 'name': 'x', + \ 'callback': 'x', + \ 'executable': 'x', + \ 'command': 'x', + \} + + AssertEqual ale#linter#PreProcess(g:linter).read_buffer, 1 diff --git a/test/test_linter_retrieval.vader b/test/test_linter_retrieval.vader index 5dadf6bc..bc5b62ef 100644 --- a/test/test_linter_retrieval.vader +++ b/test/test_linter_retrieval.vader @@ -1,6 +1,6 @@ Before: - let g:testlinter1 = {'name': 'testlinter1', 'executable': 'testlinter1', 'command': 'testlinter1', 'callback': 'testCB1', 'output_stream': 'stdout'} - let g:testlinter2 = {'name': 'testlinter2', 'executable': 'testlinter2', 'command': 'testlinter2', 'callback': 'testCB2', 'output_stream': 'stdout'} + let g:testlinter1 = {'name': 'testlinter1', 'executable': 'testlinter1', 'command': 'testlinter1', 'callback': 'testCB1', 'output_stream': 'stdout', 'read_buffer': 1} + let g:testlinter2 = {'name': 'testlinter2', 'executable': 'testlinter2', 'command': 'testlinter2', 'callback': 'testCB2', 'output_stream': 'stdout', 'read_buffer': 0} call ale#linter#Reset() let g:ale_linters = {} @@ -40,4 +40,4 @@ Then (Linters for dot-seperated filetypes should be properly handled): AssertEqual [g:testlinter1, g:testlinter2], ale#linter#Get('testft1.testft2') Execute (Try to load a linter from disk): - AssertEqual [{'name': 'testlinter', 'output_stream': 'stdout', 'executable': 'testlinter', 'command': 'testlinter', 'callback': 'testCB'}], ale#linter#Get('testft') + AssertEqual [{'name': 'testlinter', 'output_stream': 'stdout', 'executable': 'testlinter', 'command': 'testlinter', 'callback': 'testCB', 'read_buffer': 1}], ale#linter#Get('testft')