-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Make file redirections apply only to external commands #12785
base: main
Are you sure you want to change the base?
Conversation
Why can't we make it work with internal commands too? |
@fdncred Are you referring to the file redirections ( |
Yup. Seems like a UX problem if people have to remember that redirection only works for externals. Seems like we'd want to have the same experience whether it's internal or external. But not necessarily for this PR. I'm fine with incremental moves. |
I'm not sure about it either. Because sometimes it's easy to do something like this: |
IMO, all redirections should either work with internal streams and values, or they all should not work. Having some work and others not work is inconsistent. Following from this, the semantics with some redirections don't quite make sense with internal streams and values. E.g., I don't think Additionally, allowing file redirections on streams and values may create some "gotcha" situations. def cmd [] {
^extern1
^extern2
^extern3 | str contains 'text' # returns a bool
}
# I want to log/redirect the stdout and stderr of the external commands to file,
# but still want to be able to access the return value
if (cmd o+e> log.txt) {
print 'text found'
} Edit: the file redirections also only currently work with string and binary values anyways and so are a little limited. |
I agree that we need to be consistent in everything like this, but I would push that more towards being able to redirect stderr/stdout with internal and external commands as the way we should be consistent. Again, we can start making things consistent with this PR, but we need to have redirection work as well. I mean the entire reason we introduced redirection was to allow people's muscle memory to apply in nushell too. If we limit it to only externals, it kind of defeats the purpose IMO. |
Looking at someone's dotfiles I just noticed this, which I assume will stop working with this PR since http get isn't external. Is that correct or is it only for http get -r https://static.rust-lang.org/rustup/dist/x86_64-unknown-linux-gnu/rustup-init o> rustup-init |
This pr introduces another strange difference between external and internal that I don't understand as a user. My more general question is: Why is there a user side difference if external command essentially is just a |
More strange cases:
|
In that case i think may it work the same way as internal commands? discard all values and return the last one
|
That is correct.
External commands output a stream of arbitrary bytes, but string values in nushell are encoded as utf-8. So, the two are not necessarily compatible. Some commands assume utf-8 and try to split the external command output on lines, creating an error if this is not possible. Other commands collect all of the external command input into a string value (this may consume lots of memory if the external command output is large), and this will also create an error if utf-8 decoding fails. IMO, some of our commands should not be accepting external command output. E.g., what is
This is the one internal command where it might make sense to respect the current stdout and stderr destination. Currently,
That is correct, nushell errors are structured values in the sense that they can be created (e.g., via And yes, external command stderr is a stream of bytes (unstructured data). |
To me it seems that On a related note there are some user assumptions around those redirections behaving more like POSIX e.g. in #12686 |
Hmm, actually I think I might hold off on this for now.
|
Description
File redirections like
out>
anderr>
refer to the stdout and stderr streams of external commands/processes. However, theout>
,out+err>
,out>>
, andout+err>>
file redirections also work with values and list streams, saving any values to the file. This is despite the fact that nushell values and list streams do not quite have a defined "stdout" or "stderr". Currently, the pipe redirections (e>|
ando+e>|
) only work on external commands, erroring if used on a value or list stream.User-Facing Changes
Breaking change:
o>
,o>>
,o+e>
, ando+e>>
now only apply to external commands.