Close #4401 - Use subtle defaults for virtual-text

Default virtual-text to the Comment highlight group and prefix
virtual-text messages with comment text for each language by default.

Messages can now be formatted with `%type%` to print the error type.

The Vim 9.0 version has been updated in the Docker image to add test
coverage for virtual-text.

Dockerfile

@@ -1,7 +1,7 @@
 FROM testbed/vim:24
 
 RUN install_vim -tag v8.0.0027 -build \
-                -tag v9.0.0133 -build \
+                -tag v9.0.0297 -build \
                 -tag neovim:v0.2.0 -build \
                 -tag neovim:v0.8.0 -build
 

autoload/ale.vim

@@ -254,6 +254,7 @@ function! ale#GetLocItemMessage(item, format_string) abort
     " \=l:variable is used to avoid escaping issues.
     let l:msg = substitute(l:msg, '\v\%([^\%]*)code([^\%]*)\%', l:code_repl, 'g')
     let l:msg = substitute(l:msg, '\V%severity%', '\=l:severity', 'g')
+    let l:msg = substitute(l:msg, '\V%type%', '\=l:type', 'g')
     let l:msg = substitute(l:msg, '\V%linter%', '\=l:linter_name', 'g')
     " Replace %s with the text.
     let l:msg = substitute(l:msg, '\V%s', '\=a:item.text', 'g')

autoload/ale/virtualtext.vim

@@ -4,7 +4,7 @@ scriptencoding utf-8
 " Description: Shows lint message for the current line as virtualtext, if any
 
 if !hlexists('ALEVirtualTextError')
-    highlight link ALEVirtualTextError SpellBad
+    highlight link ALEVirtualTextError Comment
 endif
 
 if !hlexists('ALEVirtualTextStyleError')
@@ -12,7 +12,7 @@ if !hlexists('ALEVirtualTextStyleError')
 endif
 
 if !hlexists('ALEVirtualTextWarning')
-    highlight link ALEVirtualTextWarning SpellCap
+    highlight link ALEVirtualTextWarning Comment
 endif
 
 if !hlexists('ALEVirtualTextStyleWarning')
@@ -23,11 +23,15 @@ if !hlexists('ALEVirtualTextInfo')
     highlight link ALEVirtualTextInfo ALEVirtualTextWarning
 endif
 
+let g:ale_virtualtext_prefix =
+\   get(g:, 'ale_virtualtext_prefix', '%comment% %type%: ')
 " Controls the milliseconds delay before showing a message.
 let g:ale_virtualtext_delay = get(g:, 'ale_virtualtext_delay', 10)
+
 let s:cursor_timer = get(s:, 'cursor_timer', -1)
 let s:last_pos = get(s:, 'last_pos', [0, 0, 0])
 let s:hl_list = get(s:, 'hl_list', [])
+let s:last_message = ''
 
 if !has_key(s:, 'has_virt_text')
     let s:has_virt_text = 0
@@ -47,72 +51,6 @@ if !has_key(s:, 'has_virt_text')
     endif
 endif
 
