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

Closed
wants to merge 4 commits into from
Closed

REPL shell mode for Windows #33427

wants to merge 4 commits into from

Conversation

musm
Copy link
Contributor

@musm musm commented Sep 30, 2019

GregPlowman and others added 4 commits March 26, 2019 16:45
REPL shell mode for Windows

Documentation JULIA_SHELL environment variable

Interacting with Julia docs - xref JULIA_SHELL

Fix whitespace JULIA_SHELL doc

Fix JULIA_SHELL cross-reference

Address code review comments

rebase fix
@musm musm changed the title Shellmodewin REPL shell mode for Windows Sep 30, 2019
@vtjnash
Copy link
Member

vtjnash commented Oct 1, 2019

Could you help land #30614? That’s generally what has prevented this PR from happening thus far.

@musm musm closed this Oct 1, 2019
@mgkuhn
Copy link
Contributor

mgkuhn commented Oct 4, 2019

I'm note sure my code in #30614 is actually applicable for passing a command-line to CMD.EXE itself. It is only of use to get strings past CMD.EXE, to one of the EXE applications that then parse the command-line they received using the Microsoft C (CommandLineToArgvW) conventions (including Julia.EXE). The code in #30614 is not at all of use for passing shell command lines to any other applications that do not use CommandLineToArgvW, including for example all built-in commands of CMD.EXE itself.

Rather than playing around with escaping, I suspect REPL Shell mode needs to work fundamentally different on platforms (such as Windows) that invoke other processes with a single command line, rather than an ARGV array. I suspect this misunderstanding may have stalled progress here.

@musm
Copy link
Contributor Author

musm commented Oct 4, 2019

@mgkuhn Your comment is a bit confusing to me.

Let me try to clarify my interpretation of how things should and do work. Feel free to correct me if I have said something incorrect.

The repl shell mode in julia essentially is supposed to act like running commands through the julia function run(Cmd("hello") or more simply run(command). As such it has to first go through Base.shell_split and other machinery to interpolate julia variables (in particular).

Now to have a reasonably functioning "repl" mode (I put it in quotes, since it's not a true repl mode in the sense of opening up cmd.exe and running commands in there). We need to join these arguments back into a single string according to the "inverse" rules that CommandLineToArgvW uses, which is now implemented by the function winsomely in #33465 . This then gives us a single string we can pass to cmd.exe, but we also need to take care to escape special characters for cmd.exe to avoid any security issues that the julia "repl" mode could have by users messing around in the environment, such as modifying environment variables etc. and thus we sanitize the input my running the single string through other escaping function that are specific to cmd.exe or powershell.exe .

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.

5 participants