Skip to content

Fix: Add special EscapePowershellArgument handling - MOVED to 1611 #1609

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

Closed
wants to merge 3 commits into from
Closed

Fix: Add special EscapePowershellArgument handling - MOVED to 1611 #1609

wants to merge 3 commits into from

Conversation

JustinGrote
Copy link
Collaborator

@JustinGrote JustinGrote commented Oct 29, 2021

This PR has moved to #1611

@JustinGrote JustinGrote reopened this Oct 30, 2021
@JustinGrote JustinGrote changed the title Fix: PowershellArgumentNeedsEscaping does not account for a single quoted string Fix: Add special EscapePowershellArgument handling Oct 30, 2021
@JustinGrote
Copy link
Collaborator Author

JustinGrote commented Oct 30, 2021

@andschwa @rjmholt OK, this works for me, but I question the wisdom of being "overly helpful" on the argument escaping, I think it will just set up editors for failure in the future by submitting commands that are syntactically incorrect but we "fix for them" and then when they try to run those commands in some other context, they fail.

Should we not just return some sort of invalid syntax error instead to the client so that they are able to submit it right the first time?

CC @SeeminglyScience for his opinion as well

@JustinGrote JustinGrote marked this pull request as draft October 30, 2021 16:30
@JustinGrote
Copy link
Collaborator Author

I have a weird uppercase lowercase branch thing happening that is messing with this PR, will resubmit as new PR.

@JustinGrote JustinGrote closed this Nov 1, 2021
@JustinGrote JustinGrote deleted the JustinGrote/issue1608 branch November 1, 2021 18:17
@JustinGrote JustinGrote changed the title Fix: Add special EscapePowershellArgument handling Fix: Add special EscapePowershellArgument handling - MOVED to 1611 Nov 2, 2021
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.

1 participant