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

fix(pane): handle ^M and ^[K codes properly #3488

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Hylian
Copy link
Contributor

@Hylian Hylian commented Jul 10, 2024

  • ^M is the control code for carriage return ('\r')
  • ^[K is equivalent to ^[0K

@imsnif
Copy link
Member

imsnif commented Jul 10, 2024

Hey @Hylian - thanks for this. The CI tells us this is failing some vttests, which might mean the fix should be a little more nuanced. On Discord we talked about providing the exact sequence that is causing trouble. Ideally a minimal reproduction that we could cat inside and outside of Zellij to compare and see the different behavior.

You could use the --debug flag as instructed in the ISSUE_TEMPLATE with the offending program if you like.

Could you provide this minimal reproduction so that I'll take a closer look? Thanks.

@Hylian
Copy link
Contributor Author

Hylian commented Jul 10, 2024

Yep, will do. I was having some trouble reproducing the same scenario on my personal machine (unfortunately, I can't directly share the failing build output.) I'll file an issue after investigating a bit more and checking my assumptions.

@Hylian Hylian force-pushed the hylian/control-code-fixes branch from e999e5b to ff3cdb1 Compare July 10, 2024 18:25
* ESC M should scroll screen up
* ESC[K is equivalent to ESC[0K
@Hylian Hylian force-pushed the hylian/control-code-fixes branch from ff3cdb1 to f91fc6b Compare July 10, 2024 18:25
@Hylian
Copy link
Contributor Author

Hylian commented Jul 10, 2024

Had a brainfart and mixed up ^M and ESC M, since vim seems to display them the same. Fixed in updated patch.

Also created #3489 with repro for the issue I'm seeing. It doesn't seem directly related to this patch, so root cause may be different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants