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: (prototype) add support for vim diagnostics #2020

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ollien
Copy link
Collaborator

@ollien ollien commented May 27, 2024

Relates to #1993

This is a prototype I threw together while my flight was delayed... I wouldn't review it too closely, but if you have any broad thoughts about approach, I'd love to hear them!

The virtual text for overlays is ... not great. I suspect there's some bug in how we draw these, but investigating that hasn't been my highest priority.

image

@ollien
Copy link
Collaborator Author

ollien commented May 27, 2024

Also, this is not really related to the PR, but I think I'm doing something wrong. lua vim.diagnostic.goto_prev({severity=vim.diagnostic.severity.ERROR, float=false}) works, but nnoremap [e lua vim.diagnostic.goto_prev({severity=vim.diagnostic.severity.ERROR, float=false}) then pressing [e prints "Already at oldest change"...? Not sure why that is, doesn't quite make sense to me

@justinmk
Copy link
Collaborator

Not in favor. This is non-trivial code that will grow, and need to be debugged, and currently the justification is that vscode doesn't have a mapping that navigates by severity...?

@ollien
Copy link
Collaborator Author

ollien commented May 27, 2024

I don't think it's that complex, but I do understand your hesitation. There's already more loose ends popping up as I work...

@xiyaowong
Copy link
Collaborator

The virtual text for overlays is ... not great. I suspect there's some bug in how we draw these, but investigating that hasn't been my highest priority.

We absolutely only need to implement the navigation feature.

@ollien
Copy link
Collaborator Author

ollien commented May 28, 2024

The virtual text for overlays is ... not great. I suspect there's some bug in how we draw these, but investigating that hasn't been my highest priority.

We absolutely only need to implement the navigation feature.

Agreed. I don't want these overlays here necessarily, it's just that it may be informing of us of another bug with virtual text.

@xiyaowong

This comment was marked as outdated.

@justinmk
Copy link
Collaborator

I have a use case. I set up the following mappings to navigate errors,

Mappings were also the use-case for #2045 . The question is whether it is worth the risk of adding a lot of code, for a few mappings.

A future version of vscode may gain the behavior you want, but then it's unlikely that we will know when we can untangle all of this extra code.

@xiyaowong
Copy link
Collaborator

xiyaowong commented Jul 11, 2024

A future version of vscode may gain the behavior you want, but then it's unlikely that we will know when we can untangle all of this extra code.

I didn't look carefully before; these commands have been available for a long time. editor.action.marker.next editor.action.marker.prev

@ollien
Copy link
Collaborator Author

ollien commented Jul 11, 2024

A future version of vscode may gain the behavior you want, but then it's unlikely that we will know when we can untangle all of this extra code.

I didn't look carefully before; these commands have been available for a long time. editor.action.marker.next editor.action.marker.prev

We've discussed this previously (though I admit I can't find the conversation). Unfortunately this doesn't let you navigate by severity

@xiyaowong
Copy link
Collaborator

I don't have this need. However, I have no objections to this PR. It's just a habitual update of outdated content.

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

3 participants