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.

Sign up for GitHub

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

Jump to bottom

Escape single quotes while translating dropped Win32 paths #18007

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

Escape single quotes while translating dropped Win32 paths #18007

wants to merge 1 commit into from
+12 −0

Conversation

a4lg
Copy link

@a4lg a4lg commented Oct 8, 2024 •
edited
Loading

Summary of the Pull Request

This commit escapes single quotes ' (allowed in the Win32 subsystem) with '\'' (finish quote, print a single quote then begin quote again), which is a valid escape in the context of the POSIX shell, when a file/folder is dropped to a WSL tab.

Note: '"'"' on the first submission of this PR.

References and Relevant Issues

Detailed Description of the Pull Request / Additional comments

This is a follow-up of #16214 and fixes #18006.

Validation Steps Performed

PR Checklist

@microsoft-github-policy-service microsoft-github-policy-service bot added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Oct 8, 2024
@a4lg
Copy link
Author

a4lg commented Oct 8, 2024

@microsoft-github-policy-service agree

@lhecker
Copy link
Member

lhecker commented Oct 8, 2024

Would it be simpler to paste with double quotes and handle escaping of significant characters like $, ', or " with backspaces?

@a4lg
Copy link
Author

a4lg commented Oct 8, 2024 •
edited
Loading

@lhecker I would prefer searching for just one character ('), not three (three you mentioned minus ' plus backquote) I guess. Plus, single quote approach makes making mistakes or shell-dependent sequences almost impossible.

The inserting sequence '"'"' is odd (I have to admit) but in both quote cases (double and single), we would not escape so frequently.

@a4lg
Copy link
Author

a4lg commented Oct 8, 2024 •
edited
Loading

Also, escaping just like @sh formatting logic in jq seems simpler. Will change:

Old: '"'"'
New: '\''

Additional references:

  1. POSIX Shell & Utilities: 2.2 Quoting
  2. POSIX Shell & Utilities: 2.2.1 Escape Character (Backslash)
  3. jq 1.7 Manual: Format strings and escaping (Examples)

@DHowett
Copy link
Member

DHowett commented Nov 18, 2024

Hey, sorry about this - I didn't realize we had an open PR in this area when I changed how path translation works. You will probably have conflicts or outright failures once you merge main because isWSL has moved to a farm upstate.

@a4lg
Copy link
Author

a4lg commented Nov 22, 2024 •
edited
Loading

@DHowett I see. I'll check the code again and resubmit the new version later (if #18006 is remaining).

@a4lg
Copy link
Author

a4lg commented Nov 22, 2024

Checked.
Surprisingly, there's no merge conflicts... but fails to compile due to the lost isWSL variable.

The solution seems to be simple and working on it.

@a4lg a4lg changed the title Escape single quotes for WSL Escape single quotes while translating dropped Win32 paths Nov 22, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Product-Terminal The new Windows Terminal. labels Nov 22, 2024
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@a4lg
Copy link
Author

a4lg commented Nov 22, 2024

Overhauled the PR to adopt my changes over @DHowett's.

@a4lg
Escape single quotes while translating dropped Win32 paths
d7b92af 
When file/folder is dropped to the terminal, its path is translated and
quoted with a pair of single quotes if necessary.

However, despite that the Win32 subsystem allows single quote, the
terminal control does not escape it.  It causes a path containing one or
more single quotes incorrect on the POSIX shell context
(see Issue microsoft#18006 for an example).

With this commit, the terminal control escapes a single quote with a
valid escape sequence `'\''` (finish quote, print a single quote then
begin quote again).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lhecker lhecker lhecker approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

WSL paths need further escape of single quotes
3 participants
@a4lg @lhecker @DHowett