Fix #171 - Implement basic error highlighting

8 files changed, 168 insertions, 11 deletions

diff --git a/autoload/ale/cleanup.vim b/autoload/ale/cleanup.vim
index d720cc96..3b0b1d90 100644
--- a/autoload/ale/cleanup.vim
+++ b/autoload/ale/cleanup.vim
@@ -10,6 +10,11 @@ function! ale#cleanup#Buffer(buffer) abort
             call ale#engine#ClearJob(l:job)
         endfor
 
+        " Clear delayed highlights for a buffer being removed.
+        if g:ale_set_highlights
+            call ale#highlight#UnqueueHighlights(a:buffer)
+        endif
+
         call remove(g:ale_buffer_info, a:buffer)
     endif
 endfunction
diff --git a/autoload/ale/engine.vim b/autoload/ale/engine.vim
index 7d22ad11..fc6a7309 100644
--- a/autoload/ale/engine.vim
+++ b/autoload/ale/engine.vim
@@ -234,9 +234,6 @@ function! s:HandleExit(job) abort
 
     " Call user autocommands. This allows users to hook into ALE's lint cycle.
     silent doautocmd User ALELint
-
-    " Mark line 200, column 17 with a squiggly line or something
-    " matchadd('ALEError', '\%200l\%17v')
 endfunction
 
 function! ale#engine#SetResults(buffer, loclist) abort
@@ -252,6 +249,10 @@ function! ale#engine#SetResults(buffer, loclist) abort
         " Don't load/run if not already loaded.
         call ale#statusline#Update(a:buffer, a:loclist)
     endif
+
+    if g:ale_set_highlights
+        call ale#highlight#SetHighlights(a:buffer, a:loclist)
+    endif
 endfunction
 
 function! s:HandleExitNeoVim(job, data, event) abort
diff --git a/autoload/ale/highlight.vim b/autoload/ale/highlight.vim
new file mode 100644
index 00000000..348226ea
--- /dev/null
+++ b/autoload/ale/highlight.vim
@@ -0,0 +1,75 @@
+scriptencoding utf8
+" Author: w0rp <devw0rp@gmail.com>
+" Description: This module implements error/warning highlighting.
+
+if !hlexists('ALEError')
+    highlight link ALEError SpellBad
+endif
+
+if !hlexists('ALEWarning')
+    highlight link ALEWarning SpellCap
+endif
+
+" This map holds highlights to be set when buffers are opened.
+" We can only set highlights for whatever the current buffer is, so we will
+" wait until the buffer is entered again to show the highlights, unless
+" the buffer is in focus when linting completes.
+let s:buffer_highlights = {}
+
+function! ale#highlight#UnqueueHighlights(buffer) abort
+    if has_key(s:buffer_highlights, a:buffer)
+        call remove(s:buffer_highlights, a:buffer)
+    endif
+endfunction
+
+function! s:RemoveOldHighlights() abort
+    for l:match in getmatches()
+        if l:match['group'] ==# 'ALEError' || l:match['group'] ==# 'ALEWarning'
+            call matchdelete(l:match['id'])
+        endif
+    endfor
+endfunction
+
+function! ale#highlight#UpdateHighlights() abort
+    let l:buffer = bufnr('%')
+    let l:has_new_items = has_key(s:buffer_highlights, l:buffer)
+    let l:loclist = l:has_new_items ? remove(s:buffer_highlights, l:buffer) : []
+
+    if l:has_new_items || !g:ale_enabled
+        call s:RemoveOldHighlights()
+    endif
+
+    if l:has_new_items
+        for l:item in l:loclist
+            let l:col = l:item.col
+            let l:group = l:item.type ==# 'E' ? 'ALEError' : 'ALEWarning'
+            let l:line = l:item.lnum
+            let l:size = 1
+
+            call matchaddpos(l:group, [[l:line, l:col, l:size]])
+        endfor
+    endif
+endfunction
+
+augroup ALEHighlightBufferGroup
+    autocmd!
+    autocmd BufEnter * call ale#highlight#UpdateHighlights()
+augroup END
+
+function! ale#highlight#SetHighlights(buffer, loclist) abort
+    " Only set set items for the buffer if ALE is enabled.
+    if g:ale_enabled
+        " Set a list of items to be set as highlights for a buffer when
+        " we next open it.
+        "
+        " We'll filter the loclist down to items we can set now.
+        let s:buffer_highlights[a:buffer] = filter(
+        \   a:loclist,
+        \   'v:val.bufnr == a:buffer && v:val.col > 0'
+        \)
+
+        " Update highlights for the current buffer, which may or may not
+        " be the buffer we just set highlights for.
+        call ale#highlight#UpdateHighlights()
+    endif
+endfunction
diff --git a/autoload/ale/sign.vim b/autoload/ale/sign.vim
index e2b3fa74..9d613480 100644
--- a/autoload/ale/sign.vim
+++ b/autoload/ale/sign.vim
@@ -12,14 +12,6 @@ if !hlexists('ALEWarningSign')
     highlight link ALEWarningSign todo
 endif
 
-if !hlexists('ALEError')
-    highlight link ALEError SpellBad
-endif
-
-if !hlexists('ALEWarning')
-    highlight link ALEWarning SpellCap
-endif
-
 " Signs show up on the left for error markers.
 execute 'sign define ALEErrorSign text=' . g:ale_sign_error
 \   . ' texthl=ALEErrorSign'
diff --git a/doc/ale.txt b/doc/ale.txt
index 110f9075..855abd1e 100644
--- a/doc/ale.txt
+++ b/doc/ale.txt
@@ -322,6 +322,17 @@ g:ale_open_list                                               *g:ale_open_list*
   to `1`, in which case the window will be kept open until closed manually.
 
 
+g:ale_set_highlights                                     *g:ale_set_highlights*
+
+  Type: |Number|
+  Default: `has('syntax')`
+
+  When this option is set to `1`, highlights will be set in for erros and
+  warnings. The `ALEError` and `ALEWarning` highlight groups will be used to
+  provide highlights, and default to linking to `SpellBad` and `SpellCap`
+  respectively by default.
+
+
 g:ale_set_loclist                                           *g:ale_set_loclist*
 
   Type: |Number|
diff --git a/plugin/ale.vim b/plugin/ale.vim
index d65a99cb..6afa603b 100644
--- a/plugin/ale.vim
+++ b/plugin/ale.vim
@@ -81,6 +81,9 @@ let g:ale_keep_list_window_open = get(g:, 'ale_keep_list_window_open', 0)
 " This is enabled by default only if the 'signs' feature exists.
 let g:ale_set_signs = get(g:, 'ale_set_signs', has('signs'))
 
+" This flag can be set to 0 to disable setting error highlights.
+let g:ale_set_highlights = get(g:, 'ale_set_highlights', has('syntax'))
+
 " These variables dicatate what sign is used to indicate errors and warnings.
 let g:ale_sign_error = get(g:, 'ale_sign_error', '>>')
 let g:ale_sign_warning = get(g:, 'ale_sign_warning', '--')
@@ -161,6 +164,11 @@ function! s:ALEToggle() abort
             " Clear signs, loclist, quicklist
             call ale#engine#SetResults(l:buffer, [])
         endfor
+
+        " Remove highlights for the current buffer now.
+        if g:ale_set_higlights
+            call ale#highlight#UpdateHighlights()
+        endif
     endif
 
     call s:ALEInitAuGroups()
diff --git a/test/test_highlight_clearing.vader b/test/test_highlight_clearing.vader
new file mode 100644
index 00000000..bf805924
--- /dev/null
+++ b/test/test_highlight_clearing.vader
@@ -0,0 +1,4 @@
+Execute(ALE should be able to queue highlights and clear them for some other buffer):
+  " We'll just make sure that this doesn't blow up.
+  call ale#highlight#SetHighlights(bufnr('%') + 1, [])
+  call ale#highlight#UnqueueHighlights(bufnr('%') + 1)
diff --git a/test/test_highlight_placement.vader b/test/test_highlight_placement.vader
new file mode 100644
index 00000000..b3e40c18
--- /dev/null
+++ b/test/test_highlight_placement.vader
@@ -0,0 +1,61 @@
+Before:
+  function! GenerateResults(buffer, output)
+    return [
+    \ {
+    \   'lnum': 1,
+    \   'col': 1,
+    \   'bufnr': bufnr('%'),
+    \   'vcol': 0,
+    \   'nr': -1,
+    \   'type': 'E',
+    \   'text': 'foo',
+    \ },
+    \ {
+    \   'lnum': 2,
+    \   'col': 1,
+    \   'bufnr': bufnr('%'),
+    \   'vcol': 0,
+    \   'nr': -1,
+    \   'type': 'W',
+    \   'text': 'bar',
+    \ },
+    \ {
+    \   'lnum': 3,
+    \   'col': 5,
+    \   'bufnr': bufnr('%'),
+    \   'vcol': 0,
+    \   'nr': -1,
+    \   'type': 'E',
+    \   'text': 'wat',
+    \ },
+    \]
+  endfunction
+
+  call ale#linter#Define('testft', {
+  \ 'name': 'x',
+  \ 'executable': 'echo',
+  \ 'command': 'echo',
+  \ 'callback': 'GenerateResults',
+  \})
+
+After:
+  delfunction GenerateResults
+  call ale#linter#Reset()
+  let g:ale_buffer_info = {}
+
+Given testft(A Javscript file with warnings/errors):
+  foo
+  bar
+  baz wat
+
+Execute(Highlights should be set when a linter runs):
+  call ale#Lint()
+  call ale#engine#WaitForJobs(2000)
+
+  AssertEqual
+  \ [
+  \   {'group': 'ALEError', 'id': 4, 'priority': 10, 'pos1': [1, 1, 1]},
+  \   {'group': 'ALEWarning', 'id': 5, 'priority': 10, 'pos1': [2, 1, 1]},
+  \   {'group': 'ALEError', 'id': 6, 'priority': 10, 'pos1': [3, 5, 1]}
+  \ ],
+  \ getmatches()