-function! ale#virtualtext#Clear(buf) abort
-    if !s:has_virt_text
-        return
-    endif
-
-    if has('nvim')
-        call nvim_buf_clear_namespace(a:buf, s:ns_id, 0, -1)
-    else
-        if s:emulate_virt && s:last_virt != -1
-            call prop_remove({'type': 'ale'})
-            call popup_close(s:last_virt)
-            let s:last_virt = -1
-        elseif !empty(s:hl_list)
-            call prop_remove({
-            \ 'types': s:hl_list,
-            \ 'all': 1,
-            \ 'bufnr': a:buf})
-        endif
-    endif
-endfunction
-
-function! ale#virtualtext#ShowMessage(message, hl_group, buf, line) abort
-    if !s:has_virt_text || !bufexists(str2nr(a:buf))
-        return
-    endif
-
-    let l:prefix = get(g:, 'ale_virtualtext_prefix', '> ')
-    let l:msg = l:prefix.trim(substitute(a:message, '\n', ' ', 'g'))
-
-    if has('nvim')
-        call nvim_buf_set_virtual_text(a:buf, s:ns_id, a:line-1, [[l:msg, a:hl_group]], {})
-    elseif s:emulate_virt
-        let l:left_pad = col('$')
-        call prop_add(a:line, l:left_pad, {
-        \ 'type': 'ale',
-        \})
-        let s:last_virt = popup_create(l:msg, {
-        \ 'line': -1,
-        \ 'padding': [0, 0, 0, 1],
-        \ 'mask': [[1, 1, 1, 1]],
-        \ 'textprop': 'ale',
-        \ 'highlight': a:hl_group,
-        \ 'fixed': 1,
-        \ 'wrap': 0,
-        \ 'zindex': 2
-        \})
-    else
-        let l:type = prop_type_get(a:hl_group)
-
-        if l:type == {}
-            call prop_type_add(a:hl_group, {'highlight': a:hl_group})
-        endif
-
-        " Add highlight groups to the list so we can clear them later.
-        if index(s:hl_list, a:hl_group) == -1
-            call add(s:hl_list, a:hl_group)
-        endif
-
-        call prop_add(a:line, 0, {
-        \ 'type': a:hl_group,
-        \ 'text': ' ' . l:msg,
-        \ 'bufnr': a:buf
-        \})
-    endif
-endfunction
-
 function! s:StopCursorTimer() abort
     if s:cursor_timer != -1
         call timer_stop(s:cursor_timer)
@@ -120,17 +58,58 @@ function! s:StopCursorTimer() abort
     endif
 endfunction
 
-function! ale#virtualtext#GetHlGroup(type, sub_type) abort
+function! ale#virtualtext#ResetDataForTests() abort
-    if a:type is# 'E'
+    let s:last_pos = [0, 0, 0]
-        if a:sub_type is# 'style'
+    let s:last_message = ''
+endfunction
+
+function! ale#virtualtext#GetLastMessageForTests() abort
+    return s:last_message
+endfunction
+
+function! ale#virtualtext#GetComment(buffer) abort
+    let l:filetype = getbufvar(a:buffer, '&filetype')
+    let l:split = split(getbufvar(a:buffer, '&commentstring'), '%s')
+
+    return !empty(l:split) ? trim(l:split[0]) : '#'
+endfunction
+
+function! ale#virtualtext#Clear(buffer) abort
+    if !s:has_virt_text
+        return
+    endif
+
+    if has('nvim')
+        call nvim_buf_clear_namespace(a:buffer, s:ns_id, 0, -1)
+    else
+        if s:emulate_virt && s:last_virt != -1
+            call prop_remove({'type': 'ale'})
+            call popup_close(s:last_virt)
+            let s:last_virt = -1
+        elseif !empty(s:hl_list)
+            call prop_remove({
+            \   'types': s:hl_list,
+            \   'all': 1,
+            \   'bufnr': a:buffer,
+            \})
+        endif
+    endif
+endfunction
+
+function! ale#virtualtext#GetGroup(item) abort
+    let l:type = get(a:item, 'type', 'E')
+    let l:sub_type = get(a:item, 'sub_type', '')
+
+    if l:type is# 'E'
+        if l:sub_type is# 'style'
             return 'ALEVirtualTextStyleError'
         endif
 
         return 'ALEVirtualTextError'
     endif
 
-    if a:type is# 'W'
+    if l:type is# 'W'
-        if a:sub_type is# 'style'
+        if l:sub_type is# 'style'
             return 'ALEVirtualTextStyleWarning'
         endif
 
@@ -140,8 +119,67 @@ function! ale#virtualtext#GetHlGroup(type, sub_type) abort
     return 'ALEVirtualTextInfo'
 endfunction
 
+function! ale#virtualtext#ShowMessage(buffer, item) abort
+    if !s:has_virt_text || !bufexists(str2nr(a:buffer))
+        return
+    endif
+
+    let l:line = a:item.lnum
+    let l:hl_group = ale#virtualtext#GetGroup(a:item)
+
+    " Get a language-appropriate comment character, or default to '#'.
+    let l:comment = ale#virtualtext#GetComment(a:buffer)
+    let l:prefix = ale#Var(a:buffer, 'virtualtext_prefix')
+    let l:prefix = ale#GetLocItemMessage(a:item, l:prefix)
+    let l:prefix = substitute(l:prefix, '\V%comment%', '\=l:comment', 'g')
+    let l:msg = l:prefix . substitute(a:item.text, '\n', ' ', 'g')
+
+    " Store the last message we're going to set so we can read it in tests.
+    let s:last_message = l:msg
+
+    if has('nvim')
+        call nvim_buf_set_virtual_text(
+        \   a:buffer,
+        \   s:ns_id, l:line - 1,
+        \   [[l:msg, l:hl_group]],
+        \   {}
+        \)
+    elseif s:emulate_virt
+        let l:left_pad = col('$')
+        call prop_add(l:line, l:left_pad, {'type': 'ale'})
+        let s:last_virt = popup_create(l:msg, {
+        \   'line': -1,
+        \   'padding': [0, 0, 0, 1],
+        \   'mask': [[1, 1, 1, 1]],
+        \   'textprop': 'ale',
+        \   'highlight': l:hl_group,
+        \   'fixed': 1,
+        \   'wrap': 0,
+        \   'zindex': 2
+        \})
+    else
+        let l:type = prop_type_get(l:hl_group)
+
+        if l:type == {}
+            call prop_type_add(l:hl_group, {'highlight': l:hl_group})
+        endif
+
+        " Add highlight groups to the list so we can clear them later.
+        if index(s:hl_list, l:hl_group) == -1
+            call add(s:hl_list, l:hl_group)
+        endif
+
+        call prop_add(l:line, 0, {
+        \   'type': l:hl_group,
+        \   'text': ' ' . l:msg,
+        \   'bufnr': a:buffer,
+        \})
+    endif
+endfunction
+
 function! ale#virtualtext#ShowCursorWarning(...) abort
-    if g:ale_virtualtext_cursor isnot# 'current' && g:ale_virtualtext_cursor != 1
+    if g:ale_virtualtext_cursor isnot# 'current'
+    \&& g:ale_virtualtext_cursor != 1
         return
     endif
 
@@ -155,23 +193,19 @@ function! ale#virtualtext#ShowCursorWarning(...) abort
         return
     endif
 
-    let [l:info, l:loc] = ale#util#FindItemAtCursor(l:buffer)
+    let [l:info, l:item] = ale#util#FindItemAtCursor(l:buffer)
-
     call ale#virtualtext#Clear(l:buffer)
 
-    if !empty(l:loc)
+    if !empty(l:item)
-        let l:msg = l:loc.text
+        call ale#virtualtext#ShowMessage(l:buffer, l:item)
-        let l:type = get(l:loc, 'type', 'E')
-        let l:style = get(l:loc, 'sub_type', '')
-        let l:hl_group = ale#virtualtext#GetHlGroup(l:type, l:style)
-        call ale#virtualtext#ShowMessage(l:msg, l:hl_group, l:buffer, line('.'))
     endif
 endfunction
 
 function! ale#virtualtext#ShowCursorWarningWithDelay() abort
     let l:buffer = bufnr('')
 
-    if g:ale_virtualtext_cursor isnot# 'current' && g:ale_virtualtext_cursor != 1
+    if g:ale_virtualtext_cursor isnot# 'current'
+    \&& g:ale_virtualtext_cursor != 1
         return
     endif
 
@@ -198,19 +232,16 @@ function! ale#virtualtext#ShowCursorWarningWithDelay() abort
     endif
 endfunction
 
-function! ale#virtualtext#SetTexts(buf, loclist) abort
+function! ale#virtualtext#SetTexts(buffer, loclist) abort
     if !has('nvim') && s:emulate_virt
         return
     endif
 
-    call ale#virtualtext#Clear(a:buf)
+    call ale#virtualtext#Clear(a:buffer)
 
-    for l in a:loclist
+    for l:item in a:loclist
-        if l['bufnr'] != a:buf
+        if l:item.bufnr == a:buffer
-            continue
+            call ale#virtualtext#ShowMessage(a:buffer, l:item)
         endif
