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

Rewrite which #1144

Merged
merged 12 commits into from Jan 1, 2020
Merged

Rewrite which #1144

merged 12 commits into from Jan 1, 2020

Conversation

avandesa
Copy link
Contributor

@avandesa avandesa commented Jan 1, 2020

This pull request is a re-write of the which command primarily to identify nushell built-in commands. The command now behaves very similarly to GNU which and zsh's which command, while outputting usable nushell objects instead of a plain string. The path searching is also now asynchronous, avoiding blocking the thread.

This was primarily accomplished by writing a new, asynchronous crate called ichwh. It handles all of the path searching, and the nu command manages the arguments and the value conversion.

The command is cross-platform. The ichwh documentation has a note about path searching on windows versus unix.

Documentation for the command is also added by this PR.

Future Work

  • When aliases & functions are added to nu, the command should be updated to detect them
  • Expand the command to accept multiple arguments

Resolves #1067

This expands the built-in `which` command to detect nushell commands
that may have the same name as a binary in the path.
Previously, it would discard any argument besides the first. This allows
`which` to process multiple arguments. It also makes the output a stream
of rows.
In order to use `ichwh`, I'll need to use async_stream. But in order to
avoid lifetime errors with that, I have to process the command args
before using them. I'll admit I don't fully understand what is going on
with the `args.process(...)` function, but it works.
This commit transitions from `which` to `ichwh`. The path search is now
done asynchronously.
Escaped commands passed to wich (e.g., `which "^ls"`), are now searched
before builtins.
This commit resolves two warnings from clippy, in light of nushell#1142.
`ichwh@0.2.1` has support for local paths.
@gitpod-io
Copy link

gitpod-io bot commented Jan 1, 2020

@sophiajt
Copy link
Member

sophiajt commented Jan 1, 2020

Looks good!

@sophiajt sophiajt merged commit b304de8 into nushell:master Jan 1, 2020
@avandesa avandesa deleted the which-expansion branch January 1, 2020 16:50
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.

[Feature Request] Interpret which as a shell builtin
2 participants