-
-
Notifications
You must be signed in to change notification settings - Fork 652
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
Zellij replaces random chars with whitespace on the prompt #943
Comments
Is this a regression? Does it happen with 0.22.1? When you have the time, if you can follow the instructions for reporting a visual issue and attach the logs it'll be really helpful in debugging this. |
I'll do my best; I did not see this in 0.20.1, but I did not have that unicode character then. It's hard to "instantly" reproduce, it starts happening after working for a while. I'll see later (tomorrow?) if I can find a sure-fire set of steps, and refine on that. |
ok, what I did:
|
If you need more, I can add debug tracing and/or screen recording tomorrow! |
Was this reproducible, or would you appreciate debug logging? |
Hey, sorry - I didn't get to this yet. Might need a few days. Debug logging would be very helpful if you can provide it! |
The debug logs; I redacted a small part from zellij-9.log, I hope this is still usable? |
I just tested with a "clean" prompt, and the issue is not there. I'll iterate over my current prompt (adding more elements) and see if I can pinpoint it |
Awesome, thanks! Could you also add the size of the terminal to the logs? You can get this with |
cols: 117 |
It's the unicode character |
Try with simply this: export PS1="🏠" Then do the steps in my first post. |
Don't even need the prompt:
Paste the house first on the prompt, then space, then |
And for completeness' sake, you can replace ctrl+left with just hitting the left key until you are in the right spot. |
What shell / terminal emulator are you using? I can't reproduce with zsh on xfce4-terminal. |
Bash inside tilix |
I just reproduced it with these steps:
|
Did anyone manage to reproduce this? |
I did not manage to get to this yet. These sorts of issues are high priority for me, but this was one of those weeks where everything was high priority :) I hope to get to this soon. |
I removed the clock as a workaround, so this is not breaking my work. Of course, this should be fixed (if anyone else can reproduce it). Otherwise I'm very curious what's special about me :-) |
99% it has to do with a combination of the wide-character and some cursor jump-to ANSI instruction (to hazard a guess). While we have wide-character handling in our interpreter, some edge-cases slip through sometime. |
Does just using arrow keys count as "jump-to"? Of maybe I have no idea what jump-to ANSI instructions are... |
Hey, so I found the issue. Thanks for the excellent reproduction steps, they really helped! We track wide characters as one character with a width attribute of more than 1 (this is a trade-off because it makes the line wrapping much easier to handle - at least in my head). Because of this, when we do all sorts of operations on them we have to account for their width. In most cases we handle this, but in this case we didn't. The case was: inserting a character in a line that contains one or more wide characters before it. The reason you were seeing it when overriding the When overriding a part of the line without a space after it, it prints the entire rest of the line to the terminal (this was when you were overriding the When you were typing the I was not able to reproduce this with Anyway - I issued a fix for this which will be in main once the CI is happy. I want to hold off a little bit to release a version for this (since this isn't a regression) until we have some more stuff in main? I'm guessing it won't take very long. I hope you can be patient? (or run from main directly if you're brave :) ). |
@JCallicoat was using |
Sure, here it is. |
Thx, testing it now (first check was ok: no more wipe-out) |
There is a new? different? bug now: when using backspace after adding the |
So, just to be clear, the bug is purely visual: the command is what it should be, but visually it's wrong. |
Yes, another bug! Here's a musl with a fix, want to give it a try? |
So far so good! Just wondering if it would be possible to attach zellij with a new version, to an older running version. What are the issues if that would be possible? |
Awesome! And sorry for the bit-by-bit approach here. This is one of the darker parts of the code and I look forward to refactoring it. I might try to go over stuff and fixing these wide char edge cases more thoroughly when I have the chance. As for attaching to an existing session - the client part mostly handles STDIN/STDOUT, so even if this were possible it wouldn't help much with this bug (as the state, even the visual state, is kept in the server part - which is running the older version). |
And as a side note - it will probably work as long as we didn't change the protocol between the two parts, which we don't do very often. I plan on versioning it to make it clear when this is a no-go, but it's one of those TODO things. |
When I try this (attach to a newer version, as with these small bug fixes), I seem to get a new instance instead; Don't worry about the bit-by-bit approach, this is perfectly reasonable for a |
Aha, yes! That's because the servers are themselves versioned. I forgot about that bit. There's one server process for each session, and their IPC session file is in a directory that includes the version. So, apparently the functionality is already there in a way. |
So far I had no whitespace issues, so this is fixed for me... |
I seem to have a variant of this issue:
it eats the next character ( |
Just updated to v0.26.0, can no longer reproduce. Probably the fix was not yet in 0.25, which I upgraded to earlier... |
Yes, I fixed this one a few days ago :) |
This can be closed, then? |
Although PS1 seems to work fine with wide characters, you can get this exact same problem by using 🏠 or whatever in your RIGHT prompt. Running This is on Should this issue be reopened? |
Let's not conflate things. If you can reproduce this behavior, please open a new issue. But please be sure to follow the instructions for reporting a graphical issue and attach the relevant logs. |
Don't have much time to find a complete reproduction, apologies...
ctrl+L
to clear the screen, it shows what it should show)The text was updated successfully, but these errors were encountered: