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.

Sign up for GitHub

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

Jump to bottom

cmd/govim: leaving userbusy always updates text property highlighs #700

Merged
merged 1 commit into from
Jan 21, 2020
Merged

cmd/govim: leaving userbusy always updates text property highlighs #700

merged 1 commit into from
Jan 21, 2020

Conversation

leitzler
Copy link
Member

The diagnostic highlights were always updated when leaving userbusy.
If the buffer changed since the last diagnostic received from gopls,
we might end up with a buffer that doesn't contain the lines/cols
where we try to place text properties. That did thrown an error.

This fix adds the same logic that exists for other diagnostic
features, so that leaving userbusy only triggers text property
placment if there are new diagnostics.

@leitzler leitzler requested a review from myitcv January 20, 2020 22:42
myitcv
myitcv approved these changes Jan 21, 2020
View reviewed changes
Copy link
Member

@myitcv myitcv left a comment

Choose a reason for hiding this comment

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

LGTM with a couple of comments. Including the fact that I think we do need to follow up with the try/catch safety net for signs and highlights.

cmd/govim/buffer_events.go Show resolved Hide resolved
cmd/govim/gopls_client.go Show resolved Hide resolved
cmd/govim/vimstate.go Show resolved Hide resolved
@leitzler
cmd/govim: leaving userbusy always updates text property highlighs
1d56fa8 
Diagnostic highlights are always updated when leaving userbusy.
If the buffer changed since the last diagnostic received from gopls,
we might end up with a buffer that doesn't contain the lines/cols
where we try to place text properties. This will thrown an error.

This fix adds the same logic that exists for other diagnostic
features, so that leaving userbusy only triggers text property
placment if there are new diagnostics.
@leitzler leitzler force-pushed the cmd_govim_textpropdiag_userbusy branch from c19ac51 to 1d56fa8 Compare January 21, 2020 11:41
@leitzler leitzler merged commit de8e9ac into master Jan 21, 2020
@leitzler leitzler deleted the cmd_govim_textpropdiag_userbusy branch January 21, 2020 12:10
@leitzler leitzler mentioned this pull request Jan 27, 2020
Closed
leitzler added a commit that referenced this pull request Jan 27, 2020
@leitzler
cmd/govim: Ensure text prop highlights aren't placed when disabled
bba493c 
The config HighlightDiagnostics can be used to disable in-code
highlights. PR #700 accidently broke that feature so that it was
possible to get highlights even when disabled.

This PR ensures that text prop highlights aren't placed when they
are disabled by the config.

Fixes #724
@leitzler leitzler mentioned this pull request Jan 27, 2020
Merged
leitzler added a commit that referenced this pull request Jan 27, 2020
@leitzler
cmd/govim: Ensure text prop highlights aren't placed when disabled (#730
60a1430 
)

The config HighlightDiagnostics can be used to disable in-code
highlights. PR #700 accidently broke that feature so that it was
possible to get highlights even when disabled.

This PR ensures that text prop highlights aren't placed when they
are disabled by the config.

Fixes #724
@leitzler leitzler mentioned this pull request Feb 16, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@myitcv myitcv myitcv approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@leitzler @myitcv