From a18472cc58b85ba4e037fe3ad4c3f525aef74a85 Mon Sep 17 00:00:00 2001 From: w0rp Date: Tue, 27 Dec 2022 23:11:53 +0000 Subject: 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 | 2 +- autoload/ale.vim | 1 + autoload/ale/virtualtext.vim | 183 +++++++++++++++++++++++----------------- doc/ale-development.txt | 2 +- doc/ale.txt | 24 ++++-- test/test_cursor_warnings.vader | 22 ++++- test/test_virtualtext.vader | 179 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 328 insertions(+), 85 deletions(-) create mode 100644 test/test_virtualtext.vader diff --git a/Dockerfile b/Dockerfile index b7b42a3f..2985ecc5 100644 --- a/Dockerfile +++ b/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 diff --git a/autoload/ale.vim b/autoload/ale.vim index 519cb5fa..23315913 100644 --- a/autoload/ale.vim +++ b/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') diff --git a/autoload/ale/virtualtext.vim b/autoload/ale/virtualtext.vim index 6f4e500f..59c69b78 100644 --- a/autoload/ale/virtualtext.vim +++ b/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,13 +51,36 @@ if !has_key(s:, 'has_virt_text') endif endif -function! ale#virtualtext#Clear(buf) abort +function! s:StopCursorTimer() abort + if s:cursor_timer != -1 + call timer_stop(s:cursor_timer) + let s:cursor_timer = -1 + endif +endfunction + +function! ale#virtualtext#ResetDataForTests() abort + let s:last_pos = [0, 0, 0] + 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:buf, s:ns_id, 0, -1) + 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'}) @@ -61,87 +88,98 @@ function! ale#virtualtext#Clear(buf) abort let s:last_virt = -1 elseif !empty(s:hl_list) call prop_remove({ - \ 'types': s:hl_list, - \ 'all': 1, - \ 'bufnr': a:buf}) + \ 'types': s:hl_list, + \ 'all': 1, + \ 'bufnr': a:buffer, + \}) endif endif endfunction -function! ale#virtualtext#ShowMessage(message, hl_group, buf, line) abort - if !s:has_virt_text || !bufexists(str2nr(a:buf)) +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 l:type is# 'W' + if l:sub_type is# 'style' + return 'ALEVirtualTextStyleWarning' + endif + + return 'ALEVirtualTextWarning' + endif + + return 'ALEVirtualTextInfo' +endfunction + +function! ale#virtualtext#ShowMessage(buffer, item) abort + if !s:has_virt_text || !bufexists(str2nr(a:buffer)) return endif - let l:prefix = get(g:, 'ale_virtualtext_prefix', '> ') - let l:msg = l:prefix.trim(substitute(a:message, '\n', ' ', 'g')) + 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:buf, s:ns_id, a:line-1, [[l:msg, a:hl_group]], {}) + 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(a:line, l:left_pad, { - \ 'type': 'ale', - \}) + 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': a:hl_group, - \ 'fixed': 1, - \ 'wrap': 0, - \ 'zindex': 2 + \ '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(a:hl_group) + let l:type = prop_type_get(l:hl_group) if l:type == {} - call prop_type_add(a:hl_group, {'highlight': a:hl_group}) + 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, a:hl_group) == -1 - call add(s:hl_list, a:hl_group) + if index(s:hl_list, l:hl_group) == -1 + call add(s:hl_list, l:hl_group) endif - call prop_add(a:line, 0, { - \ 'type': a:hl_group, - \ 'text': ' ' . l:msg, - \ 'bufnr': a:buf + call prop_add(l:line, 0, { + \ 'type': l:hl_group, + \ 'text': ' ' . l:msg, + \ 'bufnr': a:buffer, \}) endif endfunction -function! s:StopCursorTimer() abort - if s:cursor_timer != -1 - call timer_stop(s:cursor_timer) - let s:cursor_timer = -1 - endif -endfunction - -function! ale#virtualtext#GetHlGroup(type, sub_type) abort - if a:type is# 'E' - if a:sub_type is# 'style' - return 'ALEVirtualTextStyleError' - endif - - return 'ALEVirtualTextError' - endif - - if a:type is# 'W' - if a:sub_type is# 'style' - return 'ALEVirtualTextStyleWarning' - endif - - return 'ALEVirtualTextWarning' - endif - - return 'ALEVirtualTextInfo' -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) - let l:msg = l:loc.text - 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('.')) + if !empty(l:item) + call ale#virtualtext#ShowMessage(l:buffer, l:item) 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 - if l['bufnr'] != a:buf - continue + for l:item in a:loclist + if l:item.bufnr == a:buffer + 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 diff --git a/doc/ale-development.txt b/doc/ale-development.txt index 946c49a1..6bc009fd 100644 --- a/doc/ale-development.txt +++ b/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. diff --git a/doc/ale.txt b/doc/ale.txt index ebc5489c..949ceae0 100644 --- a/doc/ale.txt +++ b/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|. diff --git a/test/test_cursor_warnings.vader b/test/test_cursor_warnings.vader index b767d225..7aac7740 100644 --- a/test/test_cursor_warnings.vader +++ b/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' diff --git a/test/test_virtualtext.vader b/test/test_virtualtext.vader new file mode 100644 index 00000000..8fc1ead5 --- /dev/null +++ b/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 "\