Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement buffer-wide virtual text support #4289

Merged
merged 2 commits into from
Sep 7, 2022

Conversation

vimpostor
Copy link
Contributor

@vimpostor vimpostor commented Aug 23, 2022

This implements buffer-wide virtual text as detailed in #2962, both for vim and neovim.

You can activate it with let g:ale_virtualtext_cursor = 2.

Note that sometimes there is a problem, where very similar warnings appear
twice as virtual text, but this is not a bug in the implementation, rather it's
a problem of using multiple linters that return very similar warnings.

Demo

Screenshot_20220823_204435

@vimpostor vimpostor changed the title Allow virtual text to appear for all warnings of the buffer Implement buffer-wide virtual text support Aug 23, 2022
Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can verify this works in Neovim 0.7 and does not break anything as far as I can see. Again would be better if someone (not the PR author) can check this works fine in their environment using Vim.

@RyanSquared
Copy link
Member

Error detected while processing function ale#virtualtext#ShowCursorWarning[17]..ale#virtualtext#Clear:
line   12:
E121: Undefined variable: s:hl_list
Error detected while processing function ale#virtualtext#ShowCursorWarning[17]..ale#virtualtext#Clear:
line   12:
E116: Invalid arguments for function empty(s:hl_list)

Using: VIM - Vi IMproved 8.2 (2019 Dec 12, compiled Apr 18 2022 19:26:30)

@RyanSquared
Copy link
Member

I believe this happens because s:hl_list is only set when s:emulate_virt is not set, but the following is run as a test: if s:emulate_virt && s:last_virt != -1. When opening a file, s:emulate_virt && s:last_virt != -1 is not true, so it goes to the second expression, which relies on s:hl_list. This can be fixed by checking !s:emulate_virt && !empty(s:hl_list), as it will skip the second half of the expression if the first half fails.

@RyanSquared
Copy link
Member

This also completely breaks all virtualtext for Vim using emulated virtual text. I am unsure if the buffer-wide virtualtext should be supported or not, but I would prefer it to not break vim 8.x setups.

@vimpostor
Copy link
Contributor Author

vimpostor commented Aug 24, 2022

This also completely breaks all virtualtext for Vim using emulated virtual text. I am unsure if the buffer-wide virtualtext should be supported or not, but I would prefer it to not break vim 8.x setups.

Yeah, sorry I only tested with vim 9 and neovim, will fix that asap

I originally intended to remove the vim 8.2 compatibility virtual text, because it is a total hack and buggy on its own, but I guess we can leave it for now.
FYI buffer-wide virtual text will require vim 9 (or neovim)

@WhiteBlackGoose
Copy link

WhiteBlackGoose commented Aug 31, 2022

Again would be better if someone (not the PR author) can check this works fine in their environment using Vim.

Works fine! Thank you all, people who make it!

image

info:

NVIM v0.7.0
Build type: Release
LuaJIT 2.1.0-beta3
Compiled by runner@fv-az316-460
ALEInfo

 Current Filetype: cs
Available Linters: ['csc', 'cspell', 'mcs', 'mcsc']
  Enabled Linters: []
  Ignored Linters: []
 Suggested Fixers: 
  'dotnet-format' - Fix C# files with dotnet format.
  'remove_trailing_lines' - Remove all blank lines at the end of a file.
  'trim_whitespace' - Remove all trailing whitespace characters at the end of every line.
  'uncrustify' - Fix C, C++, C#, ObjectiveC, ObjectiveC++, D, Java, Pawn, and VALA files with uncrustify.
 Linter Variables:

 Global Variables:

