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

Fixed: fully consume unknown CSI sequences containing non-numeric parameter byte #4417

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

Conversation

krobelus
Copy link

@krobelus krobelus commented Mar 1, 2025

  • To-do: this is untested, would appreciate your help on that (I don't have Android Studio or a device set up).

Standard ECMA-48: Control Functions for Coded Character Sets specifies
the format of CSI commands. Wikipedia has a concise description:
https://en.wikipedia.org/wiki/ANSI_escape_code#Control_Sequence_Introducer_commands

Do this for at least for sequences like "CSI = 5 u" that
contain non-numeric parameter bytes.

This fixes a problem with fish shell 4.0.0 which (for better or worse)
uses that sequence.

This patch introduces a slight inconsistency: for the above example,
"unknownSequence('u')" will be called. The resulting log message may
be confusing, because we do support "CSI u". We should fix this to
log the entire unknown sequence. I can try doing that.

In future we should also fully consume all other unknown CSI commands.
A contrived example would be "CSI ! ! a". If desired, I'm happy to
fix those as well, as I have some context already (but I don't think
that would block this patch).

Closes #4338

…ameter byte

To-do: this is untested, would appreciate help.

Standard ECMA-48: Control Functions for Coded Character Sets specifies
the format of CSI commands.  Wikipedia has a concise description:
https://en.wikipedia.org/wiki/ANSI_escape_code#Control_Sequence_Introducer_commands

Do this for at least for sequences like "CSI = 5 u" that
contain non-numeric parameter bytes.

This fixes a problem with fish shell 4.0.0 which (for better or worse)
uses that sequence.

This patch introduces a slight inconsistency: for the above example,
"unknownSequence('u')" will be called.  The resulting log message may
be confusing, because we *do* support "CSI u". We should fix this to
log the entire unknown sequence. I can try doing that.

In future we should also fully consume all other unknown CSI commands.
A contrived example would be "CSI ! ! a".  If desired, I'm happy to
fix those as well, as I have some context already (but I don't think
that would block this patch).

Closes termux#4338
@robertkirkman
Copy link

I have compiled and tested this PR

I can confirm that this passes the test case given in the issue,
the command printf '\x1b[=5u' no longer prints anything, whereas without this PR it prints 5u.

Tested device: Samsung Galaxy S III SPH-L710, Android 7.1.2

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.

[Bug]: may fail to ignore escape sequences for kitty progressive enhancements
3 participants