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

fix stale state or lack of updates on changing branches #2182

Merged
merged 4 commits into from
Jan 29, 2023

Conversation

rchl
Copy link
Member

@rchl rchl commented Jan 28, 2023

When file has been "reloaded" by ST due to branch change or just modified externally then the requests that normally happen on file changes didn't run. That includes code lenses, symbol highlights, color boxes, semantic tokens, document link and inlay hints. That result in lack of those features until file was modified or stale state like with those code lenses for example:

Screenshot 2023-01-28 at 22 44 56

Comment on lines +907 to +909
different, current_region = self._update_stored_region_async()
if not different:
return
Copy link
Member Author

@rchl rchl Jan 28, 2023

Choose a reason for hiding this comment

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

As has been mentioned in some other issue, it's not really correct to return here if region didn't change as the file might still have changed (especially on file "reload"). The impact of this is very small currently though - pretty much only symbol highlights could misbehave.

@predragnikolic
Copy link
Member

Yes, we run into this issue when implementing inlay hints sublimelsp/LSP-typescript#91 (comment)

The cause of the bug was the _update_stored_region_async,
because it returned different False where it should returned True
in situations (like you concluded) if region didn't change but the file might still have changed:

see gif

IMO, It would be best to replace the _update_stored_region_async (because it doesn't function correctly)
with a different way to check if the document have changed. See sublimelsp/LSP-typescript@8e0d468

@rchl
Copy link
Member Author

rchl commented Jan 29, 2023

IMO, It would be best to replace the _update_stored_region_async (because it doesn't function correctly)
with a different way to check if the document have changed. See sublimelsp/LSP-typescript@8e0d468

Currently only symbol highlight and signature help relies on this check so it's not much of an issue. Other features don't go through that code path.

@rchl rchl merged commit a2d137e into main Jan 29, 2023
@rchl rchl deleted the fix/update-on-reload branch January 29, 2023 12:46
rchl added a commit that referenced this pull request Jan 31, 2023
* main: (80 commits)
  Perform inlay hint action on double instead of single click (#2175)
  support canceling pending completions request (#2177)
  Implement type hierarchy request (#2180)
  fix stale state or lack of updates on changing branches (#2182)
  Add timestamp and request duration in LSP logs (#2181)
  don't show diagnostics panel on save by default (#2179)
  trigger "on_server_response_async" also for "initialize" response (#2172)
  feat(API): allow setting additional text in permanent server status (#2173)
  Implement call hierarchy request (#2151)
  workaround for View Listeners not being attached for new transient view (#2174)
  Make Document Symbols behavior more consistent with built-in Goto Symbol (#2166)
  docs: fsautocomplete config - remove redundant argument (#2165)
  Allow missing window/workDoneProgress/create request from the server (#2159)
  Use "plaintext" language ID for plain text files (#2164)
  Improve type annotations (#2162)
  Don't use "escapeall" extension when formatting with mdpopups (#2163)
  Cut 1.21.0
  Fix inlay hint parts wrapping into multiple lines (#2153)
  Ensure commands triggered from minihtml run on correct view (#2154)
  Add "Source Action" entry to the "Edit" main menu (#2149)
  ...
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