let g:ale_cache_executable_check_failures = v:null
let g:ale_change_sign_column_color = 0
let g:ale_command_wrapper = v:null
let g:ale_completion_delay = v:null
let g:ale_completion_enabled = 0
let g:ale_completion_max_suggestions = v:null
let g:ale_disable_lsp = 0
let g:ale_echo_cursor = 1
let g:ale_echo_msg_error_str = 'Error'
let g:ale_echo_msg_format = '[%linter%] %s'
let g:ale_echo_msg_info_str = 'Info'
let g:ale_echo_msg_warning_str = 'Warning'
let g:ale_enabled = 1
let g:ale_fix_on_save = 0
let g:ale_fixers = {}
let g:ale_history_enabled = 1
let g:ale_history_log_output = 1
let g:ale_keep_list_window_open = 0
let g:ale_lint_delay = 200
let g:ale_lint_on_enter = 1
let g:ale_lint_on_filetype_changed = 1
let g:ale_lint_on_insert_leave = 1
let g:ale_lint_on_save = 1
let g:ale_lint_on_text_changed = 'normal'
let g:ale_linter_aliases = {}
let g:ale_linters = {}
let b:ale_linters = ['code_checker']
let g:ale_linters_explicit = 0
let g:ale_linters_ignore = {}
let g:ale_list_vertical = 0
let g:ale_list_window_size = 10
let g:ale_loclist_msg_format = '[%linter%] %s'
let g:ale_max_buffer_history_size = v:null
let g:ale_max_signs = -1
let g:ale_maximum_file_size = v:null
let g:ale_open_list = 0
let g:ale_pattern_options = v:null
let g:ale_pattern_options_enabled = v:null
let g:ale_root = {}
let g:ale_set_balloons = 0
let g:ale_set_highlights = 1
let g:ale_set_loclist = 1
let g:ale_set_quickfix = 0
let g:ale_set_signs = 1
let g:ale_sign_column_always = 0
let g:ale_sign_error = ' '
let g:ale_sign_info = ' '
let g:ale_sign_offset = 1000000
let g:ale_sign_style_error = ' '
let g:ale_sign_style_warning = ' '
let g:ale_sign_warning = ' '
let g:ale_sign_highlight_linenrs = 0
let g:ale_statusline_format = v:null
let g:ale_type_map = {}
let g:ale_use_global_executables = v:null
let g:ale_virtualtext_cursor = 2
let g:ale_warn_about_trailing_blank_lines = 1
let g:ale_warn_about_trailing_whitespace = 1
  Command History:

@jeffdn
Copy link

jeffdn commented Sep 2, 2022

This works great for me:

image

ViM v9.0.350

hsanson
hsanson previously approved these changes Sep 6, 2022
Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution and to those that verified it working.

@hsanson
Copy link
Contributor

hsanson commented Sep 6, 2022

@vimpostor there is a merge conflict. Appreciated if you can take a look into it.

This is more robust and has the additional sideeffect that it will make
it easier to implement showing virtual text for all warnings
simultaneously.

We definitely do not want to do a call to prop_remove() for every
virtual text as that will cause noticeable lag when many warnings are
present, thus we can use this to remove all virtual text lines with one
call in the future.

Fixes dense-analysis#4294

refs: vim/vim#10945
This can be enabled with:

let g:ale_virtualtext_cursor = 2

It is implemented both for neovim and vim 9.0.0297.

Note that sometimes it may appear like some warnings are displayed
multiple times. This is not a bug in the virtual text implementation,
but a sideeffect of multiple linters returning similar results.

For example for Rust, the 'cargo' and 'rls' linters appear to be
activated at the same time, but they sometimes return identical errors.
This causes the virtual text to show the same warning twice.

In the future we can mitigate this problem by removing duplicate errors
from our internal location list.

However users can also achieve cleaner warnings simply by activating
only one linter for each language (or multiple unambiguous linters).

For example for Rust, the problem could be solved with:

let g:ale_linters = {'rust': ['analyzer']}

Fixes dense-analysis#2962
Fixes dense-analysis#3666
@vimpostor
Copy link
Contributor Author

vimpostor commented Sep 6, 2022 via email

@hsanson hsanson merged commit 9feba11 into dense-analysis:master Sep 7, 2022
@WhiteBlackGoose
Copy link

Thanks!

mnikulin pushed a commit to mnikulin/ale that referenced this pull request Nov 12, 2023
* Remove virtual text via types-filter

This is more robust and has the additional sideeffect that it will make
it easier to implement showing virtual text for all warnings
simultaneously.

We definitely do not want to do a call to prop_remove() for every
virtual text as that will cause noticeable lag when many warnings are
present, thus we can use this to remove all virtual text lines with one
call in the future.

Fixes dense-analysis#4294

refs: vim/vim#10945

* Allow virtual text to appear for all warnings of the buffer

This can be enabled with:

let g:ale_virtualtext_cursor = 2

It is implemented both for neovim and vim 9.0.0297.

Note that sometimes it may appear like some warnings are displayed
multiple times. This is not a bug in the virtual text implementation,
but a sideeffect of multiple linters returning similar results.

For example for Rust, the 'cargo' and 'rls' linters appear to be
activated at the same time, but they sometimes return identical errors.
This causes the virtual text to show the same warning twice.

In the future we can mitigate this problem by removing duplicate errors
from our internal location list.

However users can also achieve cleaner warnings simply by activating
only one linter for each language (or multiple unambiguous linters).

For example for Rust, the problem could be solved with:

let g:ale_linters = {'rust': ['analyzer']}

Fixes dense-analysis#2962
Fixes dense-analysis#3666
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants