Reduce unwraps

Remove a number of unwraps. In some cases, a `?` just worked as is. I also made it possible to use `?` to go from Result<OutputStream, ShellError> to OutputStream. Finally, started updating PerItemCommand to be able to use the signature deserialization logic, which substantially reduces unwraps.

This is still in-progress work, but tests pass and it should be clear to merge and keep iterating on master.

src/commands/autoview.rs

@@ -17,7 +17,7 @@ impl WholeStreamCommand for Autoview {
         args: CommandArgs,
         registry: &CommandRegistry,
     ) -> Result<OutputStream, ShellError> {
-        args.process_raw(registry, autoview)?.run()
+        Ok(args.process_raw(registry, autoview)?.run())
     }
 
     fn signature(&self) -> Signature {
@@ -40,28 +40,20 @@ pub fn autoview(
             } = input[0usize]
             {
                 let binary = context.expect_command("binaryview");
-                let result = binary.run(raw.with_input(input), &context.commands).await.unwrap();
+                let result = binary.run(raw.with_input(input), &context.commands);
                 result.collect::<Vec<_>>().await;
             } else if is_single_text_value(&input) {
                 let text = context.expect_command("textview");
-                let result = text.run(raw.with_input(input), &context.commands).await.unwrap();
+                let result = text.run(raw.with_input(input), &context.commands);
                 result.collect::<Vec<_>>().await;
             } else if equal_shapes(&input) {
                 let table = context.expect_command("table");
-                let result = table.run(raw.with_input(input), &context.commands).await.unwrap();
+                let result = table.run(raw.with_input(input), &context.commands);
                 result.collect::<Vec<_>>().await;
             } else {
                 let table = context.expect_command("table");
-                let result = table.run(raw.with_input(input), &context.commands).await.unwrap();
+                let result = table.run(raw.with_input(input), &context.commands);
                 result.collect::<Vec<_>>().await;
-                //println!("TODO!")
-                // TODO
-                // let mut host = context.host.lock().unwrap();
-                // for i in input.iter() {
-                //     let view = GenericView::new(&i);
-                //     handle_unexpected(&mut *host, |host| crate::format::print_view(&view, host));
-                //     host.stdout("");
-                // }
             }
         }
     }))

src/commands/classified.rs

