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

feat(diagnostic): ansi code colored disagnostics in floating and spli… #456

Merged
merged 7 commits into from
Jul 26, 2024

Conversation

xzbdmw
Copy link
Contributor

@xzbdmw xzbdmw commented Jul 23, 2024

close #442
float:
截屏2024-07-24 06 01 02

split:
截屏2024-07-24 06 01 09

The idea is using nvim_open_term to let nvim render ansi code diagnostic for us, there is one problem, the window size must bigger than both of diagnostic's width and content, if not, previous lines get cleared by nvim somehow(idk why) like this image, notice the error: is missing.

截屏2024-07-24 05 31 47

You can try my first commit and set width to a small number say 20.
I don't use rust often lately, hope it works well😸

Copy link
Contributor

Review Checklist

Does this PR follow the Contribution Guidelines? Following is a partial checklist:

Proper conventional commit scoping:

  • For example, fix(lsp): some lsp-related bugfix

  • Pull request title has the appropriate conventional commit prefix.

If applicable:

  • Tested
    • Tests have been added.
    • Tested manually (Steps to reproduce in PR description).
  • Updated documentation.
  • Updated CHANGELOG.md

---@param rendered_diagnostic string
local function render_ansi_code_diagnostic(rendered_diagnostic)
local lines =
vim.split(rendered_diagnostic:gsub('[\27\155][][()#;?%d]*[A-PRZcf-ntqry=><~]', ''), '\n', { trimempty = true })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Owner

Choose a reason for hiding this comment

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

suggestion: Add this info as a comment

Suggested change
vim.split(rendered_diagnostic:gsub('[\27\155][][()#;?%d]*[A-PRZcf-ntqry=><~]', ''), '\n', { trimempty = true })
-- adopted from https://stackoverflow.com/questions/48948630/lua-ansi-escapes-pattern
vim.split(rendered_diagnostic:gsub('[\27\155][][()#;?%d]*[A-PRZcf-ntqry=><~]', ''), '\n', { trimempty = true })

Comment on lines +314 to +304
vim.api.nvim_feedkeys(
vim.api.nvim_replace_termcodes(
'<cmd>lua vim.api.nvim_set_current_win('
.. winnr
.. ')<CR>'
.. [[<c-\><c-n>]]
.. '<cmd>lua vim.api.nvim_win_set_cursor('
.. winnr
.. ',{1,0})<CR>',
true,
false,
true
),
'n',
true
)
Copy link
Contributor Author

@xzbdmw xzbdmw Jul 23, 2024

Choose a reason for hiding this comment

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

This is to ensure cursor syncs properly, nvim_win_set_cursor is not a synchronized call, when cursor first enters the window, Terminal mode is activated, and the cursor is at last line.

@mrcjkb
Copy link
Owner

mrcjkb commented Jul 23, 2024

Hey 👋

Thanks for the PR 🙏

The screenshots look awesome! I'll take a closer look later this week.

Copy link
Owner

@mrcjkb mrcjkb left a comment

Choose a reason for hiding this comment

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

Great work! 🚀

Just a few comments.

@@ -234,6 +240,106 @@ local function get_rendered_diagnostic(diagnostic)
end
end

local function validate_window_dimensions(float_preview_lines, user_float_win_config)
Copy link
Owner

Choose a reason for hiding this comment

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

issue: missing type annotations.
[see my other comment before addressing this]

Suggested change
local function validate_window_dimensions(float_preview_lines, user_float_win_config)
---@param float_preview_lines string[]
---@param user_float_win_config table
---@return table
local function validate_window_dimensions(float_preview_lines, user_float_win_config)

c[key] = nil
end

local width, height = vim.lsp.util._make_floating_popup_size(float_preview_lines, c)
Copy link
Owner

Choose a reason for hiding this comment

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

issue: This is a private function, not part of Neovim's public API.

I think we can live without validate_window_dimensions. Most users won't set dimensions in their configs and by the looks of it, vim.lsp.util.open_floating_preview determines the correct size.

Although users setting boundaries in their configs could lead to the content not being able to fit, I don't think silently correcting max_width or max_height is desirable behaviour, as it could leave users confused about why their boundaries aren't being respected.

---@param rendered_diagnostic string
local function render_ansi_code_diagnostic(rendered_diagnostic)
local lines =
vim.split(rendered_diagnostic:gsub('[\27\155][][()#;?%d]*[A-PRZcf-ntqry=><~]', ''), '\n', { trimempty = true })
Copy link
Owner

Choose a reason for hiding this comment

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

suggestion: Add this info as a comment

Suggested change
vim.split(rendered_diagnostic:gsub('[\27\155][][()#;?%d]*[A-PRZcf-ntqry=><~]', ''), '\n', { trimempty = true })
-- adopted from https://stackoverflow.com/questions/48948630/lua-ansi-escapes-pattern
vim.split(rendered_diagnostic:gsub('[\27\155][][()#;?%d]*[A-PRZcf-ntqry=><~]', ''), '\n', { trimempty = true })

Comment on lines 290 to 276
local autocmd_id = vim.api.nvim_create_autocmd('WinEnter', {
callback = function(args)
if args.buf == bufnr then
vim.api.nvim_feedkeys(
vim.api.nvim_replace_termcodes(
[[<c-\><c-n>]] .. '<cmd>lua vim.api.nvim_win_set_cursor(' .. winnr .. ',{1,0})<CR>',
true,
false,
true
),
'n',
true
)
end
end,
})
Copy link
Owner

Choose a reason for hiding this comment

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

suggestion: User buffer-local autocommand instead of checking for args.buf.

Suggested change
local autocmd_id = vim.api.nvim_create_autocmd('WinEnter', {
callback = function(args)
if args.buf == bufnr then
vim.api.nvim_feedkeys(
vim.api.nvim_replace_termcodes(
[[<c-\><c-n>]] .. '<cmd>lua vim.api.nvim_win_set_cursor(' .. winnr .. ',{1,0})<CR>',
true,
false,
true
),
'n',
true
)
end
end,
})
local autocmd_id = vim.api.nvim_create_autocmd('WinEnter', {
callback = function()
vim.api.nvim_feedkeys(
vim.api.nvim_replace_termcodes(
[[<c-\><c-n>]] .. '<cmd>lua vim.api.nvim_win_set_cursor(' .. winnr .. ',{1,0})<CR>',
true,
false,
true
),
'n',
true
)
end,
buffer = bufnr,
})

Comment on lines 307 to 313
vim.api.nvim_create_autocmd('WinClosed', {
once = true,
pattern = tostring(winnr),
callback = function()
vim.api.nvim_del_autocmd(autocmd_id)
end,
})
Copy link
Owner

Choose a reason for hiding this comment

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

suggestion: If using a buffer-local autocommand for the WinEnter event, this one can probably be removed.

Suggested change
vim.api.nvim_create_autocmd('WinClosed', {
once = true,
pattern = tostring(winnr),
callback = function()
vim.api.nvim_del_autocmd(autocmd_id)
end,
})

@xzbdmw
Copy link
Contributor Author

xzbdmw commented Jul 26, 2024

@mrcjkb Thank you for detailed review! have made all the suggested changes. and rebased with master.

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.

Support colored diagnostic in popup window
2 participants