Fix #876 - Save history in a separate buffer variable so history works when linting is disabled

5 files changed, 67 insertions, 80 deletions

diff --git a/autoload/ale/debugging.vim b/autoload/ale/debugging.vim
index 0042bd3c..7454bb1a 100644
--- a/autoload/ale/debugging.vim
+++ b/autoload/ale/debugging.vim
@@ -112,11 +112,7 @@ endfunction
 function! s:EchoCommandHistory() abort
     let l:buffer = bufnr('%')
 
-    if !has_key(g:ale_buffer_info, l:buffer)
-        return
-    endif
-
-    for l:item in g:ale_buffer_info[l:buffer].history
+    for l:item in ale#history#Get(l:buffer)
         if l:item.job_id is# 'executable'
             call s:EchoExecutable(l:item)
         else
diff --git a/autoload/ale/engine.vim b/autoload/ale/engine.vim
index b66a9fb6..b3ca4b99 100644
--- a/autoload/ale/engine.vim
+++ b/autoload/ale/engine.vim
@@ -53,14 +53,12 @@ function! ale#engine#InitBufferInfo(buffer) abort
         " loclist holds the loclist items after all jobs have completed.
         " temporary_file_list holds temporary files to be cleaned up
         " temporary_directory_list holds temporary directories to be cleaned up
-        " history holds a list of previously run commands for this buffer
         let g:ale_buffer_info[a:buffer] = {
         \   'job_list': [],
         \   'active_linter_list': [],
         \   'loclist': [],
         \   'temporary_file_list': [],
         \   'temporary_directory_list': [],
-        \   'history': [],
         \}
 
         return 1
@@ -557,8 +555,6 @@ function! s:RunJob(options) abort
 
     if g:ale_history_enabled
         call ale#history#Add(l:buffer, l:status, l:job_id, l:command)
-    else
-        let l:info.history = []
     endif
 
     if get(g:, 'ale_run_synchronously') == 1
diff --git a/autoload/ale/history.vim b/autoload/ale/history.vim
index 0356c022..a6282ea5 100644
--- a/autoload/ale/history.vim
+++ b/autoload/ale/history.vim
@@ -1,15 +1,20 @@
 " Author: w0rp <devw0rp@gmail.com>
 " Description: Tools for managing command history
-"
+
+" Return a shallow copy of the command history for a given buffer number.
+function! ale#history#Get(buffer) abort
+    return copy(getbufvar(a:buffer, 'ale_history', []))
+endfunction
+
 function! ale#history#Add(buffer, status, job_id, command) abort
     if g:ale_max_buffer_history_size <= 0
         " Don't save anything if the history isn't a positive number.
-        let g:ale_buffer_info[a:buffer].history = []
+        call setbufvar(a:buffer, 'ale_history', [])
 
         return
     endif
 
-    let l:history = g:ale_buffer_info[a:buffer].history
+    let l:history = getbufvar(a:buffer, 'ale_history', [])
 
     " Remove the first item if we hit the max history size.
     if len(l:history) >= g:ale_max_buffer_history_size
@@ -22,18 +27,13 @@ function! ale#history#Add(buffer, status, job_id, command) abort
     \   'command': a:command,
     \})
 
-    let g:ale_buffer_info[a:buffer].history = l:history
+    call setbufvar(a:buffer, 'ale_history', l:history)
 endfunction
 
 function! s:FindHistoryItem(buffer, job_id) abort
-    " Stop immediately if there's nothing set up for the buffer.
-    if !has_key(g:ale_buffer_info, a:buffer)
-        return {}
-    endif
-
     " Search backwards to find a matching job ID. IDs might be recycled,
     " so finding the last one should be good enough.
-    for l:obj in reverse(g:ale_buffer_info[a:buffer].history[:])
+    for l:obj in reverse(ale#history#Get(a:buffer))
         if l:obj.job_id == a:job_id
             return l:obj
         endif
@@ -46,18 +46,14 @@ endfunction
 function! ale#history#SetExitCode(buffer, job_id, exit_code) abort
     let l:obj = s:FindHistoryItem(a:buffer, a:job_id)
 
-    if !empty(l:obj)
-        " If we find a match, then set the code and status.
-        let l:obj.exit_code = a:exit_code
-        let l:obj.status = 'finished'
-    endif
+    " If we find a match, then set the code and status.
+    let l:obj.exit_code = a:exit_code
+    let l:obj.status = 'finished'
 endfunction
 
 " Set the output for a command which finished.
 function! ale#history#RememberOutput(buffer, job_id, output) abort
     let l:obj = s:FindHistoryItem(a:buffer, a:job_id)
 
-    if !empty(l:obj)
-        let l:obj.output = a:output
-    endif
+    let l:obj.output = a:output
 endfunction
diff --git a/test/test_ale_info.vader b/test/test_ale_info.vader
index 9cb768a2..8ab5ad54 100644
--- a/test/test_ale_info.vader
+++ b/test/test_ale_info.vader
@@ -1,6 +1,9 @@
 Before:
   Save g:ale_warn_about_trailing_whitespace
   Save g:ale_linters
+  Save g:ale_fixers
+
+  unlet! b:ale_history
 
   let g:ale_warn_about_trailing_whitespace = 1
 
@@ -10,6 +13,7 @@ Before:
   call ale#engine#ResetExecutableCache()
   call ale#linter#Reset()
   let g:ale_linters = {}
+  let g:ale_fixers = {}
   let g:ale_linter_aliases = {}
   let g:ale_buffer_info = {}
   let g:globals_lines = [
@@ -60,9 +64,10 @@ After:
 
   let g:ale_buffer_info = {}
 
-  unlet!  g:testlinter1
-  unlet!  g:testlinter2
+  unlet! g:testlinter1
+  unlet! g:testlinter2
 
+  unlet! b:ale_history
   unlet! b:ale_linters
   unlet! g:output
   unlet! g:globals_string
@@ -248,12 +253,10 @@ Execute (ALEInfo should output linter aliases):
 
 Given testft.testft2 (Empty buffer with two filetypes):
 Execute (ALEInfo should return command history):
-  let g:ale_buffer_info[bufnr('%')] = {
-  \ 'history': [
-  \   {'status': 'started', 'job_id': 347, 'command': 'first command'},
-  \   {'status': 'started', 'job_id': 347, 'command': ['/bin/bash', '\c', 'last command']},
-  \ ],
-  \}
+  let b:ale_history = [
+  \ {'status': 'started', 'job_id': 347, 'command': 'first command'},
+  \ {'status': 'started', 'job_id': 347, 'command': ['/bin/bash', '\c', 'last command']},
+  \]
 
   call ale#linter#Define('testft', g:testlinter1)
   call ale#linter#Define('testft2', g:testlinter2)
@@ -272,12 +275,10 @@ Execute (ALEInfo should return command history):
 
 Given testft.testft2 (Empty buffer with two filetypes):
 Execute (ALEInfo command history should print exit codes correctly):
-  let g:ale_buffer_info[bufnr('%')] = {
-  \ 'history': [
-  \   {'status': 'finished', 'exit_code': 0, 'job_id': 347, 'command': 'first command'},
-  \   {'status': 'finished', 'exit_code': 1, 'job_id': 347, 'command': ['/bin/bash', '\c', 'last command']},
-  \ ],
-  \}
+  let b:ale_history = [
+  \ {'status': 'finished', 'exit_code': 0, 'job_id': 347, 'command': 'first command'},
+  \ {'status': 'finished', 'exit_code': 1, 'job_id': 347, 'command': ['/bin/bash', '\c', 'last command']},
+  \]
 
   call ale#linter#Define('testft', g:testlinter1)
   call ale#linter#Define('testft2', g:testlinter2)
@@ -298,31 +299,29 @@ Given testft.testft2 (Empty buffer with two filetypes):
 Execute (ALEInfo command history should print command output if logging is on):
   let g:ale_history_log_output = 1
 
-  let g:ale_buffer_info[bufnr('%')] = {
-  \ 'history': [
-  \   {
-  \     'status': 'finished',
-  \     'exit_code': 0,
-  \     'job_id': 347,
-  \     'command': 'first command',
-  \     'output': ['some', 'first command output'],
-  \   },
-  \   {
-  \     'status': 'finished',
-  \     'exit_code': 1,
-  \     'job_id': 347,
-  \     'command': ['/bin/bash', '\c', 'last command'],
-  \     'output': ['different second command output'],
-  \   },
-  \   {
-  \     'status': 'finished',
-  \     'exit_code': 0,
-  \     'job_id': 347,
-  \     'command': 'command with no output',
-  \     'output': [],
-  \   },
-  \ ],
-  \}
+  let b:ale_history = [
+  \ {
+  \   'status': 'finished',
+  \   'exit_code': 0,
+  \   'job_id': 347,
+  \   'command': 'first command',
+  \   'output': ['some', 'first command output'],
+  \ },
+  \ {
+  \   'status': 'finished',
+  \   'exit_code': 1,
+  \   'job_id': 347,
+  \   'command': ['/bin/bash', '\c', 'last command'],
+  \   'output': ['different second command output'],
+  \ },
+  \ {
+  \   'status': 'finished',
+  \   'exit_code': 0,
+  \   'job_id': 347,
+  \   'command': 'command with no output',
+  \   'output': [],
+  \ },
+  \]
 
   call ale#linter#Define('testft', g:testlinter1)
   call ale#linter#Define('testft2', g:testlinter2)
@@ -354,8 +353,6 @@ Execute (ALEInfo command history should print command output if logging is on):
   \])
 
 Execute (ALEInfo should include executable checks in the history):
-  let g:ale_buffer_info[bufnr('')] = {'history': []}
-
   call ale#linter#Define('testft', g:testlinter1)
   call ale#engine#IsExecutable(bufnr(''), 'echo')
   call ale#engine#IsExecutable(bufnr(''), 'TheresNoWayThisIsExecutable')
diff --git a/test/test_history_saving.vader b/test/test_history_saving.vader
index 8207d153..3b8fb2aa 100644
--- a/test/test_history_saving.vader
+++ b/test/test_history_saving.vader
@@ -2,6 +2,8 @@ Before:
   Save g:ale_max_buffer_history_size
   Save g:ale_history_log_output
 
+  unlet! b:ale_history
+
   " Temporarily set the shell to /bin/sh, if it isn't already set that way.
   " This will make it so the test works when running it directly.
   let g:current_shell = &shell
@@ -26,6 +28,9 @@ Before:
 After:
   Restore
 
+  " Clear the history we changed.
+  unlet! b:ale_history
+
   " Reset the shell back to what it was before.
   let &shell = g:current_shell
   unlet g:current_shell
@@ -46,7 +51,7 @@ Execute(History should be set when commands are run):
   call ale#Lint()
   call ale#engine#WaitForJobs(2000)
 
-  let g:history = g:ale_buffer_info[bufnr('%')].history
+  let g:history = ale#history#Get(bufnr(''))
 
   AssertEqual 1, len(g:history)
   AssertEqual sort(['status', 'exit_code', 'job_id', 'command']), sort(keys(g:history[0]))
@@ -64,7 +69,7 @@ Execute(History should be not set when disabled):
   call ale#Lint()
   call ale#engine#WaitForJobs(2000)
 
-  AssertEqual 0, len(g:ale_buffer_info[bufnr('%')].history)
+  AssertEqual [], ale#history#Get(bufnr(''))
 
 Execute(History should include command output if logging is enabled):
   AssertEqual 'foobar', &filetype
@@ -74,35 +79,32 @@ Execute(History should include command output if logging is enabled):
   call ale#Lint()
   call ale#engine#WaitForJobs(2000)
 
-  let g:history = g:ale_buffer_info[bufnr('%')].history
+  let g:history = ale#history#Get(bufnr(''))
 
   AssertEqual 1, len(g:history)
   AssertEqual ['command history test'], g:history[0].output
 
 Execute(History items should be popped after going over the max):
-  let g:ale_buffer_info[1] = {
-  \ 'history': map(range(20), '{''status'': ''started'', ''job_id'': v:val, ''command'': ''foobar''}'),
-  \}
+  let b:ale_history = map(range(20), '{''status'': ''started'', ''job_id'': v:val, ''command'': ''foobar''}')
 
-  call ale#history#Add(1, 'started', 347, 'last command')
+  call ale#history#Add(bufnr(''), 'started', 347, 'last command')
 
   AssertEqual
   \ (
   \   map(range(1, 19), '{''status'': ''started'', ''job_id'': v:val, ''command'': ''foobar''}')
   \   + [{'status': 'started', 'job_id': 347, 'command': 'last command'}]
   \ ),
-  \ g:ale_buffer_info[1].history
+  \ ale#history#Get(bufnr(''))
 
 Execute(Nothing should be added to history if the size is too low):
   let g:ale_max_buffer_history_size = 0
-  let g:ale_buffer_info[1] = {'history': []}
 
-  call ale#history#Add(1, 'started', 347, 'last command')
+  call ale#history#Add(bufnr(''), 'started', 347, 'last command')
 
-  AssertEqual [], g:ale_buffer_info[1].history
+  AssertEqual [], ale#history#Get(bufnr(''))
 
   let g:ale_max_buffer_history_size = -2
 
   call ale#history#Add(1, 'started', 347, 'last command')
 
-  AssertEqual [], g:ale_buffer_info[1].history
+  AssertEqual [], ale#history#Get(bufnr(''))