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

Sync file system and language server after restore #4020

Merged
merged 6 commits into from
Jan 5, 2023

Conversation

hubertp
Copy link
Contributor

@hubertp hubertp commented Jan 3, 2023

Pull Request Description

VCS restore operation was correctly restoring the state of projects to the requested commit. Unfortunately, after the operation file system was becoming out-of-sync with language server's buffers (and IDE's content versions).

A few important changes are introduced here that complicate the interaction between components:

  1. vcs restore returns an actual diff between the current state and the
    requested commit
  2. the response is forwarded to buffer registry first rather than to the client
  3. the diff is used to identify appropriate collaborative editors and
    notify them about the need to reload buffers from file system
  4. all clients of affected open buffers are notified of the change via
    text/didChange notification. If a file was removed and there were open buffers for it, clients will be notified via file/event and editor will be stopped
  5. only then the client is notified about a successful restore operation

This PR addresses one of the two problems reported in https://www.pivotaltracker.com/story/show/184097084.

Important Notes

We need to make sure that IDE correctly responds to text/didChange notifications.

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the
    Scala,
    Java,
    and
    Rust
    style guides.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH
      ./run ide build and ./run ide watch.

@hubertp hubertp force-pushed the wip/hubert/restore-vcs-184097084 branch 3 times, most recently from 0908c1c to fb1a450 Compare January 3, 2023 16:00
Copy link
Contributor

@4e6 4e6 left a comment

Choose a reason for hiding this comment

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

This VCS feature turned out to be more involved than we originally thought 😅

@hubertp hubertp force-pushed the wip/hubert/restore-vcs-184097084 branch 2 times, most recently from 4437c81 to 8cf137d Compare January 5, 2023 10:50
VCS restore operation was correctly restoring the state of projects to
the requested commit. Unfortunately, after the operation file system was
becoming out-of-sync with language server's buffers (and IDE's content
versions).

A few important changes are introduced here that complicate the
interaction between components:
1) vcs restore returns an actual diff between the current state and the
   requested commit
2) the response is forwarded buffer registry first rather than to the client
3) the diff is used to identify appropriate collaborative editors and
   notify them about the need to reload buffers from file system
4) all clients of affected open buffers are notified of the change via
   `text/didChange` notification
5) only then the client is notified about a successful restore operation
`vcs/restore` may remove files if they have been added after the commit.
An clients with an open buffer for such files will be notified via
`file/event`.
@hubertp hubertp force-pushed the wip/hubert/restore-vcs-184097084 branch from 8cf137d to 47607e3 Compare January 5, 2023 10:50
Making tests deal with temporary flakiness/delays in forwardign messages
between actors.
@hubertp hubertp force-pushed the wip/hubert/restore-vcs-184097084 branch from 47607e3 to b14b4cc Compare January 5, 2023 13:01
@hubertp hubertp added the CI: Ready to merge This PR is eligible for automatic merge label Jan 5, 2023
@mergify mergify bot merged commit 3980c48 into develop Jan 5, 2023
@mergify mergify bot deleted the wip/hubert/restore-vcs-184097084 branch January 5, 2023 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants