Skip to content

shellIntegration.ps1: escape values in "E" (executed command) and "P" (property KV) codes#165634

Merged
Tyriar merged 4 commits intomicrosoft:mainfrom
rwe:serialize-message-ps1
Nov 23, 2022
Merged

shellIntegration.ps1: escape values in "E" (executed command) and "P" (property KV) codes#165634
Tyriar merged 4 commits intomicrosoft:mainfrom
rwe:serialize-message-ps1

Conversation

@rwe
Copy link
Contributor

@rwe rwe commented Nov 6, 2022

This implements the string escaping scheme documented for the "E" (executed) and "P" (property KV) codes. This is "pure" zsh and relies on no external utilities.

Although currently the only "required" escapes are backslash, semicolon, \a, and perhaps newlines, this conservatively escapes all non-alphanumeric characters.

Backslashes are doubled, non-alphanumerics are hex-escaped.

Sibling to:

Relates to or resolves parts of:

Caveat

Unaddressed here, but more plain to see than in the other shells, is that the encoding of multibyte codepoints is not currently well-defined by the escaping scheme. This chooses to encode as UTF-8 in order to make them byte-representable at all. Regardless, shellIntegrationAddon.ts severely mishandles the interpretation of escapes anyway, even in the ASCII range, so the point is slightly moot.

@Tyriar
Copy link
Member

Tyriar commented Nov 23, 2022

I'm seeing this when running recent command when I fix conflicts and test:

image

@Tyriar
Copy link
Member

Tyriar commented Nov 23, 2022

For this:

image

Here's the string we get:
image

@Tyriar
Copy link
Member

Tyriar commented Nov 23, 2022

image

rwe added 4 commits November 23, 2022 11:22
Instead of only escaping '\', '␤', and ';', all non-alphanumeric values
are escaped. Backslashes are doubld and other characters are encoded as
hex.

The previous version would pass invalid characters through unescaped,
including OSC sequences. Although that no longer is allowed, this
iteration does not yet enforce  particular (e.g. utf-8) encoding and
will still misbehave on multibyte sequences.
@rwe rwe force-pushed the serialize-message-ps1 branch from ef00b33 to 91f0eae Compare November 23, 2022 19:37
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works now 👍

@Tyriar Tyriar merged commit dd21124 into microsoft:main Nov 23, 2022
@rwe rwe deleted the serialize-message-ps1 branch November 23, 2022 20:50
@github-actions github-actions bot locked and limited conversation to collaborators Jan 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants