-
-
Notifications
You must be signed in to change notification settings - Fork 610
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
Windows support #2926
base: main
Are you sure you want to change the base?
Windows support #2926
Conversation
No functionality yet, but at least it compiles
# Conflicts: # Cargo.lock # zellij-server/src/background_jobs.rs
Hi, diff --git a/zellij-server/src/os_input_output.rs b/zellij-server/src/os_input_output.rs
index d23789cf..cafca34d 100644
--- a/zellij-server/src/os_input_output.rs
+++ b/zellij-server/src/os_input_output.rs
@@ -826,7 +826,7 @@ impl ServerOsApi for ServerOsInputOutput {
// See https://docs.rs/sysinfo/0.22.5/sysinfo/struct.ProcessRefreshKind.html#
system_info.refresh_processes_specifics(ProcessRefreshKind::default());
- if let Some(process) = system_info.process(pid.into()) {
+ if let Some(process) = system_info.process((pid.as_raw() as usize).into()) {
let cwd = process.cwd();
let cwd_is_empty = cwd.iter().next().is_none();
if !cwd_is_empty {
@@ -845,7 +845,7 @@ impl ServerOsApi for ServerOsInputOutput {
let mut cwds = HashMap::new();
for pid in pids {
- if let Some(process) = system_info.process(pid.into()) {
+ if let Some(process) = system_info.process((pid.as_raw() as usize).into()) {
let cwd = process.cwd();
let cwd_is_empty = cwd.iter().next().is_none();
if !cwd_is_empty {
After this change it compiles and runs a first short test without problem! To make all the tests work again another simple fix is necessary: diff --git a/zellij-server/src/tab/unit/tab_integration_tests.rs b/zellij-server/src/tab/unit/tab_integration_tests.rs
index c3309f55..c9c455c8 100644
--- a/zellij-server/src/tab/unit/tab_integration_tests.rs
+++ b/zellij-server/src/tab/unit/tab_integration_tests.rs
@@ -103,6 +103,7 @@ impl ServerOsApi for FakeInputOutput {
&mut self,
_client_id: ClientId,
_stream: LocalSocketStream,
+ _sender: LocalSocketStream,
) -> Result<IpcReceiverWithContext<ClientToServerMsg>> {
unimplemented!()
}
diff --git a/zellij-server/src/tab/unit/tab_tests.rs b/zellij-server/src/tab/unit/tab_tests.rs
index b1e2110a..b2ba2850 100644
--- a/zellij-server/src/tab/unit/tab_tests.rs
+++ b/zellij-server/src/tab/unit/tab_tests.rs
@@ -77,6 +77,7 @@ impl ServerOsApi for FakeInputOutput {
&mut self,
_client_id: ClientId,
_stream: LocalSocketStream,
+ _sender: LocalSocketStream,
) -> Result<IpcReceiverWithContext<ClientToServerMsg>> {
unimplemented!()
}
diff --git a/zellij-server/src/unit/screen_tests.rs b/zellij-server/src/unit/screen_tests.rs
index c9de7afd..258ee360 100644
--- a/zellij-server/src/unit/screen_tests.rs
+++ b/zellij-server/src/unit/screen_tests.rs
@@ -188,6 +188,7 @@ impl ServerOsApi for FakeInputOutput {
&mut self,
_client_id: ClientId,
_stream: LocalSocketStream,
+ _sender: LocalSocketStream,
) -> Result<IpcReceiverWithContext<ClientToServerMsg>> {
unimplemented!()
}
|
Pleas Keep working on this!!! I would love to have support on Windows!! |
I was naively thinking that zellij should work under windows if built from source ... this is a lot of work. |
Appreciate the work you are doing on this. |
I want to learn Rust just so that I can contribute to this PR. @iXialumy Thank you for working on this ❤️ |
If you wish to express support for this PR, please use the 👍 reaction on the original post rather than posting a reply. Some people are subscribed to this PR and receive notifications for all activity, including new comments. |
I felt like a single emoji is not enough, and that I should express my gratitude to the person in question in another way. |
I am kind of stuck now. I got to a point where zellij starts, connects a server and a client and does not crash instantly when typing something, but this is the point, where neither the compiler nor the logs give me a clear direction whats the next step to tackle. @imsnif If you or anyone else knows where to look next please hint me, otherwise I will continue to poke in the dark until I found the right place to look at. :) |
Hey @iXialumy - happy to see you're still at it. Kudos on your perseverance! On the surface, this looks like at least two different problems - even though it's possible it's one core problem that creates an invalid state and causes everything else. Without looking at the code, and from what you describe, I would guess you need to somehow handle spawning terminals. Specifically looking into how to translate what we're doing with When I go into a code base and try to fix something, I usually really hate it when someone who knows the code base well tells me something along the lines of "You're going about this the long way, you should really be doing..." - so, I'm not going to tell you that. But I will say that I think the compiler doesn't know a lot about architecture and system compatibility. The big chunk of work here is moving from pty to ConPty and adjusting the code to match, possibly even finding libraries that do this (since we're using direct syscalls on linux/mac). I'd recommend giving a read to a blog post I wrote on the subject a while ago: https://poor.dev/blog/terminal-anatomy/ That being said, since I promised I will not do what I just did, I wish you good luck and have confidence in you finishing this up in whatever way works for you. EDIT: another interesting place to look is here: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/terminal_bytes.rs#L66 - which is the async task that reads bytes from the pty handle. Not sure how you're handling it, but could be that the pty is opened (I briefly saw you're using something there already) but not read from properly. |
Hey @imsnif First of all, thanks for the great feedback. I used your example code to figure out quite a big difference in reading from ptys in windows and unix systems. This is probably why my current implementation has no output. Anyways thank you for taking your time to provide some feedback. I will take anything I can get :) |
Super cool @iXialumy !! Great work. Looking forward to seeing more progress. For reference, writing to the pty happens here: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/pty_writer.rs#L42 Hope this helps. |
First of all, @iXialumy thank you so much for your hard work! I would love to help contribute where I can! I recently cloned your branch and implemented the I'm just not sure how we want to go about letting others contribute to this? Should I be making PRs into your fork? Or am I better off creating my own PRs into zellij-org so that this PR doesn't end up being too large in the end? @imsnif, what do you think? |
Please don't make PRs into zellij-org :) |
@maxle5 Feel free to open a PR on my fork then. I think when I approve it, it should show up here too :) |
I've kind of got command output showing up. It doesn't actually read user input, but I rigged it up to do an ls command after pressing enter. The main issue was that the I'll make a PR into @iXialumy 's repo when it's a bit more ready. |
And here it is reading and writing! I've got a PR into your repo @iXialumy. Getting closer... |
@ulyssesdotcodes It looks like there are trailing tabs when isssuing commands (i.e. ls). Any fix for it so command starts at the beginning of line? |
IPC Works again with only a single connection between client and server.
*they do not want :) Until this is merged, I think the best place for issues and work tracking is this fork. Once this is merged, naturally the place for issues would be the main repository. I appreciate everyone's work on this and can indeed empathize with people wanting this merged ASAP - but this is a huge undertaking and as such would take time. Let's be patient with the developers and testers working on this and allow them to bring it to fruition. Seeing the care and collaboration going on here is very inspiring. |
Unfortunately it seems you need to use the low-level api function `ReadConsoleInput` to get this information. Then it is necessary to handle all inputs more or less manually. Tested with alacritty
I pulled this and I was able to get it compiled and running! This is great because I need to use Windows unfortunately... I have a Alacritty + Zellij + vim workflow I use on macos and ubuntu and would like to have the same workflow. But I noticed I couldn't type the colon character, I'm trying to debug and will open a PR when I find the issue! @iXialumy Any thoughts on why I might not be able to type the colon character? Can you point me in the right direction? Initially I also wanted to try to figure out the list-session command and open a PR. |
@benjaminknox I fear that bug has been introduced in iXialumy#5. |
So, I'm seeing that the colon key works in windows 11 but not windows 10... |
This reverts commit 876465b.
Some behavior I'm noticing on windows 11 as I've worked on it with alacritty is the UI in zellij sometimes completely freezes when context switching to another application and back and when the computer goes to sleep after inactivity sometimes. I'm not sure what's causing this but I'm going to see if I can capture logs as I work. Edit: Just happened, seems like a new session tries to start? (Failed to fill whole buffer log at the end seems to happen when I close the alacritty every time)
Edit: Happened again, similar log, seems like it tries to open a new shell (see the WARN), I don't know if that's root cause though?
|
Session support (minimal changes)
Thanks for the great work! I have filed an issue upstream at andfoy/winpty-rs#75, where .read() calls are essentially being throttled to 1 read() per 100ms. Couldn't find anywhere else to share this information so decided to put it here. Modifying upstream code to remove the throttle instantly made input latency feels instantaneous. |
I am starting to track things that need work at the top now for people who just want to see the status or that want to help and need a "thing" to work at:
Original Message:
I have gotten it to compile under windows and (hopefully) IPC working.
I hope this did not break anything for non-windows systems, but I did not test it at the moment.
From here I hope the compiler will guide me to all the places that have yet to be implemented.