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: persist highlights for explicit GOVIMHighlightReferences call #852

Merged
merged 1 commit into from
Apr 13, 2020

Conversation

myitcv
Copy link
Member

@myitcv myitcv commented Apr 9, 2020

When manually invoking GOVIMHighlightReferences, it makes sense to
persist the highlights for as long as we can. To be safe we assume that
making any change to any .go file will potentially invalidate the
highlighted references (we might relax this at a later stage). We also
add the GOVIMClearReferencesHighlights command to explicitly remove the
highlights.

As part of this same commit we:

  • move the definition of currentReferences to vimstate where it belongs
    (it is only ever accessed on the vimstate "thread")
  • Tidy up and simplify some logic regarding removal of highlights

@myitcv myitcv requested a review from leitzler April 9, 2020 00:11
@myitcv myitcv force-pushed the cmd_govim_persist_references_highlight branch from 7ace240 to 91e1da2 Compare April 12, 2020 04:23
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.

Just a comment, otherwise it LGTM

cmd/govim/buffer_events.go Show resolved Hide resolved
@myitcv myitcv force-pushed the cmd_govim_persist_references_highlight branch from 91e1da2 to b61398b Compare April 13, 2020 09:33
When manually invoking GOVIMHighlightReferences, it makes sense to
persist the highlights for as long as we can. To be safe we assume that
making any change to any .go file will potentially invalidate the
highlighted references (we might relax this at a later stage). We also
add the GOVIMClearReferencesHighlights command to explicitly remove the
highlights.

As part of this same commit we:

* move the definition of currentReferences to vimstate where it belongs
  (it is only ever accessed on the vimstate "thread")
* Tidy up and simplify some logic regarding removal of highlights
@myitcv myitcv force-pushed the cmd_govim_persist_references_highlight branch from b61398b to e9f282d Compare April 13, 2020 09:50
@myitcv myitcv merged commit 3139f79 into master Apr 13, 2020
@myitcv myitcv deleted the cmd_govim_persist_references_highlight branch April 13, 2020 10:07
myitcv added a commit that referenced this pull request Apr 13, 2020
In 3139f79 I incorrectly removed all reference highlights when a delta
is received. However, the logic prior to that change was that highlights
would only be moved (even in insert mode) if the cursor falls outside of
a highlighted range.

Therefore, we adopt exactly the logic that @leitzler proposed in #852 to
only remove highlights on delta if we are in manual highlighting mode.
myitcv added a commit that referenced this pull request Apr 13, 2020
…857)

In 3139f79 I incorrectly removed all reference highlights when a delta
is received. However, the logic prior to that change was that highlights
would only be moved (even in insert mode) if the cursor falls outside of
a highlighted range.

Therefore, we adopt exactly the logic that @leitzler proposed in #852 to
only remove highlights on delta if we are in manual highlighting mode.
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.

2 participants