-
-        let l:hl = ale#virtualtext#GetHlGroup(l['type'], get(l, 'sub_type', ''))
-        call ale#virtualtext#ShowMessage(l['text'], l:hl, a:buf, l['lnum'])
     endfor
 endfunction

doc/ale-development.txt

@@ -155,7 +155,7 @@ ALE runs tests with the following versions of Vim in the following
 environments.
 
 1. Vim 8.0.0027 on Linux via GitHub Actions.
-2. Vim 9.0.0133 on Linux via GitHub Actions.
+2. Vim 9.0.0297 on Linux via GitHub Actions.
 3. NeoVim 0.2.0 on Linux via GitHub Actions.
 4. NeoVim 0.8.0 on Linux via GitHub Actions.
 6. Vim 8 (stable builds) on Windows via AppVeyor.

doc/ale.txt

@@ -1063,7 +1063,8 @@ g:ale_echo_msg_format                                   *g:ale_echo_msg_format*
     `%s`           - replaced with the text for the problem
     `%...code...% `- replaced with the error code
     `%linter%`     - replaced with the name of the linter
-    `%severity%`   - replaced with the severity of the problem
+    `%severity%`   - replaced with the severity of the problem (e.g. `Error`)
+    `%type%`       - replaced with the type of the problem (e.g. `E`)
 
   The strings for `%severity%` can be configured with the following options.
 
@@ -2314,7 +2315,6 @@ g:ale_virtualtext_cursor                             *g:ale_virtualtext_cursor*
 
 g:ale_virtualtext_delay                               *g:ale_virtualtext_delay*
                                                       *b:ale_virtualtext_delay*
-
   Type: |Number|
   Default: `10`
 
@@ -2326,12 +2326,24 @@ g:ale_virtualtext_delay                               *g:ale_virtualtext_delay*
 
 
 g:ale_virtualtext_prefix                             *g:ale_virtualtext_prefix*
-
+                                                     *b:ale_virtualtext_prefix*
   Type: |String|
-  Default: `'> '`
+  Default: `'%comment% %type%: '`
 
   Prefix to be used with |g:ale_virtualtext_cursor|.
 
+  This setting can be changed in each buffer with `b:ale_virtualtext_prefix`.
+
+  All of the same format markers used for |g:ale_echo_msg_format| can be used
+  for defining the prefix, including some additional sequences of characters.
+
+    `%comment%` - replaced with comment characters in the current language
+
+  ALE will read the comment characters from |&commentstring|, reading only the
+  part before `%s`, with whitespace trimmed. If comment syntax cannot be
+  pulled from |&commentstring|, ALE will default to `'#'`.
+
+
 g:ale_virtualenv_dir_names                         *g:ale_virtualenv_dir_names*
                                                    *b:ale_virtualenv_dir_names*
 
@@ -2508,7 +2520,7 @@ ALEStyleWarningSignLineNr                           *ALEStyleWarningSignLineNr*
 
 ALEVirtualTextError                                       *ALEVirtualTextError*
 
-  Default: `highlight link ALEVirtualTextError SpellBad`
+  Default: `highlight link ALEVirtualTextError Comment`
 
   The highlight for virtualtext errors. See |g:ale_virtualtext_cursor|.
 
@@ -2536,7 +2548,7 @@ ALEVirtualTextStyleWarning                         *ALEVirtualTextStyleWarning*
 
 ALEVirtualTextWarning                                   *ALEVirtualTextWarning*
 
-  Default: `highlight link ALEVirtualTextWarning SpellCap`
+  Default: `highlight link ALEVirtualTextWarning Comment`
 
   The highlight for virtualtext errors. See |g:ale_virtualtext_cursor|.
 

test/test_cursor_warnings.vader

@@ -129,7 +129,7 @@ After:
 Given javascript(A Javscript file with warnings/errors):
   var x = 3 + 12345678
   var x = 5*2 + parseInt("10");
-  // comment
+  //" comment
 
 Execute(Messages should be shown for the correct lines):
   call cursor(1, 1)
@@ -219,6 +219,26 @@ Execute(The severity should be formatted into the message correctly):
 
   AssertEqual 'Info: Some information', g:last_message
 
