Skip to content

shellIntegration-rc.zsh: escape values in "E" (executed command) and "P" (property KV) codes #165633

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

Merged
merged 3 commits into from
Nov 23, 2022

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:

@rwe
Copy link
Contributor Author

rwe commented Nov 6, 2022

I've opened #165665 re the lint failure on the diagram comment.

The existing "hygiene" script apparently only permits the characters ┌└├ (omitting soft elbows, down-left connectors of any kind, and horizontal/vertical lines) 😑

These could be redone in ASCII, but with loss of legibility.

EDIT: The diagram is now ASCII-only.

@rwe rwe force-pushed the serialize-message-zsh branch 2 times, most recently from 0c4e2d4 to 38e2d6a Compare November 15, 2022 23:16
@Tyriar Tyriar added this to the November 2022 milestone Nov 23, 2022
@Tyriar Tyriar assigned Tyriar and unassigned meganrogge Nov 23, 2022
@Tyriar Tyriar self-requested a review November 23, 2022 18:51
Tyriar
Tyriar previously approved these changes Nov 23, 2022
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 great 👏

@Tyriar Tyriar enabled auto-merge November 23, 2022 19:38
@rwe
Copy link
Contributor Author

rwe commented Nov 23, 2022

By the way, sorry for stomping on that review with the push! I usually rebase my unreviewed branches in bulk just to keep them current and if I don't notice a review beforehand it'll clobber. I'll look into updating my scripts to check for that in the last step, since reviewers also work in batches 😄 Done with that for now unless other change requests pop up.

@Tyriar Tyriar merged commit 6333552 into microsoft:main Nov 23, 2022
@Tyriar
Copy link
Member

Tyriar commented Nov 23, 2022

No worries, that's best in my experience and what I do; don't force push after a review has occurred

@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.

5 participants