From 47d987d37f0ff4e3a0af5c4de9e2c5911ec2dd69 Mon Sep 17 00:00:00 2001 From: Jason Gedge Date: Sat, 18 Jan 2020 21:25:07 -0500 Subject: [PATCH] Add ctrl_c to RunnablePerItemContext. (#1239) Also, this commit makes `ls` a per-item command. A command that processes things item by item may still take some time to stream out the results from a single item. For example, `ls` on a directory with a lot of files could be interrupted in the middle of showing all of these files. --- src/cli.rs | 2 +- src/commands/command.rs | 4 ++++ src/commands/cp.rs | 4 +++- src/commands/ls.rs | 25 ++++++++++++++----------- src/commands/mkdir.rs | 4 +++- src/commands/mv.rs | 4 +++- src/commands/rm.rs | 4 +++- src/prelude.rs | 1 + src/shell/filesystem_shell.rs | 13 ++++++------- src/shell/help_shell.rs | 7 +++---- src/shell/shell.rs | 7 +++---- src/shell/shell_manager.rs | 9 ++++----- src/shell/value_shell.rs | 11 +++++------ tests/commands/ls.rs | 26 ++++++++++++++++++++++++++ 14 files changed, 79 insertions(+), 42 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 28f85498e..9ca315cd0 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -251,7 +251,7 @@ pub async fn cli() -> Result<(), Box> { context.add_commands(vec![ // System/file operations whole_stream_command(Pwd), - whole_stream_command(Ls), + per_item_command(Ls), whole_stream_command(Cd), whole_stream_command(Env), per_item_command(Remove), diff --git a/src/commands/command.rs b/src/commands/command.rs index f04bc8f17..dfcca6671 100644 --- a/src/commands/command.rs +++ b/src/commands/command.rs @@ -42,6 +42,7 @@ pub trait CallInfoExt { fn process<'de, T: Deserialize<'de>>( &self, shell_manager: &ShellManager, + ctrl_c: Arc, callback: fn(T, &RunnablePerItemContext) -> Result, ) -> Result, ShellError>; } @@ -50,6 +51,7 @@ impl CallInfoExt for CallInfo { fn process<'de, T: Deserialize<'de>>( &self, shell_manager: &ShellManager, + ctrl_c: Arc, callback: fn(T, &RunnablePerItemContext) -> Result, ) -> Result, ShellError> { let mut deserializer = ConfigDeserializer::from_call_info(self.clone()); @@ -59,6 +61,7 @@ impl CallInfoExt for CallInfo { context: RunnablePerItemContext { shell_manager: shell_manager.clone(), name: self.name_tag.clone(), + ctrl_c, }, callback, }) @@ -219,6 +222,7 @@ impl CommandArgs { pub struct RunnablePerItemContext { pub shell_manager: ShellManager, pub name: Tag, + pub ctrl_c: Arc, } pub struct RunnableContext { diff --git a/src/commands/cp.rs b/src/commands/cp.rs index 4919dc375..58457431a 100644 --- a/src/commands/cp.rs +++ b/src/commands/cp.rs @@ -38,7 +38,9 @@ impl PerItemCommand for Cpy { raw_args: &RawCommandArgs, _input: Value, ) -> Result { - call_info.process(&raw_args.shell_manager, cp)?.run() + call_info + .process(&raw_args.shell_manager, raw_args.ctrl_c.clone(), cp)? + .run() } } diff --git a/src/commands/ls.rs b/src/commands/ls.rs index 957c21232..62aada27f 100644 --- a/src/commands/ls.rs +++ b/src/commands/ls.rs @@ -1,7 +1,7 @@ -use crate::commands::WholeStreamCommand; +use crate::commands::command::RunnablePerItemContext; use crate::prelude::*; use nu_errors::ShellError; -use nu_protocol::{Signature, SyntaxShape}; +use nu_protocol::{CallInfo, Signature, SyntaxShape, Value}; use nu_source::Tagged; use std::path::PathBuf; @@ -9,11 +9,11 @@ pub struct Ls; #[derive(Deserialize)] pub struct LsArgs { - path: Option>, - full: bool, + pub path: Option>, + pub full: bool, } -impl WholeStreamCommand for Ls { +impl PerItemCommand for Ls { fn name(&self) -> &str { "ls" } @@ -34,14 +34,17 @@ impl WholeStreamCommand for Ls { fn run( &self, - args: CommandArgs, - registry: &CommandRegistry, + call_info: &CallInfo, + _registry: &CommandRegistry, + raw_args: &RawCommandArgs, + _input: Value, ) -> Result { - args.process(registry, ls)?.run() - // ls(args, registry) + call_info + .process(&raw_args.shell_manager, raw_args.ctrl_c.clone(), ls)? + .run() } } -fn ls(LsArgs { path, full }: LsArgs, context: RunnableContext) -> Result { - context.shell_manager.ls(path, &context, full) +fn ls(args: LsArgs, context: &RunnablePerItemContext) -> Result { + context.shell_manager.ls(args, context) } diff --git a/src/commands/mkdir.rs b/src/commands/mkdir.rs index befce8ad2..f94f6eb01 100644 --- a/src/commands/mkdir.rs +++ b/src/commands/mkdir.rs @@ -33,7 +33,9 @@ impl PerItemCommand for Mkdir { raw_args: &RawCommandArgs, _input: Value, ) -> Result { - call_info.process(&raw_args.shell_manager, mkdir)?.run() + call_info + .process(&raw_args.shell_manager, raw_args.ctrl_c.clone(), mkdir)? + .run() } } diff --git a/src/commands/mv.rs b/src/commands/mv.rs index 91f395e6f..8118777cf 100644 --- a/src/commands/mv.rs +++ b/src/commands/mv.rs @@ -44,7 +44,9 @@ impl PerItemCommand for Move { raw_args: &RawCommandArgs, _input: Value, ) -> Result { - call_info.process(&raw_args.shell_manager, mv)?.run() + call_info + .process(&raw_args.shell_manager, raw_args.ctrl_c.clone(), mv)? + .run() } } diff --git a/src/commands/rm.rs b/src/commands/rm.rs index 9b8f67ffe..dbc6de7fa 100644 --- a/src/commands/rm.rs +++ b/src/commands/rm.rs @@ -41,7 +41,9 @@ impl PerItemCommand for Remove { raw_args: &RawCommandArgs, _input: Value, ) -> Result { - call_info.process(&raw_args.shell_manager, rm)?.run() + call_info + .process(&raw_args.shell_manager, raw_args.ctrl_c.clone(), rm)? + .run() } } diff --git a/src/prelude.rs b/src/prelude.rs index 2037bbf6d..ef1ad5d8b 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -72,6 +72,7 @@ pub(crate) use nu_protocol::{errln, outln}; pub(crate) use crate::commands::command::{ CallInfoExt, CommandArgs, PerItemCommand, RawCommandArgs, RunnableContext, + RunnablePerItemContext, }; pub(crate) use crate::context::CommandRegistry; pub(crate) use crate::context::Context; diff --git a/src/shell/filesystem_shell.rs b/src/shell/filesystem_shell.rs index d30bcd206..5b1101b5f 100644 --- a/src/shell/filesystem_shell.rs +++ b/src/shell/filesystem_shell.rs @@ -1,5 +1,6 @@ use crate::commands::command::EvaluatedWholeStreamCommandArgs; use crate::commands::cp::CopyArgs; +use crate::commands::ls::LsArgs; use crate::commands::mkdir::MkdirArgs; use crate::commands::mv::MoveArgs; use crate::commands::rm::RemoveArgs; @@ -10,7 +11,6 @@ use crate::shell::shell::Shell; use crate::utils::FileStructure; use nu_errors::ShellError; use nu_protocol::{Primitive, ReturnSuccess, UntaggedValue}; -use nu_source::Tagged; use rustyline::completion::FilenameCompleter; use rustyline::hint::{Hinter, HistoryHinter}; use std::path::{Path, PathBuf}; @@ -84,14 +84,13 @@ impl Shell for FilesystemShell { fn ls( &self, - pattern: Option>, - context: &RunnableContext, - full: bool, + LsArgs { path, full }: LsArgs, + context: &RunnablePerItemContext, ) -> Result { let cwd = self.path(); let mut full_path = PathBuf::from(self.path()); - if let Some(value) = &pattern { + if let Some(value) = &path { full_path.push((*value).as_ref()) } @@ -106,7 +105,7 @@ impl Shell for FilesystemShell { let entries = std::fs::read_dir(&entry); let entries = match entries { Err(e) => { - if let Some(s) = pattern { + if let Some(s) = path { return Err(ShellError::labeled_error( e.to_string(), e.to_string(), @@ -160,7 +159,7 @@ impl Shell for FilesystemShell { let entries = match glob::glob(&full_path.to_string_lossy()) { Ok(files) => files, Err(_) => { - if let Some(source) = pattern { + if let Some(source) = path { return Err(ShellError::labeled_error( "Invalid pattern", "Invalid pattern", diff --git a/src/shell/help_shell.rs b/src/shell/help_shell.rs index ab8de2fba..d25cbdd11 100644 --- a/src/shell/help_shell.rs +++ b/src/shell/help_shell.rs @@ -1,5 +1,6 @@ use crate::commands::command::EvaluatedWholeStreamCommandArgs; use crate::commands::cp::CopyArgs; +use crate::commands::ls::LsArgs; use crate::commands::mkdir::MkdirArgs; use crate::commands::mv::MoveArgs; use crate::commands::rm::RemoveArgs; @@ -10,7 +11,6 @@ use nu_errors::ShellError; use nu_protocol::{ Primitive, ReturnSuccess, ShellTypeName, TaggedDictBuilder, UntaggedValue, Value, }; -use nu_source::Tagged; use std::ffi::OsStr; use std::path::PathBuf; @@ -135,9 +135,8 @@ impl Shell for HelpShell { fn ls( &self, - _pattern: Option>, - _context: &RunnableContext, - _full: bool, + _args: LsArgs, + _context: &RunnablePerItemContext, ) -> Result { Ok(self.commands().map(ReturnSuccess::value).to_output_stream()) } diff --git a/src/shell/shell.rs b/src/shell/shell.rs index e812e7b24..79c6fb78f 100644 --- a/src/shell/shell.rs +++ b/src/shell/shell.rs @@ -1,12 +1,12 @@ use crate::commands::command::EvaluatedWholeStreamCommandArgs; use crate::commands::cp::CopyArgs; +use crate::commands::ls::LsArgs; use crate::commands::mkdir::MkdirArgs; use crate::commands::mv::MoveArgs; use crate::commands::rm::RemoveArgs; use crate::prelude::*; use crate::stream::OutputStream; use nu_errors::ShellError; -use nu_source::Tagged; use std::path::PathBuf; pub trait Shell: std::fmt::Debug { @@ -15,9 +15,8 @@ pub trait Shell: std::fmt::Debug { fn ls( &self, - pattern: Option>, - context: &RunnableContext, - full: bool, + args: LsArgs, + context: &RunnablePerItemContext, ) -> Result; fn cd(&self, args: EvaluatedWholeStreamCommandArgs) -> Result; fn cp(&self, args: CopyArgs, name: Tag, path: &str) -> Result; diff --git a/src/shell/shell_manager.rs b/src/shell/shell_manager.rs index 273a0f06c..dde248117 100644 --- a/src/shell/shell_manager.rs +++ b/src/shell/shell_manager.rs @@ -1,5 +1,6 @@ use crate::commands::command::{EvaluatedWholeStreamCommandArgs, RunnablePerItemContext}; use crate::commands::cp::CopyArgs; +use crate::commands::ls::LsArgs; use crate::commands::mkdir::MkdirArgs; use crate::commands::mv::MoveArgs; use crate::commands::rm::RemoveArgs; @@ -8,7 +9,6 @@ use crate::shell::filesystem_shell::FilesystemShell; use crate::shell::shell::Shell; use crate::stream::OutputStream; use nu_errors::ShellError; -use nu_source::Tagged; use std::error::Error; use std::path::PathBuf; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -202,12 +202,11 @@ impl ShellManager { pub fn ls( &self, - path: Option>, - context: &RunnableContext, - full: bool, + args: LsArgs, + context: &RunnablePerItemContext, ) -> Result { if let Ok(shells) = self.shells.lock() { - shells[self.current_shell()].ls(path, context, full) + shells[self.current_shell()].ls(args, context) } else { Err(ShellError::untagged_runtime_error( "Internal error: could not lock shells ring buffer (ls)", diff --git a/src/shell/value_shell.rs b/src/shell/value_shell.rs index 896b16d53..db36f9304 100644 --- a/src/shell/value_shell.rs +++ b/src/shell/value_shell.rs @@ -1,5 +1,6 @@ use crate::commands::command::EvaluatedWholeStreamCommandArgs; use crate::commands::cp::CopyArgs; +use crate::commands::ls::LsArgs; use crate::commands::mkdir::MkdirArgs; use crate::commands::mv::MoveArgs; use crate::commands::rm::RemoveArgs; @@ -8,7 +9,6 @@ use crate::shell::shell::Shell; use crate::utils::ValueStructure; use nu_errors::ShellError; use nu_protocol::{ReturnSuccess, ShellTypeName, UntaggedValue, Value}; -use nu_source::Tagged; use std::ffi::OsStr; use std::path::{Path, PathBuf}; @@ -90,14 +90,13 @@ impl Shell for ValueShell { fn ls( &self, - target: Option>, - context: &RunnableContext, - _full: bool, + LsArgs { path, .. }: LsArgs, + context: &RunnablePerItemContext, ) -> Result { let mut full_path = PathBuf::from(self.path()); let name_tag = context.name.clone(); - if let Some(value) = &target { + if let Some(value) = &path { full_path.push(value.as_ref()); } @@ -105,7 +104,7 @@ impl Shell for ValueShell { value_system.walk_decorate(&self.value)?; if !value_system.exists(&full_path) { - if let Some(target) = &target { + if let Some(target) = &path { return Err(ShellError::labeled_error( "Can not list entries inside", "No such path exists", diff --git a/tests/commands/ls.rs b/tests/commands/ls.rs index 407202dff..98b0d252c 100644 --- a/tests/commands/ls.rs +++ b/tests/commands/ls.rs @@ -69,3 +69,29 @@ fn lists_regular_files_using_question_mark_wildcard() { assert_eq!(actual, "3"); }) } + +#[test] +fn lists_all_files_in_directories_from_stream() { + Playground::setup("ls_test_4", |dirs, sandbox| { + sandbox.mkdir("dir_a").mkdir("dir_b").with_files(vec![ + EmptyFile("root1.txt"), + EmptyFile("root2.txt"), + EmptyFile("dir_a/yehuda.10.txt"), + EmptyFile("dir_a/jonathan.10.txt"), + EmptyFile("dir_b/andres.10.txt"), + EmptyFile("dir_b/chicken_not_to_be_picked_up.100.txt"), + ]); + + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + echo dir_a dir_b + | ls $it + | count + | echo $it + "# + )); + + assert_eq!(actual, "4"); + }) +}