+Execute(The type should be formatted into the message correctly):
+  let g:ale_echo_msg_format = '%type%: %s'
+
+  call cursor(2, 9)
+  call ale#cursor#EchoCursorWarning()
+
+  AssertEqual
+  \ 'W: Infix operators must be spaced.',
+  \ g:last_message
+
+  call cursor(1, 10)
+  call ale#cursor#EchoCursorWarning()
+
+  AssertEqual 'E: Missing semicolon.', g:last_message
+
+  call cursor(1, 14)
+  call ale#cursor#EchoCursorWarning()
+
+  AssertEqual 'I: Some information', g:last_message
+
 Execute(The %code% and %ifcode% should show the code and some text):
   let g:ale_echo_msg_format = '%(code) %%s'
 

test/test_virtualtext.vader

@@ -0,0 +1,179 @@
+Before:
+  Save g:ale_buffer_info
+  Save g:ale_virtualtext_cursor
+  Save g:ale_virtualtext_delay
+  Save g:ale_virtualtext_prefix
+  Save b:ale_virtualtext_prefix
+
+  call ale#virtualtext#ResetDataForTests()
+
+  let g:setting = ''
+  let g:ale_virtualtext_delay = 0
+  let g:ale_buffer_info = {
+  \ bufnr(''): {
+  \   'loclist': [
+  \     {
+  \       'bufnr': bufnr(''),
+  \       'type': 'E',
+  \       'lnum': 1,
+  \       'col': 5,
+  \       'text': 'Line 1 error',
+  \     },
+  \     {
+  \       'bufnr': bufnr(''),
+  \       'type': 'W',
+  \       'lnum': 2,
+  \       'col': 1,
+  \       'text': 'Line 2 warning 1',
+  \     },
+  \     {
+  \       'bufnr': bufnr(''),
+  \       'type': 'W',
+  \       'lnum': 2,
+  \       'col': 5,
+  \       'text': 'Line 2 warning 2',
+  \     },
+  \   ],
+  \ },
+  \}
+
+After:
+  Restore
+
+  unlet! g:setting
+  unlet! g:ns_id
+
+Execute(The correct highlight groups should be loaded for virtual-text):
+  AssertEqual 'ALEVirtualTextError', ale#virtualtext#GetGroup({})
+  AssertEqual 'ALEVirtualTextError', ale#virtualtext#GetGroup({'type': 'E'})
+  AssertEqual 'ALEVirtualTextStyleError',
+  \ ale#virtualtext#GetGroup({'type': 'E', 'sub_type': 'style'})
+  AssertEqual 'ALEVirtualTextWarning', ale#virtualtext#GetGroup({'type': 'W'})
+  AssertEqual 'ALEVirtualTextStyleWarning',
+  \ ale#virtualtext#GetGroup({'type': 'W', 'sub_type': 'style'})
+  AssertEqual 'ALEVirtualTextInfo', ale#virtualtext#GetGroup({'type': 'I'})
+
+Given python (An empty Python file):
+Execute(Comment text should be detected correctly for Python files):
+  if has('patch-9.0.0297') || has('nvim-0.8.0')
+    AssertEqual '#', ale#virtualtext#GetComment(bufnr(''))
+  endif
+
+Given java (An empty Java file):
+Execute(Comment text should be detected correctly for Java files):
+  if has('patch-9.0.0297') || has('nvim-0.8.0')
+    AssertEqual '//', ale#virtualtext#GetComment(bufnr(''))
+  endif
+
+Given html (An empty HTML file):
+Execute(Comment text should be detected correctly for HTML files):
+  if has('patch-9.0.0297') || has('nvim-0.8.0')
+    AssertEqual "\<!--", ale#virtualtext#GetComment(bufnr(''))
+  endif
+
+Given python(An example Python file):
+  # line 1
+  # line 2
+
+Execute(We should not show virtualtext when disabled):
+  if has('patch-9.0.0297') || has('nvim-0.8.0')
+    for g:setting in ['disabled', '0', 0]
+      call ale#virtualtext#ResetDataForTests()
+
+      let g:ale_virtualtext_cursor = g:setting
+      call cursor(1, 1)
+      call ale#virtualtext#ShowCursorWarningWithDelay()
+      " Tick the timer.
+      sleep 1ms
+
+      AssertEqual '', ale#virtualtext#GetLastMessageForTests()
+    endfor
+  endif
+
+Execute(We should find a virtualtext error on line 1):
+  if has('patch-9.0.0297') || has('nvim-0.8.0')
+    for g:setting in ['current', '1', 1]
+      call ale#virtualtext#ResetDataForTests()
+
+      let g:ale_virtualtext_cursor = 'current'
+      call cursor(1, 1)
+      call ale#virtualtext#ShowCursorWarningWithDelay()
+      " Tick the timer.
+      sleep 1ms
+
+      AssertEqual '# E: Line 1 error', ale#virtualtext#GetLastMessageForTests()
+
+      if has('patch-9.0.0297')
+        AssertEqual ['ALEVirtualTextError'], map(prop_list(1), {_, v -> v.type})
+        AssertEqual [], prop_list(2)
+      endif
+    endfor
+  endif
+
+Execute(We should find a virtualtext error on line 2):
+  if has('patch-9.0.0297') || has('nvim-0.8.0')
+    let g:ale_virtualtext_cursor = 'current'
+    call cursor(2, 5)
+    call ale#virtualtext#ShowCursorWarningWithDelay()
+    " Tick the timer.
+    sleep 1ms
+
+    AssertEqual '# W: Line 2 warning 2', ale#virtualtext#GetLastMessageForTests()
+
+    if has('patch-9.0.0297')
+      AssertEqual [], prop_list(1)
+      AssertEqual ['ALEVirtualTextWarning'], map(prop_list(2), {_, v -> v.type})
+    endif
+  endif
+
+Execute(We should be able to change the virtualtext prefix globally):
+  let g:ale_virtualtext_prefix = '> '
+
+  if has('patch-9.0.0297') || has('nvim-0.8.0')
+    let g:ale_virtualtext_cursor = 'current'
+    call cursor(1, 1)
+    call ale#virtualtext#ShowCursorWarningWithDelay()
+    " Tick the timer.
+    sleep 1ms
+
+    AssertEqual '> Line 1 error', ale#virtualtext#GetLastMessageForTests()
+  endif
+
+Execute(We should be able to change the virtualtext prefix per-buffer):
+  let b:ale_virtualtext_prefix = 'B> '
+
+  if has('patch-9.0.0297') || has('nvim-0.8.0')
+    let g:ale_virtualtext_cursor = 'current'
+    call cursor(1, 1)
+    call ale#virtualtext#ShowCursorWarningWithDelay()
+    " Tick the timer.
+    sleep 1ms
+
+    AssertEqual 'B> Line 1 error', ale#virtualtext#GetLastMessageForTests()
+  endif
+
+Execute(We should be format in other data from the loclist items):
+  let g:ale_virtualtext_prefix = '%severity%: '
+
+  if has('patch-9.0.0297') || has('nvim-0.8.0')
+    let g:ale_virtualtext_cursor = 'current'
+    call cursor(1, 1)
+    call ale#virtualtext#ShowCursorWarningWithDelay()
+    " Tick the timer.
+    sleep 1ms
+
+    AssertEqual 'Error: Line 1 error', ale#virtualtext#GetLastMessageForTests()
+  endif
+
+Execute(We should set errors across all lines):
+  if has('patch-9.0.0297') || has('nvim-0.8.0')
+    call ale#virtualtext#SetTexts(bufnr(''), g:ale_buffer_info[bufnr('')].loclist)
+
+    AssertEqual '# W: Line 2 warning 2', ale#virtualtext#GetLastMessageForTests()
+
+    if has('patch-9.0.0297')
+      AssertEqual ['ALEVirtualTextError'], map(prop_list(1), {_, v -> v.type})
+      AssertEqual ['ALEVirtualTextWarning', 'ALEVirtualTextWarning'],
+      \ map(prop_list(2), {_, v -> v.type})
+    endif
+  endif