From 6f2ef051951f7cf28b0dea3f5e8c9a71f026604e Mon Sep 17 00:00:00 2001 From: Jason Gedge Date: Sun, 26 Apr 2020 21:22:01 -0400 Subject: [PATCH] Resolves https://github.com/nushell/nushell/issues/1658 (#1660) For example, when running the following: crates/nu-cli/src nushell currently parses this as an external command. Before running the command, we check to see if it's a directory. If it is, we "auto cd" into that directory, otherwise we go through normal external processing. If we put a trailing slash on it though, shells typically interpret that as "user is explicitly referencing directory". So crates/nu-cli/src/ should not be interpreted as "run an external command". We intercept a trailing slash in the head position of a command in a pipeline as such, and inject a `cd` internal command. --- crates/nu-cli/src/cli.rs | 12 +- crates/nu-cli/src/commands/cd.rs | 17 +- crates/nu-cli/src/commands/run_external.rs | 185 +++++++++++++++----- crates/nu-cli/src/shell/filesystem_shell.rs | 27 +-- crates/nu-cli/src/shell/help_shell.rs | 15 +- crates/nu-cli/src/shell/shell.rs | 3 +- crates/nu-cli/src/shell/shell_manager.rs | 5 +- crates/nu-cli/src/shell/value_shell.rs | 23 +-- tests/shell/pipeline/commands/external.rs | 21 ++- 9 files changed, 223 insertions(+), 85 deletions(-) diff --git a/crates/nu-cli/src/cli.rs b/crates/nu-cli/src/cli.rs index c9fae069c9..d02a7b94d4 100644 --- a/crates/nu-cli/src/cli.rs +++ b/crates/nu-cli/src/cli.rs @@ -223,7 +223,8 @@ fn create_default_starship_config() -> Option { } pub fn create_default_context( - syncer: &mut crate::env::environment_syncer::EnvironmentSyncer, + syncer: &mut crate::EnvironmentSyncer, + interactive: bool, ) -> Result> { syncer.load_environment(); @@ -348,7 +349,7 @@ pub fn create_default_context( whole_stream_command(FromIcs), whole_stream_command(FromVcf), // "Private" commands (not intended to be accessed directly) - whole_stream_command(RunExternalCommand), + whole_stream_command(RunExternalCommand { interactive }), ]); cfg_if::cfg_if! { @@ -378,7 +379,7 @@ pub async fn run_vec_of_pipelines( redirect_stdin: bool, ) -> Result<(), Box> { let mut syncer = crate::EnvironmentSyncer::new(); - let mut context = crate::create_default_context(&mut syncer)?; + let mut context = create_default_context(&mut syncer, false)?; let _ = crate::load_plugins(&mut context); @@ -477,8 +478,8 @@ pub async fn cli() -> Result<(), Box> { #[cfg(not(windows))] const DEFAULT_COMPLETION_MODE: CompletionType = CompletionType::List; - let mut syncer = crate::env::environment_syncer::EnvironmentSyncer::new(); - let mut context = create_default_context(&mut syncer)?; + let mut syncer = crate::EnvironmentSyncer::new(); + let mut context = create_default_context(&mut syncer, true)?; let _ = load_plugins(&mut context); @@ -911,7 +912,6 @@ pub fn print_err(err: ShellError, host: &dyn Host, source: &Text) { #[cfg(test)] mod tests { - #[quickcheck] fn quickcheck_parse(data: String) -> bool { if let Ok(lite_block) = nu_parser::lite_parse(&data, 0) { diff --git a/crates/nu-cli/src/commands/cd.rs b/crates/nu-cli/src/commands/cd.rs index e14543c1d0..8f766545b0 100644 --- a/crates/nu-cli/src/commands/cd.rs +++ b/crates/nu-cli/src/commands/cd.rs @@ -1,7 +1,16 @@ use crate::commands::WholeStreamCommand; use crate::prelude::*; + +use std::path::PathBuf; + use nu_errors::ShellError; use nu_protocol::{Signature, SyntaxShape}; +use nu_source::Tagged; + +#[derive(Deserialize)] +pub struct CdArgs { + pub(crate) path: Option>, +} pub struct Cd; @@ -27,12 +36,10 @@ impl WholeStreamCommand for Cd { args: CommandArgs, registry: &CommandRegistry, ) -> Result { - cd(args, registry) + args.process(registry, cd)?.run() } } -fn cd(args: CommandArgs, registry: &CommandRegistry) -> Result { - let shell_manager = args.shell_manager.clone(); - let args = args.evaluate_once(registry)?; - shell_manager.cd(args) +fn cd(args: CdArgs, context: RunnableContext) -> Result { + context.shell_manager.cd(args, &context) } diff --git a/crates/nu-cli/src/commands/run_external.rs b/crates/nu-cli/src/commands/run_external.rs index bb9dd97014..f7f9a16af6 100644 --- a/crates/nu-cli/src/commands/run_external.rs +++ b/crates/nu-cli/src/commands/run_external.rs @@ -1,29 +1,39 @@ +use crate::commands::cd::CdArgs; use crate::commands::classified::external; use crate::commands::WholeStreamCommand; use crate::prelude::*; use derive_new::new; use parking_lot::Mutex; +use std::path::PathBuf; use nu_errors::ShellError; use nu_protocol::hir::{Expression, ExternalArgs, ExternalCommand, Literal, SpannedExpression}; use nu_protocol::{ReturnSuccess, Signature, SyntaxShape}; +use nu_source::Tagged; #[derive(Deserialize)] pub struct RunExternalArgs {} #[derive(new)] -pub struct RunExternalCommand; +pub struct RunExternalCommand { + /// Whether or not nushell is being used in an interactive context + pub(crate) interactive: bool, +} -fn spanned_expression_to_string(expr: SpannedExpression) -> String { +fn spanned_expression_to_string(expr: SpannedExpression) -> Result { if let SpannedExpression { expr: Expression::Literal(Literal::String(s)), .. } = expr { - s + Ok(s) } else { - "notacommand!!!".to_string() + Err(ShellError::labeled_error( + "Expected string for command name", + "expected string", + expr.span, + )) } } @@ -45,7 +55,7 @@ impl WholeStreamCommand for RunExternalCommand { args: CommandArgs, registry: &CommandRegistry, ) -> Result { - let positionals = args.call_info.args.positional.ok_or_else(|| { + let positionals = args.call_info.args.positional.clone().ok_or_else(|| { ShellError::untagged_runtime_error("positional arguments unexpectedly empty") })?; @@ -53,49 +63,89 @@ impl WholeStreamCommand for RunExternalCommand { let name = positionals .next() - .map(spanned_expression_to_string) .ok_or_else(|| { - ShellError::untagged_runtime_error( - "run_external unexpectedly missing external name positional arg", - ) - })?; + ShellError::untagged_runtime_error("run_external called with no arguments") + }) + .and_then(spanned_expression_to_string)?; - let command = ExternalCommand { - name, - name_tag: args.call_info.name_tag.clone(), - args: ExternalArgs { - list: positionals.collect(), - span: args.call_info.args.span, - }, + let mut external_context = { + #[cfg(windows)] + { + Context { + registry: registry.clone(), + host: args.host.clone(), + shell_manager: args.shell_manager.clone(), + ctrl_c: args.ctrl_c.clone(), + current_errors: Arc::new(Mutex::new(vec![])), + windows_drives_previous_cwd: Arc::new(Mutex::new( + std::collections::HashMap::new(), + )), + } + } + #[cfg(not(windows))] + { + Context { + registry: registry.clone(), + host: args.host.clone(), + shell_manager: args.shell_manager.clone(), + ctrl_c: args.ctrl_c.clone(), + current_errors: Arc::new(Mutex::new(vec![])), + } + } }; - let mut external_context; - #[cfg(windows)] - { - external_context = Context { - registry: registry.clone(), - host: args.host.clone(), - shell_manager: args.shell_manager.clone(), - ctrl_c: args.ctrl_c.clone(), - current_errors: Arc::new(Mutex::new(vec![])), - windows_drives_previous_cwd: Arc::new(Mutex::new(std::collections::HashMap::new())), - }; - } - #[cfg(not(windows))] - { - external_context = Context { - registry: registry.clone(), - host: args.host.clone(), - shell_manager: args.shell_manager.clone(), - ctrl_c: args.ctrl_c.clone(), - current_errors: Arc::new(Mutex::new(vec![])), - }; - } - let scope = args.call_info.scope.clone(); + let is_interactive = self.interactive; - let is_last = args.call_info.args.is_last; - let input = args.input; let stream = async_stream! { + let command = ExternalCommand { + name, + name_tag: args.call_info.name_tag.clone(), + args: ExternalArgs { + list: positionals.collect(), + span: args.call_info.args.span, + }, + }; + + // If we're in interactive mode, we will "auto cd". That is, instead of interpreting + // this as an external command, we will see it as a path and `cd` into it. + if is_interactive { + if let Some(path) = maybe_autocd_dir(&command, &mut external_context).await { + let cd_args = CdArgs { + path: Some(Tagged { + item: PathBuf::from(path), + tag: args.call_info.name_tag.clone(), + }) + }; + + let context = RunnableContext { + input: InputStream::empty(), + shell_manager: external_context.shell_manager.clone(), + host: external_context.host.clone(), + ctrl_c: external_context.ctrl_c.clone(), + registry: external_context.registry.clone(), + name: args.call_info.name_tag.clone(), + }; + + let result = external_context.shell_manager.cd(cd_args, &context); + match result { + Ok(mut stream) => { + while let Some(value) = stream.next().await { + yield value; + } + }, + Err(e) => { + yield Err(e); + }, + _ => {} + } + + return; + } + } + + let scope = args.call_info.scope.clone(); + let is_last = args.call_info.args.is_last; + let input = args.input; let result = external::run_external_command( command, &mut external_context, @@ -120,3 +170,54 @@ impl WholeStreamCommand for RunExternalCommand { Ok(stream.to_output_stream()) } } + +#[allow(unused_variables)] +async fn maybe_autocd_dir<'a>(cmd: &ExternalCommand, ctx: &mut Context) -> Option { + // We will "auto cd" if + // - the command name ends in a path separator, or + // - it's not a command on the path and no arguments were given. + let name = &cmd.name; + let path_name = if name.ends_with(std::path::MAIN_SEPARATOR) + || (cmd.args.is_empty() + && PathBuf::from(name).is_dir() + && dunce::canonicalize(name).is_ok() + && ichwh::which(&name).await.unwrap_or(None).is_none()) + { + Some(name) + } else { + None + }; + + path_name.map(|name| { + #[cfg(windows)] + { + if name.ends_with(':') { + // This looks like a drive shortcut. We need to a) switch drives and b) go back to the previous directory we were viewing on that drive + // But first, we need to save where we are now + let current_path = ctx.shell_manager.path(); + + let split_path: Vec<_> = current_path.split(':').collect(); + if split_path.len() > 1 { + ctx.windows_drives_previous_cwd + .lock() + .insert(split_path[0].to_string(), current_path); + } + + let name = name.to_uppercase(); + let new_drive: Vec<_> = name.split(':').collect(); + + if let Some(val) = ctx.windows_drives_previous_cwd.lock().get(new_drive[0]) { + val.to_string() + } else { + name + } + } else { + name.to_string() + } + } + #[cfg(not(windows))] + { + name.to_string() + } + }) +} diff --git a/crates/nu-cli/src/shell/filesystem_shell.rs b/crates/nu-cli/src/shell/filesystem_shell.rs index 6c4eef4971..f88269b2b5 100644 --- a/crates/nu-cli/src/shell/filesystem_shell.rs +++ b/crates/nu-cli/src/shell/filesystem_shell.rs @@ -1,3 +1,4 @@ +use crate::commands::cd::CdArgs; use crate::commands::command::EvaluatedWholeStreamCommandArgs; use crate::commands::cp::CopyArgs; use crate::commands::ls::LsArgs; @@ -10,9 +11,7 @@ use crate::prelude::*; use crate::shell::completer::NuCompleter; use crate::shell::shell::Shell; use crate::utils::FileStructure; -use nu_errors::ShellError; -use nu_parser::expand_ndots; -use nu_protocol::{Primitive, ReturnSuccess, UntaggedValue}; + use rustyline::completion::FilenameCompleter; use rustyline::hint::{Hinter, HistoryHinter}; use std::collections::HashMap; @@ -21,6 +20,11 @@ use std::path::{Component, Path, PathBuf}; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; +use nu_errors::ShellError; +use nu_parser::expand_ndots; +use nu_protocol::{Primitive, ReturnSuccess, UntaggedValue}; +use nu_source::Tagged; + pub struct FilesystemShell { pub(crate) path: String, pub(crate) last_path: String, @@ -171,21 +175,20 @@ impl Shell for FilesystemShell { Ok(stream.interruptible(ctrl_c).to_output_stream()) } - fn cd(&self, args: EvaluatedWholeStreamCommandArgs) -> Result { - let path = match args.nth(0) { + fn cd(&self, args: CdArgs, name: Tag) -> Result { + let path = match args.path { None => match dirs::home_dir() { Some(o) => o, _ => { return Err(ShellError::labeled_error( "Cannot change to home directory", "cannot go to home", - &args.call_info.name_tag, + &name, )) } }, Some(v) => { - let target = v.as_path()?; - + let Tagged { item: target, tag } = v; if target == Path::new("-") { PathBuf::from(&self.last_path) } else { @@ -193,7 +196,7 @@ impl Shell for FilesystemShell { ShellError::labeled_error( "Cannot change to directory", "directory not found", - &v.tag, + &tag, ) })?; @@ -201,7 +204,7 @@ impl Shell for FilesystemShell { return Err(ShellError::labeled_error( "Cannot change to directory", "is not a directory", - &v.tag, + &tag, )); } @@ -216,7 +219,7 @@ impl Shell for FilesystemShell { ShellError::labeled_error( "Cannot change to directory", format!("cannot stat ({})", e), - &v.tag, + &tag, ) })?; @@ -224,7 +227,7 @@ impl Shell for FilesystemShell { return Err(ShellError::labeled_error( "Cannot change to directory", "permission denied", - &v.tag, + &tag, )); } } diff --git a/crates/nu-cli/src/shell/help_shell.rs b/crates/nu-cli/src/shell/help_shell.rs index fc32adc39f..6b5e333c85 100644 --- a/crates/nu-cli/src/shell/help_shell.rs +++ b/crates/nu-cli/src/shell/help_shell.rs @@ -1,3 +1,4 @@ +use crate::commands::cd::CdArgs; use crate::commands::command::EvaluatedWholeStreamCommandArgs; use crate::commands::cp::CopyArgs; use crate::commands::ls::LsArgs; @@ -7,12 +8,15 @@ use crate::commands::rm::RemoveArgs; use crate::data::command_dict; use crate::prelude::*; use crate::shell::shell::Shell; + +use std::ffi::OsStr; +use std::path::PathBuf; + use nu_errors::ShellError; use nu_protocol::{ Primitive, ReturnSuccess, ShellTypeName, TaggedDictBuilder, UntaggedValue, Value, }; -use std::ffi::OsStr; -use std::path::PathBuf; +use nu_source::Tagged; #[derive(Clone, Debug)] pub struct HelpShell { @@ -149,12 +153,11 @@ impl Shell for HelpShell { Ok(output.into()) } - fn cd(&self, args: EvaluatedWholeStreamCommandArgs) -> Result { - let path = match args.nth(0) { + fn cd(&self, args: CdArgs, _name: Tag) -> Result { + let path = match args.path { None => "/".to_string(), Some(v) => { - let target = v.as_path()?; - + let Tagged { item: target, .. } = v; let mut cwd = PathBuf::from(&self.path); if target == PathBuf::from("..") { diff --git a/crates/nu-cli/src/shell/shell.rs b/crates/nu-cli/src/shell/shell.rs index 79c6fb78fa..da0432a1f5 100644 --- a/crates/nu-cli/src/shell/shell.rs +++ b/crates/nu-cli/src/shell/shell.rs @@ -1,3 +1,4 @@ +use crate::commands::cd::CdArgs; use crate::commands::command::EvaluatedWholeStreamCommandArgs; use crate::commands::cp::CopyArgs; use crate::commands::ls::LsArgs; @@ -18,7 +19,7 @@ pub trait Shell: std::fmt::Debug { args: LsArgs, context: &RunnablePerItemContext, ) -> Result; - fn cd(&self, args: EvaluatedWholeStreamCommandArgs) -> Result; + fn cd(&self, args: CdArgs, name: Tag) -> Result; fn cp(&self, args: CopyArgs, name: Tag, path: &str) -> Result; fn mkdir(&self, args: MkdirArgs, name: Tag, path: &str) -> Result; fn mv(&self, args: MoveArgs, name: Tag, path: &str) -> Result; diff --git a/crates/nu-cli/src/shell/shell_manager.rs b/crates/nu-cli/src/shell/shell_manager.rs index 1b9eefa8a9..8ad03c1f91 100644 --- a/crates/nu-cli/src/shell/shell_manager.rs +++ b/crates/nu-cli/src/shell/shell_manager.rs @@ -1,3 +1,4 @@ +use crate::commands::cd::CdArgs; use crate::commands::command::{EvaluatedWholeStreamCommandArgs, RunnablePerItemContext}; use crate::commands::cp::CopyArgs; use crate::commands::ls::LsArgs; @@ -141,10 +142,10 @@ impl ShellManager { env[self.current_shell()].ls(args, context) } - pub fn cd(&self, args: EvaluatedWholeStreamCommandArgs) -> Result { + pub fn cd(&self, args: CdArgs, context: &RunnableContext) -> Result { let env = self.shells.lock(); - env[self.current_shell()].cd(args) + env[self.current_shell()].cd(args, context.name.clone()) } pub fn cp( diff --git a/crates/nu-cli/src/shell/value_shell.rs b/crates/nu-cli/src/shell/value_shell.rs index 42af8bc08b..8ecd6a306e 100644 --- a/crates/nu-cli/src/shell/value_shell.rs +++ b/crates/nu-cli/src/shell/value_shell.rs @@ -1,3 +1,4 @@ +use crate::commands::cd::CdArgs; use crate::commands::command::EvaluatedWholeStreamCommandArgs; use crate::commands::cp::CopyArgs; use crate::commands::ls::LsArgs; @@ -7,11 +8,14 @@ use crate::commands::rm::RemoveArgs; use crate::prelude::*; use crate::shell::shell::Shell; use crate::utils::ValueStructure; -use nu_errors::ShellError; -use nu_protocol::{ReturnSuccess, ShellTypeName, UntaggedValue, Value}; + use std::ffi::OsStr; use std::path::{Path, PathBuf}; +use nu_errors::ShellError; +use nu_protocol::{ReturnSuccess, ShellTypeName, UntaggedValue, Value}; +use nu_source::Tagged; + #[derive(Clone)] pub struct ValueShell { pub(crate) path: String, @@ -127,19 +131,18 @@ impl Shell for ValueShell { Ok(output.into()) } - fn cd(&self, args: EvaluatedWholeStreamCommandArgs) -> Result { - let destination = args.nth(0); + fn cd(&self, args: CdArgs, name: Tag) -> Result { + let destination = args.path; let path = match destination { None => "/".to_string(), - Some(v) => { - let target = v.as_path()?; - + Some(ref v) => { + let Tagged { item: target, .. } = v; let mut cwd = PathBuf::from(&self.path); - if target == PathBuf::from("..") { + if target == &PathBuf::from("..") { cwd.pop(); - } else if target == PathBuf::from("-") { + } else if target == &PathBuf::from("-") { cwd = PathBuf::from(&self.last_path); } else { match target.to_str() { @@ -169,7 +172,7 @@ impl Shell for ValueShell { return Err(ShellError::labeled_error( "Can not change to path inside", "No such path exists", - &args.call_info.name_tag, + &name, )); } diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs index 0c464c0790..caaa6bb440 100644 --- a/tests/shell/pipeline/commands/external.rs +++ b/tests/shell/pipeline/commands/external.rs @@ -25,7 +25,7 @@ fn automatically_change_directory() { use nu_test_support::playground::Playground; Playground::setup("cd_test_5_1", |dirs, sandbox| { - sandbox.within("autodir").mkdir("bar"); + sandbox.mkdir("autodir"); let actual = nu!( cwd: dirs.test(), @@ -39,6 +39,25 @@ fn automatically_change_directory() { }) } +#[test] +fn automatically_change_directory_with_trailing_slash_and_same_name_as_command() { + use nu_test_support::playground::Playground; + + Playground::setup("cd_test_5_1", |dirs, sandbox| { + sandbox.mkdir("cd"); + + let actual = nu!( + cwd: dirs.test(), + r#" + cd/ + pwd | echo $it + "# + ); + + assert!(actual.ends_with("cd")); + }) +} + mod it_evaluation { use super::nu; use nu_test_support::fs::Stub::{EmptyFile, FileWithContent, FileWithContentToBeTrimmed};