Update errors and improve ctrl-c

src/cli.rs

@@ -101,13 +101,10 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
         cc.store(true, Ordering::SeqCst);
     })
     .expect("Error setting Ctrl-C handler");
-
+    let mut ctrlcbreak = false;
     loop {
         if ctrl_c.load(Ordering::SeqCst) {
             ctrl_c.store(false, Ordering::SeqCst);
-            if let ShellError::String(s) = ShellError::string("CTRL-C") {
-                context.host.lock().unwrap().stdout(&format!("{:?}", s));
-            }
             continue;
         }
 
@@ -133,6 +130,20 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
                 rl.add_history_entry(line.clone());
             }
 
+            LineResult::CtrlC => {
+                if ctrlcbreak {
+                    std::process::exit(0);
+                } else {
+                    context
+                        .host
+                        .lock()
+                        .unwrap()
+                        .stdout("CTRL-C pressed (again to quit)");
+                    ctrlcbreak = true;
+                    continue;
+                }
+            }
+
             LineResult::Error(mut line, err) => match err {
                 ShellError::Diagnostic(diag) => {
                     let host = context.host.lock().unwrap();
@@ -176,6 +187,7 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
                     .stdout(&format!("A surprising fatal error occurred.\n{:?}", err));
             }
         }
+        ctrlcbreak = false;
     }
     rl.save_history("history.txt").unwrap();
 
@@ -185,6 +197,7 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
 enum LineResult {
     Success(String),
     Error(String, ShellError),
+    CtrlC,
     Break,
 
     #[allow(unused)]
@@ -200,6 +213,7 @@ impl std::ops::Try for LineResult {
             LineResult::Success(s) => Ok(Some(s)),
             LineResult::Error(_, s) => Err(s),
             LineResult::Break => Ok(None),
+            LineResult::CtrlC => Ok(None),
             LineResult::FatalError(err) => Err(err),
         }
     }
@@ -258,27 +272,26 @@ async fn process_line(readline: Result<String, ReadlineError>, ctx: &mut Context
                     (None, _) => break,
 
                     (Some(ClassifiedCommand::Expr(_)), _) => {
-                        return LineResult::Error(line.clone(), ShellError::unimplemented(
+                        return LineResult::Error(
-                            "Expression-only commands",
+                            line.clone(),
-                        ))
+                            ShellError::unimplemented("Expression-only commands"),
+                        )
                     }
 
                     (_, Some(ClassifiedCommand::Expr(_))) => {
-                        return LineResult::Error(line.clone(), ShellError::unimplemented(
+                        return LineResult::Error(
-                            "Expression-only commands",
+                            line.clone(),
-                        ))
+                            ShellError::unimplemented("Expression-only commands"),
+                        )
                     }
 
-                    (Some(ClassifiedCommand::Sink(_)), Some(_)) => {
+                    (Some(ClassifiedCommand::Sink(SinkCommand { name_span, .. })), Some(_)) => {
-                        return LineResult::Error(line.clone(), ShellError::string("Commands like table, save, and autoview must come last in the pipeline"))
+                        return LineResult::Error(line.clone(), ShellError::maybe_labeled_error("Commands like table, save, and autoview must come last in the pipeline", "must come last", name_span));
                     }
 
                     (Some(ClassifiedCommand::Sink(left)), None) => {
                         let input_vec: Vec<Value> = input.objects.collect().await;
-                        left.run(
+                        left.run(ctx, input_vec)?;
-                            ctx,
-                            input_vec,
-                        )?;
                         break;
                     }
 
@@ -290,13 +303,12 @@ async fn process_line(readline: Result<String, ReadlineError>, ctx: &mut Context
                         Err(err) => return LineResult::Error(line.clone(), err),
                     },
 
-                    (
+                    (Some(ClassifiedCommand::Internal(left)), Some(_)) => {
-                        Some(ClassifiedCommand::Internal(left)),
+                        match left.run(ctx, input).await {
-                        Some(_),
+                            Ok(val) => ClassifiedInputStream::from_input_stream(val),
-                    ) => match left.run(ctx, input).await {
+                            Err(err) => return LineResult::Error(line.clone(), err),
-                        Ok(val) => ClassifiedInputStream::from_input_stream(val),
+                        }
-                        Err(err) => return LineResult::Error(line.clone(), err),
+                    }
-                    },
 
                     (Some(ClassifiedCommand::Internal(left)), None) => {
                         match left.run(ctx, input).await {
@@ -313,13 +325,12 @@ async fn process_line(readline: Result<String, ReadlineError>, ctx: &mut Context
                         Err(err) => return LineResult::Error(line.clone(), err),
                     },
 
-                    (
+                    (Some(ClassifiedCommand::External(left)), Some(_)) => {
-                        Some(ClassifiedCommand::External(left)),
+                        match left.run(ctx, input, StreamNext::Internal).await {
-                        Some(_),
+                            Ok(val) => val,
-                    ) => match left.run(ctx, input, StreamNext::Internal).await {
+                            Err(err) => return LineResult::Error(line.clone(), err),
-                        Ok(val) => val,
+                        }
-                        Err(err) => return LineResult::Error(line.clone(), err),
+                    }
-                    },
 
                     (Some(ClassifiedCommand::External(left)), None) => {
                         match left.run(ctx, input, StreamNext::Last).await {
@@ -332,9 +343,7 @@ async fn process_line(readline: Result<String, ReadlineError>, ctx: &mut Context
 
             LineResult::Success(line.clone())
         }
-        Err(ReadlineError::Interrupted) => {
+        Err(ReadlineError::Interrupted) => LineResult::CtrlC,
-            LineResult::Error("".to_string(), ShellError::string("CTRL-C"))
-        }
         Err(ReadlineError::Eof) => {
             println!("CTRL-D");
             LineResult::Break

src/commands/cd.rs

@@ -13,17 +13,23 @@ pub fn cd(args: CommandArgs) -> Result<OutputStream, ShellError> {
             let path = match args.positional.first() {
                 None => match dirs::home_dir() {
                     Some(o) => o,
-                    _ => return Err(ShellError::string("Can not change to home directory")),
+                    _ => {
+                        return Err(ShellError::maybe_labeled_error(
+                            "Can not change to home directory",
+                            "can not go to home",
+                            args.name_span,
+                        ))
+                    }
                 },
                 Some(v) => {
                     let target = v.as_string()?.clone();
                     match dunce::canonicalize(cwd.join(&target).as_path()) {
                         Ok(p) => p,
                         Err(_) => {
-                            return Err(ShellError::labeled_error(
+                            return Err(ShellError::maybe_labeled_error(
                                 "Can not change to directory",
                                 "directory not found",
-                                args.positional[0].span.clone(),
+                                Some(args.positional[0].span.clone()),
                             ));
                         }
                     }
@@ -35,10 +41,10 @@ pub fn cd(args: CommandArgs) -> Result<OutputStream, ShellError> {
                 Ok(_) => {}
                 Err(_) => {
                     if args.positional.len() > 0 {
-                        return Err(ShellError::labeled_error(
+                        return Err(ShellError::maybe_labeled_error(
                             "Can not change to directory",
                             "directory not found",
-                            args.positional[0].span.clone(),
+                            Some(args.positional[0].span.clone()),
                         ));
                     } else {
                         return Err(ShellError::string("Can not change to directory"));

src/commands/classified.rs

@@ -151,38 +151,6 @@ impl InternalCommand {
                 }
             }
         }
-        /*
-        let stream = result.filter_map(move |v| match v {
-            //ReturnValue::Value(Value::Error(err)) => futures::future::err(Err(err)),
-            ReturnValue::Action(action) => match action {
-                CommandAction::ChangePath(path) => {
-                    env.lock().unwrap().last_mut().map(|x| {
-                        x.path = path;
-                        x
-                    });
-                    futures::future::ready(None)
-                }
-                CommandAction::Enter(obj) => {
-                    let new_env = Environment {
-                        obj: obj,
-                        path: PathBuf::from("/"),
-                    };
-                    env.lock().unwrap().push(new_env);
-                    futures::future::ready(None)
-                }
-                CommandAction::Exit => {
-                    let mut v = env.lock().unwrap();
-                    if v.len() == 1 {
-                        std::process::exit(0);
-                    }
-                    v.pop();
-                    futures::future::ready(None)
-                }
-            },
-
-            ReturnValue::Value(v) => futures::future::ready(Some(v)),
-        });
-        */
         Ok(stream.boxed() as InputStream)
     }
 }

src/commands/enter.rs

@@ -7,7 +7,11 @@ use std::path::{Path, PathBuf};
 
 pub fn enter(args: CommandArgs) -> Result<OutputStream, ShellError> {
     if args.positional.len() == 0 {
-        return Err(ShellError::string("open requires a filepath or url"));
+        return Err(ShellError::maybe_labeled_error(
+            "open requires a path or url",
+            "missing path",
+            args.name_span,
+        ));
     }
 
     let cwd = args

src/commands/first.rs

@@ -5,15 +5,11 @@ use crate::prelude::*;
 
 pub fn first(args: CommandArgs) -> Result<OutputStream, ShellError> {
     if args.positional.len() == 0 {
-        if let Some(span) = args.name_span {
+        return Err(ShellError::maybe_labeled_error(
-            return Err(ShellError::labeled_error(
+            "First requires an amount",
-                "First requires an amount",
+            "needs parameter",
-                "needs parameter",
+            args.name_span,
-                span,
+        ));
-            ));
-        } else {
-            return Err(ShellError::string("first requires an amount."));
-        }
     }
 
     let amount = args.positional[0].as_i64();

src/commands/from_xml.rs

@@ -15,7 +15,7 @@ fn from_node_to_value<'a, 'd>(n: &roxmltree::Node<'a, 'd>) -> Value {
             .filter(|x| match x {
                 Value::Primitive(Primitive::String(f)) => {
                     if f.trim() == "" {
-                        false 
+                        false
                     } else {
                         true
                     }
@@ -57,11 +57,16 @@ pub fn from_xml_string_to_value(s: String) -> Value {
 
 pub fn from_xml(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let out = args.input;
+    let span = args.name_span;
     Ok(out
-        .map(|a| match a {
+        .map(move |a| match a {
-            Value::Primitive(Primitive::String(s)) => ReturnValue::Value(from_xml_string_to_value(s)),
+            Value::Primitive(Primitive::String(s)) => {
-            _ => ReturnValue::Value(Value::Error(Box::new(ShellError::string(
+                ReturnValue::Value(from_xml_string_to_value(s))
+            }
+            _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
                 "Trying to convert XML from non-string".to_string(),
+                "given non-string",
+                span,
             )))),
         })
         .boxed())

src/commands/get.rs

@@ -11,7 +11,7 @@ fn get_member(path: &str, span: Span, obj: &Value) -> Option<Value> {
             None => {
                 return Some(Value::Error(Box::new(ShellError::labeled_error(
                     "Unknown field",
-                    "unknown field",
+                    "object missing field",
                     span,
                 ))));
             }
@@ -23,15 +23,11 @@ fn get_member(path: &str, span: Span, obj: &Value) -> Option<Value> {
 
 pub fn get(args: CommandArgs) -> Result<OutputStream, ShellError> {
     if args.positional.len() == 0 {
-        if let Some(span) = args.name_span {
+        return Err(ShellError::maybe_labeled_error(
-            return Err(ShellError::labeled_error(
+            "Get requires a field or field path",
-                "Get requires a field or field path",
+            "needs parameter",
-                "needs parameter",
+            args.name_span,
-                span,
+        ));
-            ));
-        } else {
-            return Err(ShellError::string("get requires fields."));
-        }
     }
 
     let amount = args.positional[0].as_i64();

src/commands/ls.rs

@@ -31,7 +31,11 @@ pub fn ls(args: CommandArgs) -> Result<OutputStream, ShellError> {
                             s.span,
                         ));
                     } else {
-                        return Err(ShellError::string(e.to_string()));
+                        return Err(ShellError::maybe_labeled_error(
+                            e.to_string(),
+                            e.to_string(),
+                            args.name_span,
+                        ));
                     }
                 }
                 Ok(o) => o,

src/commands/open.rs

@@ -6,7 +6,11 @@ use std::path::{Path, PathBuf};
 
 pub fn open(args: CommandArgs) -> Result<OutputStream, ShellError> {
     if args.positional.len() == 0 {
-        return Err(ShellError::string("open requires a filepath or url"));
+        return Err(ShellError::maybe_labeled_error(
+            "Open requires a path or url",
+            "needs path or url",
+            args.name_span,
+        ));
     }
 
     let cwd = args

src/commands/pick.rs

@@ -5,15 +5,11 @@ use crate::prelude::*;
 
 pub fn pick(args: CommandArgs) -> Result<OutputStream, ShellError> {
     if args.positional.len() == 0 {
-        if let Some(span) = args.name_span {
+        return Err(ShellError::maybe_labeled_error(
-            return Err(ShellError::labeled_error(
+            "Pick requires fields",
-                "Pick requires fields",
+            "needs parameter",
-                "needs parameter",
+            args.name_span,
-                span,
+        ));
-            ));
-        } else {
-            return Err(ShellError::string("pick requires fields."));
-        }
     }
 
     let fields: Result<Vec<String>, _> = args.positional.iter().map(|a| a.as_string()).collect();

src/commands/reject.rs

@@ -5,15 +5,11 @@ use crate::prelude::*;
 
 pub fn reject(args: CommandArgs) -> Result<OutputStream, ShellError> {
     if args.positional.len() == 0 {
-        if let Some(span) = args.name_span {
+        return Err(ShellError::maybe_labeled_error(
-            return Err(ShellError::labeled_error(
+            "Reject requires fields",
-                "Reject requires fields",
+            "needs parameter",
-                "needs parameter",
+            args.name_span,
-                span,
+        ));
-            ));
-        } else {
-            return Err(ShellError::string("reject requires fields."));
-        }
     }
 
     let fields: Result<Vec<String>, _> = args.positional.iter().map(|a| a.as_string()).collect();

src/commands/save.rs

@@ -6,7 +6,11 @@ use std::path::{Path, PathBuf};
 
 pub fn save(args: SinkCommandArgs) -> Result<(), ShellError> {
     if args.positional.len() == 0 {
-        return Err(ShellError::string("save requires a filepath"));
+        return Err(ShellError::maybe_labeled_error(
+            "Save requires a filepath",
+            "needs path",
+            args.name_span,
+        ));
     }
 
     let cwd = args

src/commands/size.rs

@@ -6,8 +6,12 @@ use std::fs::File;
 use std::io::prelude::*;
 
 pub fn size(args: CommandArgs) -> Result<OutputStream, ShellError> {
-    if args.positional.is_empty() {
+    if args.positional.len() == 0 {
-        return Err(ShellError::string("size requires at least one file"));
+        return Err(ShellError::maybe_labeled_error(
+            "Size requires a filepath",
+            "needs path",
+            args.name_span,
+        ));
     }
     let cwd = args
         .env

src/commands/skip.rs

@@ -3,15 +3,11 @@ use crate::prelude::*;
 
 pub fn skip(args: CommandArgs) -> Result<OutputStream, ShellError> {
     if args.positional.len() == 0 {
-        if let Some(span) = args.name_span {
+        return Err(ShellError::maybe_labeled_error(
-            return Err(ShellError::labeled_error(
+            "Skip requires an amount",
-                "Skip requires an amount",
+            "needs parameter",
-                "needs parameter",
+            args.name_span,
-                span,
+        ));
-            ));
-        } else {
-            return Err(ShellError::string("skip requires an amount."));
-        }
     }
 
     let amount = args.positional[0].as_i64();

src/commands/view.rs

@@ -4,15 +4,11 @@ use prettyprint::PrettyPrinter;
 
 pub fn view(args: CommandArgs) -> Result<OutputStream, ShellError> {
     if args.positional.len() == 0 {
-        if let Some(span) = args.name_span {
+        return Err(ShellError::maybe_labeled_error(
-            return Err(ShellError::labeled_error(
+            "View requires a filename",
-                "View requires a filename",
+            "needs parameter",
-                "needs parameter",
+            args.name_span,
-                span,
+        ));
-            ));
-        } else {
-            return Err(ShellError::string("view requires a filename."));
-        }
     }
 
     let target = match args.positional[0].as_string() {

src/commands/where_.rs

@@ -25,8 +25,12 @@ impl Command for Where {
 }
 
 pub fn r#where(args: CommandArgs) -> Result<OutputStream, ShellError> {
-    if args.positional.is_empty() {
+    if args.positional.len() == 0 {
-        return Err(ShellError::string("select requires a field"));
+        return Err(ShellError::maybe_labeled_error(
+            "Where requires a condition",
+            "needs condition",
+            args.name_span,
+        ));
     }
 
     let block = args.positional[0].as_block()?;

src/errors.rs

@@ -53,6 +53,20 @@ impl ShellError {
         )
     }
 
+    crate fn maybe_labeled_error(
+        msg: impl Into<String>,
+        label: impl Into<String>,
+        span: Option<Span>,
+    ) -> ShellError {
+        match span {
+            Some(span) => ShellError::diagnostic(
+                Diagnostic::new(Severity::Error, msg.into())
+                    .with_label(Label::new_primary(span).with_message(label.into())),
+            ),
+            None => ShellError::string(msg),
+        }
+    }
+
     crate fn string(title: impl Into<String>) -> ShellError {
         ShellError::String(StringError::new(title.into(), Value::nothing()))
     }