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.

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 #23703

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 52 additions & 22 deletions base/client.jl
Original file line number Diff line number Diff line change
Expand Up @@ -84,35 +84,65 @@ answer_color() = text_colors[repl_color("JULIA_ANSWER_COLOR", default_color_answ
stackframe_lineinfo_color() = repl_color("JULIA_STACKFRAME_LINEINFO_COLOR", :bold)
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_name = Base.basename(shell[1])
repl_cmd(line::AbstractString, out) = repl_cmd(Val(Sys.iswindows()), line, out)

if isempty(cmd.exec)
function repl_cmd(iswindows::Val{false}, line::AbstractString, out)
shell = shell_split(get(ENV, "JULIA_SHELL", get(ENV,"SHELL","/bin/sh")))
shell_name = basename(shell[1])
cmd = cmd_gen(eval(Main, shell_parse(line)[1]))
if isempty(cmd)
throw(ArgumentError("no cmd to execute"))
elseif cmd.exec[1] == "cd"
new_oldpwd = pwd()
if length(cmd.exec) > 2
throw(ArgumentError("cd method only takes one argument"))
elseif length(cmd.exec) == 2
dir = cmd.exec[2]
if dir == "-"
if !haskey(ENV, "OLDPWD")
error("cd: OLDPWD not set")
end
cd(ENV["OLDPWD"])
else
cd(@static Sys.iswindows() ? dir : readchomp(`$shell -c "echo $(shell_escape(dir))"`))
elseif cmd[1] == "cd" && length(cmd) <= 2
repl_cd(cmd, shell, out)
else
run(ignorestatus(isa(STDIN, TTY) ? `$shell -i -c "$(shell_wrap_true(shell_name, cmd))"` : `$shell -c "$(shell_wrap_true(shell_name, cmd))"`))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be written a little more cleanly as

ttyopt = STDIN isa TTY ? "-i" : ""
run(ignorestatus(`$shell $ttyopt -c "$(shell_wrap_true(shell_name, cmd))"`))

since -i is the only thing that differs between the two branches.

Copy link
Contributor Author

@GregPlowman GregPlowman Sep 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes of course, will update.
Although probably better to use command syntax rather than strings:

ttyopt = STDIN isa TTY ? `-i` : ``

https://discourse.julialang.org/t/interpolating-an-empty-string-inside-backticks/2428/3

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, good call. I had forgotten about that.

end
nothing
end

function repl_cmd(iswindows::Val{true}, line::AbstractString, out)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use Val

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'm thinking I will just use a single run-time if statement within a common repl_cmd method.

Just for my own edification, why is using Val not recommended? Is it a performance issue?
I thought iswindows() would be known at compile time?
Are there any cases where using Val is OK?

shell = shell_split(get(ENV, "JULIA_SHELL", "cmd"))
shell_name = isempty(shell) ? "" : lowercase(splitext(basename(shell[1]))[1])
cmd = cmd_gen(eval(Main, shell_parse(line)[1]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there shouldn't be any calls to eval

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eliminated in the updated PR

if isempty(cmd)
throw(ArgumentError("no cmd to execute"))
elseif cmd[1] == "cd" && length(cmd) <= 2
repl_cd(cmd, shell, out)
else
interpolated_line = eval(Main, parse(string('"', escape_string(line), '"')))
if shell_name == ""
command = cmd
elseif shell_name == "cmd"
command = Cmd(`$shell /s /c $(string('"', interpolated_line, '"'))`, windows_verbatim=true)
elseif shell_name == "powershell"
command = `$shell -Command $interpolated_line`
elseif shell_name == "busybox"
command = `$shell sh -c $interpolated_line`
else
command = `$shell $interpolated_line`
end
run(ignorestatus(command))
end
nothing
end

function repl_cd(cmd::Cmd, shell, out)
new_oldpwd = pwd()
if length(cmd) == 2
dir = cmd[2]
if dir == "-"
if !haskey(ENV, "OLDPWD")
error("cd: OLDPWD not set")
end
cd(ENV["OLDPWD"])
else
cd()
cd(@static Sys.iswindows() ? dir : readchomp(`$shell -c "echo $(shell_escape(dir))"`))
end
ENV["OLDPWD"] = new_oldpwd
println(out, pwd())
else
run(ignorestatus(@static Sys.iswindows() ? cmd : (isa(STDIN, TTY) ? `$shell -i -c "$(shell_wrap_true(shell_name, cmd))"` : `$shell -c "$(shell_wrap_true(shell_name, cmd))"`)))
cd()
end
nothing
ENV["OLDPWD"] = new_oldpwd
println(out, pwd())
end

function shell_wrap_true(shell_name, cmd)
Expand Down
8 changes: 2 additions & 6 deletions base/repl/REPL.jl
Original file line number Diff line number Diff line change
Expand Up @@ -779,13 +779,9 @@ function setup_interface(
(repl.envcolors ? Base.input_color : repl.input_color) : "",
repl = repl,
complete = ShellCompletionProvider(),
# Transform "foo bar baz" into `foo bar baz` (shell quoting)
# and pass into Base.repl_cmd for processing (handles `ls` and `cd`
# special)
# Pass entered line into Base.repl_cmd for processing (handles `cd` special)
on_done = respond(repl, julia_prompt) do line
Expr(:call, :(Base.repl_cmd),
:(Base.cmd_gen($(Base.shell_parse(line)[1]))),
outstream(repl))
Expr(:call, :(Base.repl_cmd), line, outstream(repl))
end)


Expand Down