Move 'start' to use ShellError (#1743)

* Move 'start' to use ShellError

* Remove unnecessary changes

* Add missing macOS change

* Add default

* More fixed

* More fixed

Cargo.lock

@@ -2432,7 +2432,6 @@ dependencies = [
 name = "nu_plugin_start"
 version = "0.1.0"
 dependencies = [
- "ansi_term 0.12.1",
  "nu-build",
  "nu-errors",
  "nu-plugin",

crates/nu_plugin_start/Cargo.toml

@@ -14,9 +14,10 @@ nu-plugin = { path = "../nu-plugin", version = "0.13.0" }
 nu-protocol = { path = "../nu-protocol", version = "0.13.0" }
 nu-source = { path = "../nu-source", version = "0.13.0" }
 nu-errors = { path = "../nu-errors", version = "0.13.0" }
-ansi_term = "0.12.1"
 url = "2.1.1"
 open = "1.4.0"
 
 [build-dependencies]
 nu-build = { version = "0.13.0", path = "../nu-build" }
+nu-errors = { version = "0.13.0", path = "../nu-errors" }
+nu-source = { version = "0.13.0", path = "../nu-source" }

crates/nu_plugin_start/src/main.rs

@@ -2,8 +2,5 @@ use nu_plugin::serve_plugin;
 use nu_plugin_start::Start;
 
 fn main() {
-    serve_plugin(&mut Start {
-        filenames: vec![],
-        application: None,
-    });
+    serve_plugin(&mut Start::new());
 }

crates/nu_plugin_start/src/nu/mod.rs

@@ -1,6 +1,6 @@
 use nu_errors::ShellError;
 use nu_plugin::Plugin;
-use nu_protocol::{CallInfo, Signature, SyntaxShape, Value};
+use nu_protocol::{CallInfo, ReturnValue, Signature, SyntaxShape};
 
 use crate::start::Start;
 
@@ -14,12 +14,11 @@ impl Plugin for Start {
                 SyntaxShape::String,
                 "Specifies the application used for opening the files/directories/urls",
                 Some('a'),
-            ))
+            )
+            .filter())
     }
-    fn sink(&mut self, call_info: CallInfo, input: Vec<Value>) {
-        self.parse(call_info, input);
-        if let Err(e) = self.exec() {
-            println!("{}", e);
-        }
+    fn begin_filter(&mut self, call_info: CallInfo) -> Result<Vec<ReturnValue>, ShellError> {
+        self.parse(call_info)?;
+        self.exec().map(|_| vec![])
     }
 }

crates/nu_plugin_start/src/start.rs

@@ -1,78 +1,79 @@
-use ansi_term::Color;
-use nu_protocol::{CallInfo, Value};
-use std::error::Error;
-use std::fmt;
+use nu_errors::ShellError;
+use nu_protocol::CallInfo;
+use nu_source::{Tag, Tagged, TaggedItem};
 use std::path::Path;
 
 #[cfg(not(target_os = "windows"))]
 use std::process::{Command, Stdio};
 
+#[derive(Default)]
 pub struct Start {
-    pub filenames: Vec<String>,
+    pub tag: Tag,
+    pub filenames: Vec<Tagged<String>>,
     pub application: Option<String>,
 }
 
