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

cmd/govim: de-dupe quickfix diagnostics and retain selected index #476

Merged
merged 1 commit into from Sep 4, 2019

Conversation

myitcv
Copy link
Member

@myitcv myitcv commented Sep 2, 2019

No description provided.

Copy link
Member

@leitzler leitzler left a comment

Choose a reason for hiding this comment

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

One small thing, otherwise LGTM

Currently, because of golang/go#32443 we can
unnecessarily update the quickfix window with diagnostics. What's more,
when we update the quickfix list we lose the selected index; the first
row is selected when the quickfix list is reset.

This presents something of a problem when jumping to a quickfix error in
a file that is not currently open, where that error is not the first in
the quickfix list. Because jumping to that error means we trigger a
didOpen, which triggers all diagnostics to be sent again (even though
they haven't changed), which causes the first row to be selected again.
The user, understandably, get's very confused.

So for now, we use reflect.DeepEqual to de-dupe the diagnostics we
receive from gopls, pending a fix to
golang/go#32443. We also keep track of the
last set error diagnostics, so that when setting new diagnostics we can
reselect the currently selected row. Or, if that row no longer exists,
select the first row. This changes the times that setqflist is called.
Hence, fix up tests which previously asserted based on this behaviour.

As part of this change we also retrigger clearing/population of the
quickfix list (and signs) as the relevant config changes.

Also slightly optimise the sign placement code to return early if signs
are disabled.
@myitcv myitcv merged commit 956c1eb into master Sep 4, 2019
@myitcv myitcv deleted the dedupe_quickfix_diagnostics branch September 4, 2019 14:13
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

2 participants