From ee41153a945876ad0c7f0927ffa7b5a5afdaca89 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Tue, 30 Apr 2024 09:57:31 +0100 Subject: [PATCH] feat(diagnostic): revert default behaviour of goto_next/prev() Follow-up to #28490 Problem: The new behaviour of goto_next/prev() of navigating to the next highest severity doesn't work well when diagnostic providers have different interpretations of severities. E.g. the user may be blocked from navigating to a useful LSP warning, due to some linter error. Solution: The behaviour of next highest severity is now a hidden option `_highest = true`. We can revisit how to integrate this behaviour during the 0.11 cycle. --- runtime/doc/diagnostic.txt | 7 ++- runtime/doc/news.txt | 5 --- runtime/lua/vim/diagnostic.lua | 59 +++++++++++++++---------- test/functional/lua/diagnostic_spec.lua | 14 +++--- 4 files changed, 45 insertions(+), 40 deletions(-) diff --git a/runtime/doc/diagnostic.txt b/runtime/doc/diagnostic.txt index 4958dcfe67..ac10b4ea39 100644 --- a/runtime/doc/diagnostic.txt +++ b/runtime/doc/diagnostic.txt @@ -389,9 +389,8 @@ Lua module: vim.diagnostic *diagnostic-api* |nvim_win_get_cursor()|. • {wrap}? (`boolean`, default: `true`) Whether to loop around file or not. Similar to 'wrapscan'. - • {severity}? (`vim.diagnostic.Severity`) See - |diagnostic-severity|. If `nil`, go to the - diagnostic with the highest severity. + • {severity}? (`vim.diagnostic.SeverityFilter`) See + |diagnostic-severity|. • {float}? (`boolean|vim.diagnostic.Opts.Float`, default: `true`) If `true`, call |vim.diagnostic.open_float()| after moving. If a @@ -435,7 +434,7 @@ Lua module: vim.diagnostic *diagnostic-api* • {update_in_insert}? (`boolean`, default: `false`) Update diagnostics in Insert mode (if `false`, diagnostics are updated on |InsertLeave|) - • {severity_sort}? (`boolean|{reverse?:boolean}`, default: `false) + • {severity_sort}? (`boolean|{reverse?:boolean}`, default: `false`) Sort diagnostics by severity. This affects the order in which signs and virtual text are displayed. When true, higher severities are diff --git a/runtime/doc/news.txt b/runtime/doc/news.txt index 57f0f1d6d4..bae0030a14 100644 --- a/runtime/doc/news.txt +++ b/runtime/doc/news.txt @@ -140,11 +140,6 @@ The following changes may require adaptations in user config or plugins. • |nvim_open_win()| now blocks all autocommands when `noautocmd` is set, rather than just those from setting the `buffer` to display in the window. -• |vim.diagnostic.goto_next()| and |vim.diagnostic.goto_prev()| jump to the - diagnostic with the highest severity when the "severity" option is - unspecified. To use the old behavior, use: >lua - vim.diagnostic.goto_next({ severity = { min = vim.diagnostic.severity.HINT } }) - ============================================================================== BREAKING CHANGES IN HEAD *news-breaking-dev* diff --git a/runtime/lua/vim/diagnostic.lua b/runtime/lua/vim/diagnostic.lua index b34541c72e..67ce331891 100644 --- a/runtime/lua/vim/diagnostic.lua +++ b/runtime/lua/vim/diagnostic.lua @@ -76,7 +76,7 @@ local M = {} --- before lower severities (e.g. ERROR is displayed before WARN). --- Options: --- - {reverse}? (boolean) Reverse sort order ---- (default: `false) +--- (default: `false`) --- @field severity_sort? boolean|{reverse?:boolean} --- @class (private) vim.diagnostic.OptsResolved @@ -812,6 +812,29 @@ local function set_list(loclist, opts) end end +--- Jump to the diagnostic with the highest severity. First sort the +--- diagnostics by severity. The first diagnostic then contains the highest severity, and we can +--- discard all diagnostics with a lower severity. +--- @param diagnostics vim.Diagnostic[] +local function filter_highest(diagnostics) + table.sort(diagnostics, function(a, b) + return a.severity < b.severity + end) + + -- Find the first diagnostic where the severity does not match the highest severity, and remove + -- that element and all subsequent elements from the array + local worst = (diagnostics[1] or {}).severity + local len = #diagnostics + for i = 2, len do + if diagnostics[i].severity ~= worst then + for j = i, len do + diagnostics[j] = nil + end + break + end + end +end + --- @param position {[1]: integer, [2]: integer} --- @param search_forward boolean --- @param bufnr integer @@ -823,29 +846,13 @@ local function next_diagnostic(position, search_forward, bufnr, opts, namespace) bufnr = get_bufnr(bufnr) local wrap = if_nil(opts.wrap, true) - local diagnostics = - get_diagnostics(bufnr, vim.tbl_extend('keep', opts, { namespace = namespace }), true) + local get_opts = vim.deepcopy(opts) + get_opts.namespace = get_opts.namespace or namespace - -- When severity is unset we jump to the diagnostic with the highest severity. First sort the - -- diagnostics by severity. The first diagnostic then contains the highest severity, and we can - -- discard all diagnostics with a lower severity. - if opts.severity == nil then - table.sort(diagnostics, function(a, b) - return a.severity < b.severity - end) + local diagnostics = get_diagnostics(bufnr, get_opts, true) - -- Find the first diagnostic where the severity does not match the highest severity, and remove - -- that element and all subsequent elements from the array - local worst = (diagnostics[1] or {}).severity - local len = #diagnostics - for i = 2, len do - if diagnostics[i].severity ~= worst then - for j = i, len do - diagnostics[j] = nil - end - break - end - end + if opts._highest then + filter_highest(diagnostics) end local line_diagnostics = diagnostic_lines(diagnostics) @@ -1191,8 +1198,12 @@ end --- (default: `true`) --- @field wrap? boolean --- ---- See |diagnostic-severity|. If `nil`, go to the diagnostic with the highest severity. ---- @field severity? vim.diagnostic.Severity +--- See |diagnostic-severity|. +--- @field severity? vim.diagnostic.SeverityFilter +--- +--- Go to the diagnostic with the highest severity. +--- (default: `false`) +--- @field package _highest? boolean --- --- If `true`, call |vim.diagnostic.open_float()| after moving. --- If a table, pass the table as the {opts} parameter to |vim.diagnostic.open_float()|. diff --git a/test/functional/lua/diagnostic_spec.lua b/test/functional/lua/diagnostic_spec.lua index 2e6f7fbf36..05082bc132 100644 --- a/test/functional/lua/diagnostic_spec.lua +++ b/test/functional/lua/diagnostic_spec.lua @@ -962,7 +962,7 @@ describe('vim.diagnostic', function() eq( { 3, 0 }, exec_lua([[ - vim.diagnostic.goto_next() + vim.diagnostic.goto_next({_highest = true}) return vim.api.nvim_win_get_cursor(0) ]]) ) @@ -970,7 +970,7 @@ describe('vim.diagnostic', function() eq( { 5, 0 }, exec_lua([[ - vim.diagnostic.goto_next() + vim.diagnostic.goto_next({_highest = true}) return vim.api.nvim_win_get_cursor(0) ]]) ) @@ -991,7 +991,7 @@ describe('vim.diagnostic', function() eq( { 4, 0 }, exec_lua([[ - vim.diagnostic.goto_next() + vim.diagnostic.goto_next({_highest = true}) return vim.api.nvim_win_get_cursor(0) ]]) ) @@ -999,7 +999,7 @@ describe('vim.diagnostic', function() eq( { 6, 0 }, exec_lua([[ - vim.diagnostic.goto_next() + vim.diagnostic.goto_next({_highest = true}) return vim.api.nvim_win_get_cursor(0) ]]) ) @@ -1021,7 +1021,7 @@ describe('vim.diagnostic', function() eq( { 2, 0 }, exec_lua([[ - vim.diagnostic.goto_next({ severity = { min = vim.diagnostic.severity.HINT } }) + vim.diagnostic.goto_next() return vim.api.nvim_win_get_cursor(0) ]]) ) @@ -1029,7 +1029,7 @@ describe('vim.diagnostic', function() eq( { 3, 0 }, exec_lua([[ - vim.diagnostic.goto_next({ severity = { min = vim.diagnostic.severity.HINT } }) + vim.diagnostic.goto_next() return vim.api.nvim_win_get_cursor(0) ]]) ) @@ -1037,7 +1037,7 @@ describe('vim.diagnostic', function() eq( { 4, 0 }, exec_lua([[ - vim.diagnostic.goto_next({ severity = { min = vim.diagnostic.severity.HINT } }) + vim.diagnostic.goto_next() return vim.api.nvim_win_get_cursor(0) ]]) )