-
Notifications
You must be signed in to change notification settings - Fork 59
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
Shell escape continued #75
Conversation
…invoked by wsl.exe run in the same working dir.
… characters but not a space. The space character is appended to long arguments (--argname=value) and flag arguments (-Xvalue), while other arguments are quoted.
I quickly skimmed your changes. Regarding the |
…ommands invoked by wsl.exe run in the same working dir." This reverts commit 3877221.
…teractive_shell() and format_argument_for_non_interactive_shell(). Only the arguments to wslgit are formatted.
I always forget the interactive shell, but now I've added integration tests that run both interactive and non-interactive :) I updated how the arguments are formatted, splitting it in two functions:
Most of changes is updated tests so the change is not that intimidating. |
Is there a reason to not just use |
Yes, the only reason for the non-interactive mode is to avoid starting a Bash process at all, because this significantly slows down |
I just had to try it out... But the code is a lot cleaner that what I have done in this branch, and there is no "add a space to quote" oddities, it's just formatting a valid command string for bash to interpret using normal quoting. I vote for taking the 15 ms penalty in favor of cleaner code 😃 I'll push my branch with the Side note, I use [[ $- == *i* ]] && IS_INTERACTIVE_SHELL=1 This lets me select what parts of the .bashrc should be included or excluded depending on interactive/non-interactive. |
Yes, you are right. Running through bash is not the slow part, but executing all of Therefore, I'll go with your much simpler solution from #76 and close this. |
#54