@@ -117,16 +117,14 @@ impl InternalCommand {
         let objects: InputStream =
             trace_stream!(target: "nu::trace_stream::internal", "input" = input.objects);
 
-        let result = context
-            .run_command(
-                self.command,
-                self.name_span.clone(),
-                context.source_map.clone(),
-                self.args,
-                source,
-                objects,
-            )
-            .await?;
+        let result = context.run_command(
+            self.command,
+            self.name_span.clone(),
+            context.source_map.clone(),
+            self.args,
+            source,
+            objects,
+        );
 
         let mut result = result.values;
 
@@ -285,7 +283,7 @@ impl ExternalCommand {
                             continue;
                         }
 
-                        process = process.arg(&arg.replace("$it", &i.as_string().unwrap()));
+                        process = process.arg(&arg.replace("$it", &i.as_string()?));
                     }
                 }
             } else {

src/commands/command.rs

@@ -77,6 +77,25 @@ pub struct CallInfo {
     pub name_span: Span,
 }
 
+impl CallInfo {
+    pub fn process<'de, T: Deserialize<'de>>(
+        &self,
+        shell_manager: &ShellManager,
+        callback: fn(T, &RunnablePerItemContext) -> Result<VecDeque<ReturnValue>, ShellError>,
+    ) -> Result<RunnablePerItemArgs<T>, ShellError> {
+        let mut deserializer = ConfigDeserializer::from_call_info(self.clone());
+
+        Ok(RunnablePerItemArgs {
+            args: T::deserialize(&mut deserializer)?,
+            context: RunnablePerItemContext {
+                shell_manager: shell_manager.clone(),
+                name: self.name_span,
+            },
+            callback,
+        })
+    }
+}
+
 #[derive(Getters)]
 #[get = "crate"]
 pub struct CommandArgs {
@@ -147,7 +166,7 @@ impl CommandArgs {
         let args = self.evaluate_once(registry)?;
         let (input, args) = args.split();
         let name_span = args.call_info.name_span;
-        let mut deserializer = ConfigDeserializer::from_call_node(args);
+        let mut deserializer = ConfigDeserializer::from_call_info(args.call_info);
 
         Ok(RunnableArgs {
             args: T::deserialize(&mut deserializer)?,
@@ -180,7 +199,7 @@ impl CommandArgs {
         let args = self.evaluate_once(registry)?;
         let (input, args) = args.split();
         let name_span = args.call_info.name_span;
-        let mut deserializer = ConfigDeserializer::from_call_node(args);
+        let mut deserializer = ConfigDeserializer::from_call_info(args.call_info);
 
         Ok(RunnableRawArgs {
             args: T::deserialize(&mut deserializer)?,
@@ -198,6 +217,17 @@ impl CommandArgs {
     }
 }
 
+pub struct RunnablePerItemContext {
+    pub shell_manager: ShellManager,
+    pub name: Span,
+}
+
+impl RunnablePerItemContext {
+    pub fn cwd(&self) -> PathBuf {
+        PathBuf::from(self.shell_manager.path())
+    }
+}
+
 pub struct RunnableContext {
     pub input: InputStream,
     pub shell_manager: ShellManager,
@@ -219,6 +249,18 @@ impl RunnableContext {
     }
 }
 
+pub struct RunnablePerItemArgs<T> {
+    args: T,
+    context: RunnablePerItemContext,
+    callback: fn(T, &RunnablePerItemContext) -> Result<VecDeque<ReturnValue>, ShellError>,
+}
+
+impl<T> RunnablePerItemArgs<T> {
+    pub fn run(self) -> Result<VecDeque<ReturnValue>, ShellError> {
+        (self.callback)(self.args, &self.context)
+    }
+}
+
 pub struct RunnableArgs<T> {
     args: T,
     context: RunnableContext,
@@ -239,8 +281,11 @@ pub struct RunnableRawArgs<T> {
 }
 
 impl<T> RunnableRawArgs<T> {
-    pub fn run(self) -> Result<OutputStream, ShellError> {
-        (self.callback)(self.args, self.context, self.raw_args)
+    pub fn run(self) -> OutputStream {
+        match (self.callback)(self.args, self.context, self.raw_args) {
+            Ok(stream) => stream,
+            Err(err) => OutputStream::one(Err(err)),
+        }
     }
 }
 
@@ -475,13 +520,12 @@ impl Command {
         }
     }
 
-    pub async fn run(
-        &self,
-        args: CommandArgs,
-        registry: &registry::CommandRegistry,
-    ) -> Result<OutputStream, ShellError> {
+    pub fn run(&self, args: CommandArgs, registry: &registry::CommandRegistry) -> OutputStream {
         match self {
-            Command::WholeStream(command) => command.run(args, registry),
+            Command::WholeStream(command) => match command.run(args, registry) {
+                Ok(stream) => stream,
+                Err(err) => OutputStream::one(Err(err)),
+            },
             Command::PerItem(command) => self.run_helper(command.clone(), args, registry.clone()),
         }
     }
@@ -491,7 +535,7 @@ impl Command {
         command: Arc<dyn PerItemCommand>,
         args: CommandArgs,
         registry: CommandRegistry,
-    ) -> Result<OutputStream, ShellError> {
+    ) -> OutputStream {
         let raw_args = RawCommandArgs {
             host: args.host,
             shell_manager: args.shell_manager,
@@ -515,7 +559,7 @@ impl Command {
                 })
                 .flatten();
 
-            Ok(out.to_output_stream())
+            out.to_output_stream()
         } else {
             let nothing = Value::nothing().tagged(Tag::unknown());
             let call_info = raw_args
@@ -524,11 +568,15 @@ impl Command {
                 .evaluate(&registry, &Scope::it_value(nothing.clone()))
                 .unwrap();
             // We don't have an $it or block, so just execute what we have
-            let out = match command.run(&call_info, &registry, &raw_args.shell_manager, nothing) {
-                Ok(o) => o,
-                Err(e) => VecDeque::from(vec![ReturnValue::Err(e)]),
-            };
-            Ok(out.to_output_stream())
+
+            command
+                .run(&call_info, &registry, &raw_args.shell_manager, nothing)?
+                .into()
+            // let out = match command.run(&call_info, &registry, &raw_args.shell_manager, nothing) {
+            //     Ok(o) => o,
+            //     Err(e) => VecDeque::from(vec![ReturnValue::Err(e)]),
+            // };
+            // Ok(out.to_output_stream())
         }
     }
 }

src/commands/cp.rs

@@ -1,3 +1,4 @@
+use crate::commands::command::RunnablePerItemContext;
 use crate::errors::ShellError;
 use crate::parser::hir::SyntaxType;
 use crate::parser::registry::{CommandRegistry, Signature};
@@ -7,15 +8,22 @@ use std::path::PathBuf;
 
 pub struct Cpy;
 
+#[derive(Deserialize)]
+pub struct CopyArgs {
+    source: Tagged<PathBuf>,
+    destination: Tagged<PathBuf>,
+    recursive: Tagged<bool>,
+}
+
 impl PerItemCommand for Cpy {
     fn run(
         &self,
         call_info: &CallInfo,
         _registry: &CommandRegistry,
         shell_manager: &ShellManager,
-        _input: Tagged<Value>,
+        input: Tagged<Value>,
     ) -> Result<VecDeque<ReturnValue>, ShellError> {
-        cp(call_info, shell_manager)
+        call_info.process(shell_manager, cp)?.run()
     }
 
     fn name(&self) -> &str {
@@ -24,42 +32,24 @@ impl PerItemCommand for Cpy {
 
     fn signature(&self) -> Signature {
         Signature::build("cp")
+            .required("source", SyntaxType::Path)
+            .required("destination", SyntaxType::Path)
             .named("file", SyntaxType::Any)
             .switch("recursive")
     }
 }
 
 pub fn cp(
-    call_info: &CallInfo,
-    shell_manager: &ShellManager,
+    args: CopyArgs,
+    context: &RunnablePerItemContext,
 ) -> Result<VecDeque<ReturnValue>, ShellError> {
-    let mut source = PathBuf::from(shell_manager.path());
-    let mut destination = PathBuf::from(shell_manager.path());
-    let name_span = call_info.name_span;
+    let mut source = PathBuf::from(context.shell_manager.path());
+    let mut destination = PathBuf::from(context.shell_manager.path());
+    let name_span = context.name;
 
-    match call_info
-        .args
-        .nth(0)
-        .ok_or_else(|| ShellError::string(&format!("No file or directory specified")))?
-        .as_string()?
-        .as_str()
-    {
-        file => {
-            source.push(file);
-        }
-    }
+    source.push(&args.source.item);
 
-    match call_info
-        .args
-        .nth(1)
-        .ok_or_else(|| ShellError::string(&format!("No file or directory specified")))?
-        .as_string()?
-        .as_str()
-    {
-        file => {
-            destination.push(file);
-        }
-    }
+    destination.push(&args.destination.item);
 
     let sources = glob::glob(&source.to_string_lossy());
 
@@ -67,7 +57,7 @@ pub fn cp(
         return Err(ShellError::labeled_error(
             "Invalid pattern.",
             "Invalid pattern.",
-            call_info.args.nth(0).unwrap().span(),
+            args.source.tag,
         ));
     }
 
@@ -75,11 +65,11 @@ pub fn cp(
 
     if sources.len() == 1 {
         if let Ok(entry) = &sources[0] {
-            if entry.is_dir() && !call_info.args.has("recursive") {
+            if entry.is_dir() && !args.recursive.item {
                 return Err(ShellError::labeled_error(
                     "is a directory (not copied). Try using \"--recursive\".",
                     "is a directory (not copied). Try using \"--recursive\".",
-                    call_info.args.nth(0).unwrap().span(),
+                    args.source.tag,
                 ));
             }
 
@@ -248,7 +238,7 @@ pub fn cp(
                 return Err(ShellError::labeled_error(
                     "Copy aborted (directories found). Recursive copying in patterns not supported yet (try copying the directory directly)",
                     "Copy aborted (directories found). Recursive copying in patterns not supported yet (try copying the directory directly)",
-                    call_info.args.nth(0).unwrap().span(),
+                    args.source.tag,
                 ));
             }
 
@@ -263,7 +253,7 @@ pub fn cp(
                                 return Err(ShellError::labeled_error(
                                     e.to_string(),
                                     e.to_string(),
-                                    call_info.args.nth(0).unwrap().span(),
+                                    args.source.tag,
                                 ));
                             }
                             Ok(o) => o,
@@ -281,7 +271,7 @@ pub fn cp(
                     "Copy aborted. (Does {:?} exist?)",
                     &destination.file_name().unwrap()
                 ),
-                call_info.args.nth(1).unwrap().span(),
+                args.destination.span(),
             ));
         }
     }