Migrate editor to use remote backed buffer#10520
Migrate editor to use remote backed buffer#10520kevinyang372 wants to merge 7 commits intomasterfrom
Conversation
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR wires the code editor and file tree to open and save remote-backed buffers, adds conflict notifications, and extends buffer tracking for server-local buffers.
Concerns
- Remote
OpenBufferresponses can still replace the editable buffer unconditionally, which can drop user edits made before the initial load completes. - Local file-tree
CodeSourcestate no longer persists its file path because the only location field is skipped by serde. - Remote save failure handling can clear the dirty state even when the save failed.
- Discarding a remote conflict by closing and reopening the buffer does not guarantee a disk reload when another connection still has that server buffer open.
Found: 1 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // (which contains stale client edits). The subsequent OpenBuffer | ||
| // will create a fresh buffer and read from disk. | ||
| let path_for_close = path_str.clone(); | ||
| client.close_buffer(path_for_close); |
There was a problem hiding this comment.
CloseBuffer does not force a fresh disk read when another connection still has the same server buffer open; the server keeps the stale in-memory buffer and the subsequent OpenBuffer can return it again. Use an explicit reload/accept-server RPC or otherwise force the server to reopen from disk.
There was a problem hiding this comment.
This is a very good call. Updated to have a force_reload flag for opening buffers
moirahuang
left a comment
moirahuang
left a comment
There was a problem hiding this comment.
-
the delay is kinda long when overwriting, can we auto dismiss and then bring back the banner if it fails to save
https://github.com/user-attachments/assets/56e11d37-251f-4329-8c02-0692982b22e6 -
when we discard, we aren't correctly writing the first time. it only correctly writes the 2nd time
https://github.com/user-attachments/assets/221b8d00-f826-48da-8e03-ca39d395637e
|
|
||
| match item { | ||
| FileTreeItem::File { metadata, .. } => { | ||
| // Remote file trees don't support opening files in the editor. |
| let manager = RemoteServerManager::handle(ctx); | ||
| manager.as_ref(ctx).client_for_host(&host_id).cloned() | ||
| }; | ||
| log::debug!( |
There was a problem hiding this comment.
nit: do we need the log debugs here and below?
| return; | ||
| }; | ||
| if let BufferSource::Remote { sync_clock, .. } = &mut state.source { | ||
| *sync_clock = Some(SyncClock::from_wire(response.server_version, 0)); |
There was a problem hiding this comment.
why do we save this version as 0 vs. the latest version?
2 participants
Description
Wire editor to use remote backed buffer. A few noteable things:
Also fixed a bug where server detected conflicts aren't being properly forwarded to the client
This also removes the remote file tree gating on not opening files
Testing
Tested locally