-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
REPL shell mode for Windows #33474
REPL shell mode for Windows #33474
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,15 +31,20 @@ stackframe_lineinfo_color() = repl_color("JULIA_STACKFRAME_LINEINFO_COLOR", :bol | |
stackframe_function_color() = repl_color("JULIA_STACKFRAME_FUNCTION_COLOR", :bold) | ||
|
||
function repl_cmd(cmd, out) | ||
shell = shell_split(get(ENV, "JULIA_SHELL", get(ENV, "SHELL", "/bin/sh"))) | ||
shell_env = get(ENV, "JULIA_SHELL", nothing) | ||
if shell_env === nothing || isempty(shell_env) | ||
shell_env = Sys.iswindows() ? "cmd" : get(ENV, "SHELL", "/bin/sh") | ||
end | ||
shell = shell_split(shell_env) | ||
shell_name = Base.basename(shell[1]) | ||
Sys.iswindows() && (shell_name = lowercase(splitext(shell_name)[1])) # canonicalize for comparisons below | ||
|
||
# Immediately expand all arguments, so that typing e.g. ~/bin/foo works. | ||
cmd.exec .= expanduser.(cmd.exec) | ||
|
||
isempty(cmd.exec) && throw(ArgumentError("no cmd to execute")) | ||
|
||
if isempty(cmd.exec) | ||
throw(ArgumentError("no cmd to execute")) | ||
elseif cmd.exec[1] == "cd" | ||
if cmd.exec[1] == "cd" | ||
new_oldpwd = pwd() | ||
if length(cmd.exec) > 2 | ||
throw(ArgumentError("cd method only takes one argument")) | ||
|
@@ -58,19 +63,52 @@ function repl_cmd(cmd, out) | |
ENV["OLDPWD"] = new_oldpwd | ||
println(out, pwd()) | ||
else | ||
@static if !Sys.iswindows() | ||
local command | ||
if Sys.iswindows() | ||
if shell_name == "cmd" | ||
command = _CMD_execute(shell, cmd) | ||
elseif shell_name in ("powershell", "pwsh") | ||
command = _powershell_execute(shell, cmd) | ||
elseif shell_name == "busybox" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this really necessary ? We don't even ship with busy box anymore... I think this is from older julia code, so my suggestion is to just remove it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. arg .. looks like it is needed for testing on windows... maybe we should look into WSL for testing on windows instead of downloading busybox? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still something about this rubs me the wrong way. We should still try to remove this and try to work around them in the tests. Thoughts welcome There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should make this the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What exactly do you mean by "making this the else block"? if shell_name == "cmd"
command = _CMD_execute(shell, cmd)
elseif shell_name in ("powershell", "pwsh")
command = _powershell_execute(shell, cmd)
else
command = `$shell sh -c $(shell_escape_posixly(cmd))` Is the assumption here that if you don't want |
||
command = `$shell sh -c $(shell_escape_posixly(cmd))` | ||
else | ||
command = `$shell $cmd` | ||
end | ||
else | ||
if shell_name == "fish" | ||
shell_escape_cmd = "begin; $(shell_escape_posixly(cmd)); and true; end" | ||
command = `$shell -c $shell_escape_cmd` | ||
elseif shell_name == "pwsh" | ||
command = _powershell_execute(shell, cmd) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't this do the wrong escaping on Unix? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I'm not sure it's worth the effort to support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Powershell's handling of arguments is known (PowerShell/PowerShell#1995) to be buggy, so can't reliably be used for launching programs anyways. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @vtjnash, that's argument handling on Windows. Presumably on Unix systems There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sadly, no (e.g. PowerShell/PowerShell#10440)
Sadly, it is currently present (although both broken and undocumented—and eventually supposed to be unnecessary) |
||
else | ||
shell_escape_cmd = "($(shell_escape_posixly(cmd))) && true" | ||
command = `$shell -c $shell_escape_cmd` | ||
end | ||
cmd = `$shell -c $shell_escape_cmd` | ||
end | ||
run(ignorestatus(cmd)) | ||
run(ignorestatus(command)) | ||
end | ||
nothing | ||
end | ||
|
||
# process cmd's passed to CMD | ||
_CMD_execute(shell, cmd) = Cmd(`$shell /c $(shell_escape_CMDly(shell_escape_winsomely(cmd)))`, windows_verbatim=true) | ||
|
||
function _powershell_execute(shell, cmd) | ||
# process cmd's passed to powershell | ||
CommandType = nothing | ||
try | ||
CommandType = readchomp(`$shell -Command "Get-Command -- $(shell_escape_PWSH_cmdlet_ly(cmd.exec[1])) | Select-Object -ExpandProperty CommandType"`) | ||
catch | ||
end | ||
# TODO: while CommandType == "Alias"; CommandType = ...; end | ||
if CommandType == "Application" | ||
command = Cmd(`$shell -Command "& $(shell_escape_PWSHly(shell_escape_winsomely(cmd)))"`) | ||
else # handle Function and Cmdlet # TODO: what is the proper handling for the other types (ExternalScript, Script, Workflow, Configuration, and Filter) | ||
command = Cmd(`$shell -Command "& $(shell_escape_PWSH_cmdlet_ly(cmd))"`) | ||
end | ||
return command | ||
end | ||
|
||
# deprecated function--preserved for DocTests.jl | ||
function ip_matches_func(ip, func::Symbol) | ||
for fr in StackTraces.lookup(ip) | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -292,7 +292,7 @@ end | |||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||
shell_escaped_winsomely(args::Union{Cmd,AbstractString...})::String | ||||||||||||||||||||||||||||||||||||||
shell_escaped_winsomely(args::Union{Cmd,AbstractString...}) -> String | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
Convert the collection of strings `args` into single string suitable for passing as the argument | ||||||||||||||||||||||||||||||||||||||
string for a Windows command line. Windows passes the entire command line as a single string to | ||||||||||||||||||||||||||||||||||||||
|
@@ -312,3 +312,69 @@ julia> println(shell_escaped_winsomely("A B\\", "C")) | |||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||
shell_escape_winsomely(args::AbstractString...) = | ||||||||||||||||||||||||||||||||||||||
sprint(print_shell_escaped_winsomely, args..., sizehint=(sum(length, args)) + 3*length(args)) | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
function print_shell_escaped_CMDly(io::IO, arg::AbstractString) | ||||||||||||||||||||||||||||||||||||||
any(c -> c in ('\r', '\n'), arg) && throw(ArgumentError("Encountered unsupported character by CMD.")) | ||||||||||||||||||||||||||||||||||||||
# include " so to avoid toggling behavior of ^ | ||||||||||||||||||||||||||||||||||||||
arg = replace(arg, r"[%!^\"<>&|]" => s"^\0") | ||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Upon closer inspection of https://stackoverflow.com/questions/4094699/how-does-the-windows-command-interpreter-cmd-exe-parse-scripts/4095133#4095133, I realized that |
||||||||||||||||||||||||||||||||||||||
print(io, arg) | ||||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||
shell_escape_CMDly(arg::AbstractString) -> String | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
The unexported `shell_escape_CMDly` function takes a string and escapes any special characters | ||||||||||||||||||||||||||||||||||||||
in such a way that it is safe to pass it as an argument to some `CMD.exe`. This may be useful | ||||||||||||||||||||||||||||||||||||||
in concert with the `windows_verbatim` flag to [`Cmd`](@ref) when constructing process | ||||||||||||||||||||||||||||||||||||||
pipelines. | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
See also [`shell_escape_PWSHly`](@ref). | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
# Example | ||||||||||||||||||||||||||||||||||||||
```jldoctest | ||||||||||||||||||||||||||||||||||||||
julia> println(shell_escape_CMDly("\"A B\\\" & C")) | ||||||||||||||||||||||||||||||||||||||
^"A B\\^" ^& C | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
!important | ||||||||||||||||||||||||||||||||||||||
Due to a peculiar behavior of the CMD, each command after a literal `|` character | ||||||||||||||||||||||||||||||||||||||
(indicating a command pipeline) must have `shell_escape_CMDly` applied twice. For example: | ||||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||||
to_print = "All for 1 & 1 for all!" | ||||||||||||||||||||||||||||||||||||||
run(Cmd(Cmd(["cmd /c \"break | echo \$(shell_escape_CMDly(shell_escape_CMDly(to_print)))"]), windows_verbatim=true)) | ||||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||
shell_escape_CMDly(arg::AbstractString) = sprint(print_shell_escaped_CMDly, arg) | ||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This appears to need tests. Also, can you split this aspect into a separate commit? |
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
function print_shell_escaped_PWSHly(io::IO, arg::AbstractString) | ||||||||||||||||||||||||||||||||||||||
# escape several characters that usually have special meaning | ||||||||||||||||||||||||||||||||||||||
arg = replace(arg, r"[`\"\$#;|><&(){}=]" => s"`\0") | ||||||||||||||||||||||||||||||||||||||
# escape special control chars | ||||||||||||||||||||||||||||||||||||||
arg = replace(replace(replace(arg, '\r' => "`r"), '\t' => "`t"), '\t' => "`t") | ||||||||||||||||||||||||||||||||||||||
print(io, arg) | ||||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||
shell_escape_PWSHly(arg::AbstractString) -> String | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
Escapes special characters so they can be appropriately used with PowerShell. | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
See also [`shell_escape_CMDly`](@ref). | ||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||
shell_escape_PWSHly(arg::AbstractString) = sprint(print_shell_escaped_PWSHly, arg) | ||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's my suggested source for the
Suggested change
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
function print_shell_escaped_PWSH_cmdlet_ly(io::IO, args::AbstractString...) | ||||||||||||||||||||||||||||||||||||||
# often the shortest way to escape a powershell string is to double any single quotes and then wrap the whole thing in single quotes | ||||||||||||||||||||||||||||||||||||||
# (alternatively, we could prefix all the non-word characters with a back-tick and replace newlines with `r and `n) | ||||||||||||||||||||||||||||||||||||||
# but skip the escaping for common cases we always know are safe (e.g. so that named parameters are typically still interpreted correctly) | ||||||||||||||||||||||||||||||||||||||
isword(c::AbstractChar) = '0' <= c <= '9' || 'a' <= c <= 'z' || 'A' <= c <= 'Z' || c == '_' || c == '\\' || c == ':' || c == '/' || c == '-' | ||||||||||||||||||||||||||||||||||||||
join(io, (all(isword, arg) ? arg : string("'", replace(arg, "'" => "''"), "'") for arg in args), " ") | ||||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||
shell_escape_PWSH_cmdlet_ly(args::AbstractString...) -> String | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
Escapes special characters so they can be appropriately used with a PowerShell cmdlet (such as `echo`). | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
See also [`shell_escape_PWSHly`](@ref) and [`shell_escape_winsomely`](@ref). | ||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||
shell_escape_PWSH_cmdlet_ly(args::AbstractString...) = sprint(print_shell_escaped_PWSH_cmdlet_ly, args...) |
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.
This change is needed since we use substitution string in the
shell.jl
file.