-#[derive(Debug)]
-pub struct StartError {
-    msg: String,
-}
-
-impl StartError {
-    fn new(msg: &str) -> StartError {
-        StartError {
-            msg: msg.to_owned(),
-        }
-    }
-}
-
-impl fmt::Display for StartError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(
-            f,
-            "{}: {}",
-            Color::Red.bold().paint("start error"),
-            self.msg
-        )
-    }
-}
-
-impl Error for StartError {}
-
 impl Start {
-    pub fn parse(&mut self, call_info: CallInfo, input: Vec<Value>) {
-        input.iter().for_each(|val| {
-            if val.is_some() {
-                self.parse_value(val);
-            }
-        });
-        self.parse_filenames(&call_info);
-        self.parse_application(&call_info);
-    }
-
-    fn add_filename(&mut self, filename: String) {
-        if Path::new(&filename).exists() || url::Url::parse(&filename).is_ok() {
-            self.filenames.push(filename);
-        } else {
-            print_warning(format!(
-                "The file '{}' does not exist",
-                Color::White.bold().paint(filename)
-            ));
+    pub fn new() -> Start {
+        Start {
+            tag: Tag::unknown(),
+            filenames: vec![],
+            application: None,
         }
     }
 
-    fn parse_filenames(&mut self, call_info: &CallInfo) {
+    pub fn parse(&mut self, call_info: CallInfo) -> Result<(), ShellError> {
+        self.tag = call_info.name_tag.clone();
+        self.parse_filenames(&call_info)?;
+        self.parse_application(&call_info);
+        Ok(())
+    }
+
+    fn add_filename(&mut self, filename: Tagged<String>) -> Result<(), ShellError> {
+        if Path::new(&filename.item).exists() || url::Url::parse(&filename.item).is_ok() {
+            self.filenames.push(filename);
+            Ok(())
+        } else {
+            Err(ShellError::labeled_error(
+                format!("The file '{}' does not exist", filename.item),
+                "doesn't exist",
+                filename.tag,
+            ))
+        }
+    }
+
+    fn parse_filenames(&mut self, call_info: &CallInfo) -> Result<(), ShellError> {
         let candidates = match &call_info.args.positional {
-            Some(values) => values
-                .iter()
-                .map(|val| val.as_string())
-                .collect::<Result<Vec<String>, _>>()
-                .unwrap_or_else(|_| vec![]),
-            None => vec![],
+            Some(values) => {
+                let mut result = vec![];
+
+                for value in values.iter() {
+                    result.push(value.as_string()?.tagged(value.tag.clone()));
+                }
+
+                if result.is_empty() {
+                    return Err(ShellError::labeled_error(
+                        "No filename(s) given",
+                        "no filename(s) given",
+                        self.tag.span,
+                    ));
+                }
+                result
+            }
+            None => {
+                return Err(ShellError::labeled_error(
+                    "No filename(s) given",
+                    "no filename(s) given",
+                    self.tag.span,
+                ))
+            }
         };
 
         for candidate in candidates {
-            self.add_filename(candidate);
+            self.add_filename(candidate)?;
         }
+
+        Ok(())
     }
 
     fn parse_application(&mut self, call_info: &CallInfo) {
@@ -86,45 +87,47 @@ impl Start {
         };
     }
 
-    pub fn parse_value(&mut self, input: &Value) {
-        if let Ok(filename) = input.as_string() {
-            self.add_filename(filename);
-        } else {
-            print_warning(format!("Could not convert '{:?}' to string", input));
-        }
-    }
-
     #[cfg(target_os = "macos")]
-    pub fn exec(&mut self) -> Result<(), StartError> {
+    pub fn exec(&mut self) -> Result<(), ShellError> {
         let mut args = vec![];
-        args.append(&mut self.filenames);
+        args.append(
+            &mut self
+                .filenames
+                .iter()
+                .map(|x| x.item.clone())
+                .collect::<Vec<_>>(),
+        );
 
         if let Some(app_name) = &self.application {
             args.append(&mut vec![String::from("-a"), app_name.to_string()]);
         }
-        exec_cmd("open", &args)
+        exec_cmd("open", &args, self.tag.clone())
     }
 
     #[cfg(target_os = "windows")]
-    pub fn exec(&mut self) -> Result<(), StartError> {
+    pub fn exec(&mut self) -> Result<(), ShellError> {
         if let Some(app_name) = &self.application {
             for file in &self.filenames {
-                match open::with(file, app_name) {
+                match open::with(&file.item, app_name) {
                     Ok(_) => continue,
                     Err(_) => {
-                        return Err(StartError::new(
+                        return Err(ShellError::labeled_error(
                             "Failed to open file with specified application",
+                            "can't open with specified application",
+                            file.tag.span,
                         ))
                     }
                 }
             }
         } else {
             for file in &self.filenames {
-                match open::that(file) {
+                match open::that(&file.item) {
                     Ok(_) => continue,
                     Err(_) => {
-                        return Err(StartError::new(
+                        return Err(ShellError::labeled_error(
                             "Failed to open file with default application",
+                            "can't open with default application",
+                            file.tag.span,
                         ))
                     }
                 }
@@ -134,35 +137,43 @@ impl Start {
     }
 
     #[cfg(not(any(target_os = "windows", target_os = "macos")))]
-    pub fn exec(&mut self) -> Result<(), StartError> {
+    pub fn exec(&mut self) -> Result<(), ShellError> {
         let mut args = vec![];
-        args.append(&mut self.filenames);
+        args.append(
+            &mut self
+                .filenames
+                .iter()
+                .map(|x| x.item.clone())
+                .collect::<Vec<_>>(),
+        );
 
         if let Some(app_name) = &self.application {
-            exec_cmd(&app_name, &args)
+            exec_cmd(&app_name, &args, self.tag.clone())
         } else {
             for cmd in &["xdg-open", "gnome-open", "kde-open", "wslview"] {
-                if exec_cmd(cmd, &args).is_err() {
+                if exec_cmd(cmd, &args, self.tag.clone()).is_err() {
                     continue;
                 } else {
                     return Ok(());
                 }
             }
-            Err(StartError::new(
+            Err(ShellError::labeled_error(
                 "Failed to open file(s) with xdg-open. gnome-open, kde-open, and wslview",
+                "failed to open xdg-open. gnome-open, kde-open, and wslview",
+                self.tag.span,
             ))
         }
     }
 }
 
-fn print_warning(msg: String) {
-    println!("{}: {}", Color::Yellow.bold().paint("warning"), msg);
-}
-
 #[cfg(not(target_os = "windows"))]
-fn exec_cmd(cmd: &str, args: &[String]) -> Result<(), StartError> {
+fn exec_cmd(cmd: &str, args: &[String], tag: Tag) -> Result<(), ShellError> {
     if args.is_empty() {
-        return Err(StartError::new("No file(s) or application provided"));
+        return Err(ShellError::labeled_error(
+            "No file(s) or application provided",
+            "no file(s) or application provided",
+            tag,
+        ));
     }
     let status = match Command::new(cmd)
         .stdout(Stdio::null())
@@ -171,13 +182,21 @@ fn exec_cmd(cmd: &str, args: &[String]) -> Result<(), StartError> {
         .status()
     {
         Ok(exit_code) => exit_code,
-        Err(_) => return Err(StartError::new("Failed to run native open syscall")),
+        Err(_) => {
+            return Err(ShellError::labeled_error(
+                "Failed to run native open syscall",
+                "failed to run native open call",
+                tag,
+            ))
+        }
     };
     if status.success() {
         Ok(())
     } else {
-        Err(StartError::new(
+        Err(ShellError::labeled_error(
             "Failed to run start. Hint: The file(s)/application may not exist",
+            "failed to run",
+            tag,
         ))
     }
 }

src/plugins/nu_plugin_stable_start.rs

@@ -2,8 +2,5 @@ use nu_plugin::serve_plugin;
 use nu_plugin_start::Start;
 
 fn main() {
-    serve_plugin(&mut Start {
-        filenames: vec![],
-        application: None,
-    });
+    serve_plugin(&mut Start::new());
 }