-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
adds fallback behavior for non-ms shells #6560
adds fallback behavior for non-ms shells #6560
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple thoughts -- args
should be hoisted instead of being part of an else
branch.
However, I'm wondering about this whole approach. Doesn't pexpect
work on Windows? Why not use the same code path we use on Unix OSes as much as possible, and keep the special-casing to Windows Powershell and cmd.exe?
I'll give it a shot |
|
463d70a
to
fd363ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM -- this class is heavily overloaded and needs to be refactored, but this was a regression in 1.2 and we shouldn't let the refactor block the fix.
fd363ac
to
61091cb
Compare
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.2 1.2
# Navigate to the new working tree
cd .worktrees/backport-1.2
# Create a new branch
git switch --create backport-6560-to-1.2
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 3d422d72159418ca23105fb6ea38f86af62af23f
# Push it to GitHub
git push --set-upstream origin backport-6560-to-1.2
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.2 Then, create a pull request where the |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Pull Request Check List
Resolves: #6495
Confirmed working on MS Powershell, Powershell 7, cmd and git bash