Terminal support for remote development

[Yesterday17]

This PR introduces a MVP version of remote terminal support for validation purposes. I may do some code clean up and mark it ready for review in the following days.

There are many missing points, such as permission control and terminal syncing

Release Notes:

• Added terminal support for remote development

Screenshot

Just looks like local terminal, but in dev server.

[SomeoneToIgnore]

Looks amazing if it actually works, can't wait to test it! Right now though, I lack the ability to open the terminal still (have spawned the code both remotely and locally):

I see something related is going on though:

!!!is remote
!!!is remote
!!break

and there is a bunch of non-awaited futures that might be the culprit?

warning: unused implementer of `futures::Future` that must be used
   --> crates/terminal_view/src/terminal_panel.rs:375:13
    |
375 | /             self.replace_terminal(
376 | |                 working_directory,
377 | |                 spawn_task,
378 | |                 existing_item_index,
379 | |                 existing_terminal,
380 | |                 cx,
381 | |             );
    | |_____________^
    |
    = note: futures do nothing unless you `.await` or poll them
    = note: `#[warn(unused_must_use)]` on by default

warning: unused implementer of `futures::Future` that must be used
   --> crates/terminal_view/src/terminal_panel.rs:396:33
    |
396 | / ...                   terminal_panel.replace_terminal(
397 | | ...                       working_directory,
398 | | ...                       spawn_task,
399 | | ...                       existing_item_index,
400 | | ...                       existing_terminal,
401 | | ...                       cx,
402 | | ...                   );
    | |_______________________^
    |
    = note: futures do nothing unless you `.await` or poll them

If there are some extra steps needed to test it, would be nice to mention it in the PR/docs/elsewhere.

Also:

permission control

could be omitted entirely at this point I think: shared projects belong to the same user, and other multiplayer projects may turn the clients' terminals into readonly mode for now. Am I missing something?

[Yesterday17]

Did you run the collab server too? It requires some new requests/messages to work. I’ll add these requirements to the PR later today.

Kirill Bulatov @.***>于2024年4月28日 周日5:30写道：

Looks amazing if it actually works, can't wait to test it! Right now, though, I lack the ability to open the terminal still (have spawned the code both remotely and locally): image.png (view on web) https://github.com/zed-industries/zed/assets/2690773/59fee223-d848-4e17-b072-d96f50199aca

I see something related is going on though:

!!!is remote !!!is remote !!break

and there is a bunch of non-awaited futures that might be the culprit?

warning: unused implementer of futures::Future that must be used --> crates/terminal_view/src/terminal_panel.rs:375:13 | 375 | / self.replace_terminal( 376 | | working_directory, 377 | | spawn_task, 378 | | existing_item_index, 379 | | existing_terminal, 380 | | cx, 381 | | ); | |_____________^ | = note: futures do nothing unless you .await or poll them = note: #[warn(unused_must_use)] on by default

warning: unused implementer of futures::Future that must be used --> crates/terminal_view/src/terminal_panel.rs:396:33 | 396 | / ... terminal_panel.replace_terminal( 397 | | ... working_directory, 398 | | ... spawn_task, 399 | | ... existing_item_index, 400 | | ... existing_terminal, 401 | | ... cx, 402 | | ... ); | |_______________________^ | = note: futures do nothing unless you .await or poll them

If there are some extra steps needed to test it, would be nice to mention it in the PR/docs/elsewhere.

Also:

permission control

could be omitted entirely at this point I think: shared projects belong to the same user, and other multiplayer projects may turn the clients' terminals into readonly mode for now. Am I missing something?

— Reply to this email directly, view it on GitHub https://github.com/zed-industries/zed/pull/11108#issuecomment-2081187695, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCEFLV7DLAUPCSFEDGSRLTY7QKGBAVCNFSM6AAAAABG4LFAR6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBRGE4DONRZGU . You are receiving this because you authored the thread.Message ID: @.***>

[SomeoneToIgnore]

Oh indeed it opens on the client side this way, thank you for pointing it out — I've got confused somehow.

A few immediate notes that you might be aware about, but feels important to emphasize early:

• Judging by the logs, there seems to be lot of things going on inside collab (including frequent DB access) when I just open the client terminal and do nothing (only move the mouse around), feels odd
• While there's a way to input commands into that terminal, but no signals seem to be working: if I start cat /dev/urandom, there's no ctr-c or anything that actually stops this — even closing the remote terminal does not seem to help, the editor has to be killed to stop it
• The most important note: current mode (remote terminal can be opened by a host to run commands) will work well for the personal remote projects when finished. But for the actual multiplayer, when the clients are following the host, they should see the terminal host opens in readonly, live, with host's input and command output. So, it's not really about their input at all, but rather about a readonly view of somebody else's terminal (ergo, no permissions needed). If multiplayer does not get the readonly mode, I do not think it should get any remote terminal at all now.

[SomeoneToIgnore]

Hey @Yesterday17 , I was thinking to put more traction into this and get the both readonly and writeful remote terminals by the end of this month. Would you want to pair on this via Zed? Send me an e-mail/message via my contact details with the time that works for you. And thanks again for finding the way to make alacritty remote, that's the bit I've missed in my head when considered the feature.

[Yesterday17]

Hey @Yesterday17 , I was thinking to put more traction into this and get the both readonly and writeful remote terminals by the end of this month. Would you want to pair on this via Zed? Send me an e-mail/message via my contact details with the time that works for you. And thanks again for finding the way to make alacritty remote, that's the bit I've missed in my head when considered the feature.

Sorry for the late reply... I was on holiday during the past week, and would be busy in the remaining days of this month. So I may not have enough time to participate in the pairing. But feel free to use any code submitted in this PR!

Also, for the ctrl-c problem, I guess that's because in the current implementation, the reader is always reading from the buffer: https://github.com/zed-industries/zed/pull/11108/files#diff-09db91a35e4e0177b5b7702f0bc4d5210459f4734dc0b5e1a103e855370eecdbR63-R72 It might be fixed by adding some read latency / making the mpsc bounded.

[SomeoneToIgnore]

No worries and thanks for replying, hope you've had a nice holiday!

I think at this point, I will close this PR due to its conflicts and the desire to rewrite it slightly: so far, it seems that running an ssh foo@bar covers 80% of the cases where a "read-write" terminal needed, as this PR brings. That ssh thing is what I'm working on to integrate into Zed remote project support and it is super easy from the terminal standpoint.

But we would still need something like this for, ideally, collaborative terminal, or at least a terminal "read-only view" (seems the most feasible and useful application of the current code now). For that though, the current code has to be adjusted in some ways:

• right now, we send separate messages with byte "hunks", while collab server should already support streaming requests: seems a bit more effective, esp. for the "read-only" model
• we have to solve the reconnect/replay issue somehow: it appears to me that we cannot stream bytes from arbitrary place and would need to replay the entire byte stream again in edge cases (e.g somebody opened Vim in the beginning of the session and the rest of the commands are inside it)?
• we have to understand how to deal with multiple terminals having different size (feels that we might want to restrict certain things like resizes for such collab terminals)
• tidy up the code a bit: remove all .unwrap()s, store all tasks inside the structs instead of .detach() calls, etc.

I've started applying parts of this PR to my branch, fixing the small issues (managed to get rid of one channel pair too!): https://github.com/zed-industries/zed/compare/main...kb/remote-terminal but this is far from over, as does not solve any other aforementioned issues.

My plan was to slowly get this branch working for read-only follow-up mode and try to note and solve as many terminal-related issues as I can — but this is not going to happen this month as I've planned, esp. due to me allocating time on other things. So, feel free to continue with your branch, my branch, or wait for me to move things forward way later — seems that we have a lot of work to do before it's all ready 🙂