Skip to content

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

MilesCranmer
Copy link
Member

@MilesCranmer MilesCranmer commented May 25, 2025

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 to f 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 struct ShellSpecification{is_windows, shell_sym}. This is a bit cleaner than the manual handling of if !Sys.iswindows() and if 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 overload

Base.prepare_shell_command(::ShellSpecification{false,:my_new_shell}, cmd_or_string) = #= ... =#

Note 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 and repl_cmd_cd methods introduced in this PR could potentially be used for better shell compatibility for Windows (#23597), as you could translate dir into a cd.

Here's the main logic of this change:

struct ShellSpecification{is_windows,shell} end

# Trait to determine if shell needs cmd generation
needs_cmd(::ShellSpecification) = true
needs_cmd(::ShellSpecification{false,:nu}) = false

# Detect cd commands on each shell
is_cd_cmd(::ShellSpecification, cmd::Cmd) = cmd.exec[1] == "cd"
is_cd_cmd(::ShellSpecification, cmd::String) = false
is_cd_cmd(::ShellSpecification{false,:nu}, raw_string::String) = startswith(strip(raw_string), "cd")

function prepare_shell_command(::ShellSpecification{true,shell}, cmd) where {shell}
    return cmd # Previously `if !Sys.iswindows()`
end
function prepare_shell_command(::ShellSpecification{false,shell}, cmd) where {shell}
    # Previously the `else` branch
    shell_escape_cmd = "$(shell_escape_posixly(cmd)) && true"
    return `$shell -c $shell_escape_cmd`
end
function prepare_shell_command(::ShellSpecification{false,:fish}, cmd)
    # Previously the `if shell_name == "fish"` branch
    shell_escape_cmd = "begin; $(shell_escape_posixly(cmd)); and true; end"
    return `fish -c $shell_escape_cmd`
end
function prepare_shell_command(::ShellSpecification{false,:nu}, raw_string)
    # New logic for nushell
    return `nu -c $raw_string`
end

function pre_repl_cmd(raw_string, parsed, out)  # New entry point
    #= ... =#

    shell_spec = ShellSpecification{@static(Sys.iswindows() ? true : false),Symbol(shell_name)}()
    if needs_cmd(shell_spec)
        cmd = Base.cmd_gen(parsed)
        return repl_cmd(shell_spec, cmd, parsed, out)
    else
        # We now avoid parsing to a `Cmd` object if not needed!
        return repl_cmd(shell_spec, raw_string, parsed, out)
    end
end

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.

@MilesCranmer MilesCranmer changed the title feat(REPL): (alternative) Make shell command handling extensible via dispatch; add Nushell feat(REPL): Make shell command handling extensible via dispatch; add Nushell May 25, 2025
@MilesCranmer
Copy link
Member Author

This PR also makes it much easier to add Windows shells. I added some here: #58526

@sandyspiers
Copy link

sandyspiers commented May 27, 2025

Thanks for alternative! With this solution we dont have to manually escape ", nice!

shell> "hello" | save test.txt -f

shell> open test.txt
hello

But the $ is not working for me, escaped or not...

image

@MilesCranmer
Copy link
Member Author

Which version of the code did you run? It works for me

@sandyspiers
Copy link

I cant get e06250f to build, so the above was using fb88aff. But I've also tried with e6fa591 and faced the same issue. Tried with Linux & WSL.

@MilesCranmer
Copy link
Member Author

MilesCranmer commented May 28, 2025

What about if you just add the startup.jl config I give above? See first post. That lets you have this working without needing a custom build. (Also makes it much much easier to test different setups!)

@sandyspiers
Copy link

sandyspiers commented May 28, 2025

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 $, and so its get sent to nushell. With that change, it all seems to work with nushell, although I have not gone back to test with bash.

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.

@MilesCranmer
Copy link
Member Author

Thanks!

Also I noticed some other issues in my startup version, like how typing ; in other contexts would cause an error. Here's the fully fixed one which fixes that: https://gist.github.com/MilesCranmer/0b530cf4602905d548acdfb3bb54ded0

@MilesCranmer
Copy link
Member Author

Regarding this:

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.

I don't think I've ever had this feature. Even with --startup-file=no, pushing the up-arrow does not filter-by-mode, it just goes up through a flat list of historical commands. Are you sure this works for you without the config?

Copy link
Member

@vtjnash vtjnash left a 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

@MilesCranmer
Copy link
Member Author

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:

julia/base/client.jl

Lines 67 to 83 in 1735d8f

@static if !Sys.iswindows()
if shell_name == "fish"
shell_escape_cmd = "begin; $(shell_escape_posixly(cmd)); and true; end"
else
shell_escape_cmd = "($(shell_escape_posixly(cmd))) && true"
end
cmd = `$shell -c $shell_escape_cmd`
end
try
run(ignorestatus(cmd))
catch
# Windows doesn't shell out right now (complex issue), so Julia tries to run the program itself
# Julia throws an exception if it can't find the program, but the stack trace isn't useful
lasterr = current_exceptions()
lasterr = ExceptionStack([(exception = e[1], backtrace = [] ) for e in lasterr])
invokelatest(display_error, lasterr)
end

@MilesCranmer
Copy link
Member Author

MilesCranmer commented May 28, 2025

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 '&&'

@vtjnash
Copy link
Member

vtjnash commented May 28, 2025

Take a look at the code:

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.

@MilesCranmer
Copy link
Member Author

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 run(cmd) on Windows regardless of shell (which is what I believe led to the various Windows user feedback in #23597)

To me I think the best solution would be one of the following options:

  1. disable shell-specific logic entirely and handle all platforms with a plain run(cmd)
  2. make it easier for third party libraries to customize the shell mode behavior (e.g., via the overloadable hook in this PR)
  3. explicitly expand support for shell mode across platforms (e.g., Windows shell support in REPL #58526)
  4. ship a multi-platform shell with Julia that gets used for shell mode

I would be in favor of (2). But I think any of these options are good.

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.

REPL shell mode is incompatible with nushell
3 participants