helix icon indicating copy to clipboard operation
helix copied to clipboard

implement `goto_[first|prev|next|last]_error`

Open cgahr opened this issue 9 months ago • 4 comments

This PR implements the commands goto_[first|prev|next|last]_error. They are basically equivalent to goto_[first|prev|next|last]_diag but ignore all diagnostics that aren't severe enough.

It closes issue #3405 and supersedes PR #6331. The latter is very similar (and was used as an inspiration) but doesn't implement goto_[first|last]_error.

The four commands are mapped to [|] + e|E similar to goto_[first|prev|next|last]_diag. The latter e is IMO the obvious choice, e for error and also its proximity to d is nice, enhancing the existing muscle memory.

In theory, the severity level for which diagnostics are valid targets for goto_[first|prev|next|last]_error could be a config flag. For now, it is hard-coded to Severity::Error.

What do you think?

cgahr avatar Nov 04 '23 19:11 cgahr

I feel like first and last are unnecessary because they can be achieved very easily by, respectively: 1G<goto_next_error> and ge<goto_prev_error>

Edit: Actually, I see that there is precedence for this behavior, but it feels odd to me for that too.

kirawi avatar Nov 04 '23 20:11 kirawi

Tbh, I never thought about using gg]e (or gg]d for that matter) to get the next error/diagnostic.

In any case, I think binding the capitalized letter to first and last doesn't really waste "keymap realestate", so why not have it?

cgahr avatar Nov 05 '23 13:11 cgahr

Edit: Actually, I see that there is precedence for this behavior, but it feels odd to me for that too.

Yes, this is a pretty well established pattern.

archseer avatar Nov 15 '23 23:11 archseer

Was just coming over to file an issue to request this very thing.

Any thoughts on when this might be merged? :)

CC @archseer

aral avatar Mar 16 '24 20:03 aral