-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
feat(REPL): Make shell command handling extensible via dispatch; add Nushell #58525
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
base: master
Are you sure you want to change the base?
feat(REPL): Make shell command handling extensible via dispatch; add Nushell #58525
Conversation
Replace regex with explicit replace. Remove @static macro and use `Sys.iswindows()` instead Co-authored-by: Jakob Nybo Nissen <jakobnybonissen@gmail.com>
This PR also makes it much easier to add Windows shells. I added some here: #58526 |
Which version of the code did you run? It works for me |
What about if you just add the |
Ok so I managed to get the config you provided to work, but I had to change from on_done=respond(repl, julia_prompt) do line
return Expr(
:call,
pre_repl_cmd,
line::String,
Base.shell_parse(line)[1],
outstream(repl),
) to on_done=respond(repl, julia_prompt) do line
return Expr(
:call,
pre_repl_cmd,
line::String,
Base.shell_parse(line, false)[1],
outstream(repl),
) This stops it from interpolating the Also, one quirk I found is that using up-arrow to scroll back through history doesn't get you back into shell-prompt mode, like it would without this config. Although I haven't tried this when building from source yet, so that might be the reason. |
Thanks! Also I noticed some other issues in my startup version, like how typing |
Regarding this:
I don't think I've ever had this feature. Even with |
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 shell>
prompt is not a nu shell, or powershell, or posix shell, or some other random environment like that. It is a julia shell, for interpolating julia $values
into commands for running external programs with those values, equivalent to typing run(`$values`)
. If you wanted a nu shell, you are intended to specify running that with something like shell> nu
I don't think this is correct. Take a look at the code: Lines 67 to 83 in 1735d8f
|
This is why, if you launch Julia (current Julia) from nushell, and then try to use shell mode, it fails with the following error message: > julia --startup-file=no
_
_ _ _(_)_ | Documentation: https://docs.julialang.org
(_) | (_) (_) |
_ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 1.11.5 (2025-04-14)
_/ |\__'_|_|_|\__'_| | Official https://julialang.org/ release
|__/ |
shell> ls
Error: nu::parser::shell_andand
× The '&&' operator is not supported in Nushell
╭─[source:1:6]
1 │ (ls) && true
· ─┬
· ╰── instead of '&&', use ';' or 'and'
╰────
help: use ';' instead of the shell '&&', or 'and' instead of the boolean '&&' |
That seems like it was supposed to be deleted in #25006. It is supposed to be calling into some sort of posix shell in that part of the code (so that shell_escape works correctly and some shell aliases / builtins work correctly), but not provide a posix shell interface. I opened a PR to do that cleanup now. |
Gotcha, ok. By the way, this section: if !Sys.iswindows()
shell = shell_split(get(ENV, "JULIA_SHELL", get(ENV, "SHELL", "/bin/sh")))
shell_escape_cmd = shell_escape_posixly(cmd)
cmd = `$shell -c $shell_escape_cmd`
end results in different behaviour across platforms, even when using identical multi-platform shells like (Git) Bash/PowerShell/Nushell. This means that shell mode executes commands via the user's shell explicitly on Unix, but switches to To me I think the best solution would be one of the following options:
I would be in favor of (2). But I think any of these options are good. |
This is an alternative to #58523. As in that PR, this cleans up the branching over shells for better extensibility and maintainability. It also adds a more compatible interface with nushell. This supersedes #58523 and fixes #54291.
This makes it so that nushell closures and pipes work, like
ls | sort-by size | each {|f| $f.name}
. Before this, in #58523, it would result in an error due tof
being interpolated. And with #58413, it would result in an error due to|
being treated as a string.This introduces a function
prepare_shell_command
which dispatches on a structShellSpecification{is_windows, shell_sym}
. This is a bit cleaner than the manual handling ofif !Sys.iswindows()
andif shell_name == "fish"
. It also allows users to customize REPL logic for new shells, should they wish to. This addresses the issue I had with #54291 where there was simply no way to customize the REPL shell command handling. Now, the next time a user wants to interface with their shell, they can just overloadNote that shells like Nushell, which work better with raw strings, can now correctly handle complex syntax like pipes (e.g.,
ls | sort-by name
) and closures with$
variables.The
is_cd_cmd
andrepl_cmd_cd
methods introduced in this PR could potentially be used for better shell compatibility for Windows (#23597), as you could translatedir
into acd
.Here's the main logic of this change:
If you want to try this without re-compiling Julia, here's some code you can put in your startup.jl: https://gist.github.com/MilesCranmer/0b530cf4602905d548acdfb3bb54ded0. This code also works on 1.11 so this lets you use nushell on stable Julia! This startup.jl is obviously quite complex which is I think part of the benefit of having these various overloadable methods.