From 910869b79d32f8150d7e7f3323802cf20e3dd72a Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Sun, 16 Jun 2019 05:52:55 +1200 Subject: [PATCH 1/2] Get stream errors working --- src/cli.rs | 2 +- src/commands/cd.rs | 2 +- src/commands/classified.rs | 44 ++++++++++++++++++++++++++++++++++---- src/commands/command.rs | 2 +- src/commands/enter.rs | 2 +- src/commands/exit.rs | 5 +---- src/commands/get.rs | 20 +++++++++++------ src/commands/ls.rs | 4 ++-- src/commands/open.rs | 2 +- src/commands/save.rs | 2 +- src/commands/size.rs | 2 +- src/commands/view.rs | 2 +- src/context.rs | 6 ++++-- 13 files changed, 68 insertions(+), 27 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 24e654da3..4c631a619 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -113,7 +113,7 @@ pub async fn cli() -> Result<(), Box> { let (obj, cwd) = { let env = context.env.lock().unwrap(); - let last = env.last().unwrap(); + let last = env.back().unwrap(); (last.obj().clone(), last.path().display().to_string()) }; let readline = match obj { diff --git a/src/commands/cd.rs b/src/commands/cd.rs index d69f71e2b..8716a4362 100644 --- a/src/commands/cd.rs +++ b/src/commands/cd.rs @@ -5,7 +5,7 @@ use std::path::PathBuf; pub fn cd(args: CommandArgs) -> Result { let env = args.env.lock().unwrap(); - let latest = env.last().unwrap(); + let latest = env.back().unwrap(); match latest.obj { Value::Filesystem => { diff --git a/src/commands/classified.rs b/src/commands/classified.rs index fcd9586f2..a83a384f9 100644 --- a/src/commands/classified.rs +++ b/src/commands/classified.rs @@ -4,6 +4,7 @@ use crate::parser::lexer::Span; use crate::parser::registry::Args; use crate::prelude::*; use bytes::{BufMut, BytesMut}; +use futures::stream::StreamExt; use futures_codec::{Decoder, Encoder, Framed}; use std::io::{Error, ErrorKind}; use std::path::PathBuf; @@ -109,15 +110,50 @@ impl InternalCommand { context: &mut Context, input: ClassifiedInputStream, ) -> Result { - let result = context.run_command( + let mut result = context.run_command( self.command, self.name_span.clone(), self.args, input.objects, )?; - let env = context.env.clone(); + let mut stream = VecDeque::new(); + while let Some(item) = result.next().await { + match item { + ReturnValue::Value(Value::Error(err)) => { + return Err(*err); + } + ReturnValue::Action(action) => match action { + CommandAction::ChangePath(path) => { + context.env.lock().unwrap().back_mut().map(|x| { + x.path = path; + x + }); + } + CommandAction::Enter(obj) => { + let new_env = Environment { + obj: obj, + path: PathBuf::from("/"), + }; + context.env.lock().unwrap().push_back(new_env); + } + CommandAction::Exit => match context.env.lock().unwrap().pop_back() { + Some(Environment { + obj: Value::Filesystem, + .. + }) => std::process::exit(0), + None => std::process::exit(-1), + _ => {} + }, + }, + ReturnValue::Value(v) => { + stream.push_back(v); + } + } + } + /* 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| { @@ -146,7 +182,7 @@ impl InternalCommand { ReturnValue::Value(v) => futures::future::ready(Some(v)), }); - + */ Ok(stream.boxed() as InputStream) } } @@ -229,7 +265,7 @@ impl ExternalCommand { } process = Exec::shell(new_arg_string); } - process = process.cwd(context.env.lock().unwrap().first().unwrap().path()); + process = process.cwd(context.env.lock().unwrap().front().unwrap().path()); let mut process = match stream_next { StreamNext::Last => process, diff --git a/src/commands/command.rs b/src/commands/command.rs index 2d9faff09..ff2046206 100644 --- a/src/commands/command.rs +++ b/src/commands/command.rs @@ -8,7 +8,7 @@ use std::path::PathBuf; pub struct CommandArgs { pub host: Arc>, - pub env: Arc>>, + pub env: Arc>>, pub name_span: Option, pub positional: Vec>, pub named: indexmap::IndexMap, diff --git a/src/commands/enter.rs b/src/commands/enter.rs index d1bbef5da..972225823 100644 --- a/src/commands/enter.rs +++ b/src/commands/enter.rs @@ -14,7 +14,7 @@ pub fn enter(args: CommandArgs) -> Result { .env .lock() .unwrap() - .first() + .front() .unwrap() .path() .to_path_buf(); diff --git a/src/commands/exit.rs b/src/commands/exit.rs index 0669adee7..27438f665 100644 --- a/src/commands/exit.rs +++ b/src/commands/exit.rs @@ -1,11 +1,8 @@ use crate::commands::command::CommandAction; use crate::errors::ShellError; -use crate::object::{Primitive, Value}; -use crate::parser::lexer::Spanned; use crate::prelude::*; -use std::path::{Path, PathBuf}; -pub fn exit(args: CommandArgs) -> Result { +pub fn exit(_args: CommandArgs) -> Result { let mut stream = VecDeque::new(); stream.push_back(ReturnValue::Action(CommandAction::Exit)); Ok(stream.boxed()) diff --git a/src/commands/get.rs b/src/commands/get.rs index b3869d97a..3939e9fde 100644 --- a/src/commands/get.rs +++ b/src/commands/get.rs @@ -1,17 +1,19 @@ use crate::errors::ShellError; use crate::object::Value; +use crate::parser::lexer::Span; use crate::prelude::*; -fn get_member(path: &str, obj: &Value) -> Option { +fn get_member(path: &str, span: Span, obj: &Value) -> Option { let mut current = obj; for p in path.split(".") { match current.get_data_by_key(p) { Some(v) => current = v, None => { - return Some(Value::Error(Box::new(ShellError::string(format!( - "Object field name not found: {}", - p - ))))) + return Some(Value::Error(Box::new(ShellError::labeled_error( + "Unknown field", + "unknown field", + span, + )))); } } } @@ -44,7 +46,11 @@ pub fn get(args: CommandArgs) -> Result { .boxed()); } - let fields: Result, _> = args.positional.iter().map(|a| a.as_string()).collect(); + let fields: Result, _> = args + .positional + .iter() + .map(|a| (a.as_string().map(|x| (x, a.span)))) + .collect(); let fields = fields?; let stream = args @@ -52,7 +58,7 @@ pub fn get(args: CommandArgs) -> Result { .map(move |item| { let mut result = VecDeque::new(); for field in &fields { - match get_member(field, &item) { + match get_member(&field.0, field.1, &item) { Some(Value::List(l)) => { for item in l { result.push_back(ReturnValue::Value(item.copy())); diff --git a/src/commands/ls.rs b/src/commands/ls.rs index 70cbf9d53..bfa86218e 100644 --- a/src/commands/ls.rs +++ b/src/commands/ls.rs @@ -7,8 +7,8 @@ use std::path::{Path, PathBuf}; pub fn ls(args: CommandArgs) -> Result { let env = args.env.lock().unwrap(); - let path = env.last().unwrap().path.to_path_buf(); - let obj = &env.last().unwrap().obj; + let path = env.back().unwrap().path.to_path_buf(); + let obj = &env.back().unwrap().obj; let mut full_path = PathBuf::from(path); match &args.positional.get(0) { Some(Spanned { diff --git a/src/commands/open.rs b/src/commands/open.rs index 15392e48d..18b8e242d 100644 --- a/src/commands/open.rs +++ b/src/commands/open.rs @@ -13,7 +13,7 @@ pub fn open(args: CommandArgs) -> Result { .env .lock() .unwrap() - .first() + .front() .unwrap() .path() .to_path_buf(); diff --git a/src/commands/save.rs b/src/commands/save.rs index 5d94496cf..ea16f5f0b 100644 --- a/src/commands/save.rs +++ b/src/commands/save.rs @@ -14,7 +14,7 @@ pub fn save(args: SinkCommandArgs) -> Result<(), ShellError> { .env .lock() .unwrap() - .first() + .front() .unwrap() .path() .to_path_buf(); diff --git a/src/commands/size.rs b/src/commands/size.rs index 4fe90e25f..fb9ff767e 100644 --- a/src/commands/size.rs +++ b/src/commands/size.rs @@ -13,7 +13,7 @@ pub fn size(args: CommandArgs) -> Result { .env .lock() .unwrap() - .first() + .front() .unwrap() .path() .to_path_buf(); diff --git a/src/commands/view.rs b/src/commands/view.rs index 30dce91d8..043b30f6a 100644 --- a/src/commands/view.rs +++ b/src/commands/view.rs @@ -34,7 +34,7 @@ pub fn view(args: CommandArgs) -> Result { .env .lock() .unwrap() - .first() + .front() .unwrap() .path() .to_path_buf(); diff --git a/src/context.rs b/src/context.rs index ff571369d..931190aca 100644 --- a/src/context.rs +++ b/src/context.rs @@ -13,16 +13,18 @@ pub struct Context { commands: IndexMap>, sinks: IndexMap>, crate host: Arc>, - crate env: Arc>>, + crate env: Arc>>, } impl Context { crate fn basic() -> Result> { + let mut env = VecDeque::new(); + env.push_back(Environment::basic()?); Ok(Context { commands: indexmap::IndexMap::new(), sinks: indexmap::IndexMap::new(), host: Arc::new(Mutex::new(crate::env::host::BasicHost)), - env: Arc::new(Mutex::new(vec![Environment::basic()?])), + env: Arc::new(Mutex::new(env)), }) } From 54be5bf16e494227425fa66bae64f9c1cc58fcc4 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Sun, 16 Jun 2019 06:36:17 +1200 Subject: [PATCH 2/2] Update errors and improve ctrl-c --- src/cli.rs | 75 +++++++++++++++++++++----------------- src/commands/cd.rs | 16 +++++--- src/commands/classified.rs | 32 ---------------- src/commands/enter.rs | 6 ++- src/commands/first.rs | 14 +++---- src/commands/from_xml.rs | 13 +++++-- src/commands/get.rs | 16 +++----- src/commands/ls.rs | 6 ++- src/commands/open.rs | 6 ++- src/commands/pick.rs | 14 +++---- src/commands/reject.rs | 14 +++---- src/commands/save.rs | 6 ++- src/commands/size.rs | 8 +++- src/commands/skip.rs | 14 +++---- src/commands/view.rs | 14 +++---- src/commands/where_.rs | 8 +++- src/errors.rs | 14 +++++++ 17 files changed, 139 insertions(+), 137 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 4c631a619..8f0027fe0 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -101,13 +101,10 @@ pub async fn cli() -> Result<(), Box> { 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> { 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> { .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> { 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, ctx: &mut Context (None, _) => break, (Some(ClassifiedCommand::Expr(_)), _) => { - return LineResult::Error(line.clone(), ShellError::unimplemented( - "Expression-only commands", - )) + return LineResult::Error( + line.clone(), + ShellError::unimplemented("Expression-only commands"), + ) } (_, Some(ClassifiedCommand::Expr(_))) => { - return LineResult::Error(line.clone(), ShellError::unimplemented( - "Expression-only commands", - )) + return LineResult::Error( + line.clone(), + ShellError::unimplemented("Expression-only commands"), + ) } - (Some(ClassifiedCommand::Sink(_)), Some(_)) => { - return LineResult::Error(line.clone(), ShellError::string("Commands like table, save, and autoview must come last in the pipeline")) + (Some(ClassifiedCommand::Sink(SinkCommand { name_span, .. })), Some(_)) => { + 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 = input.objects.collect().await; - left.run( - ctx, - input_vec, - )?; + left.run(ctx, input_vec)?; break; } @@ -290,13 +303,12 @@ async fn process_line(readline: Result, ctx: &mut Context Err(err) => return LineResult::Error(line.clone(), err), }, - ( - Some(ClassifiedCommand::Internal(left)), - Some(_), - ) => match left.run(ctx, input).await { - Ok(val) => ClassifiedInputStream::from_input_stream(val), - Err(err) => return LineResult::Error(line.clone(), err), - }, + (Some(ClassifiedCommand::Internal(left)), Some(_)) => { + match left.run(ctx, input).await { + 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, ctx: &mut Context Err(err) => return LineResult::Error(line.clone(), err), }, - ( - Some(ClassifiedCommand::External(left)), - Some(_), - ) => match left.run(ctx, input, StreamNext::Internal).await { - Ok(val) => val, - Err(err) => return LineResult::Error(line.clone(), err), - }, + (Some(ClassifiedCommand::External(left)), Some(_)) => { + match left.run(ctx, input, StreamNext::Internal).await { + 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, ctx: &mut Context LineResult::Success(line.clone()) } - Err(ReadlineError::Interrupted) => { - LineResult::Error("".to_string(), ShellError::string("CTRL-C")) - } + Err(ReadlineError::Interrupted) => LineResult::CtrlC, Err(ReadlineError::Eof) => { println!("CTRL-D"); LineResult::Break diff --git a/src/commands/cd.rs b/src/commands/cd.rs index 8716a4362..380c7d583 100644 --- a/src/commands/cd.rs +++ b/src/commands/cd.rs @@ -13,17 +13,23 @@ pub fn cd(args: CommandArgs) -> Result { 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 { 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")); diff --git a/src/commands/classified.rs b/src/commands/classified.rs index a83a384f9..e77236327 100644 --- a/src/commands/classified.rs +++ b/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) } } diff --git a/src/commands/enter.rs b/src/commands/enter.rs index 972225823..1b302b1b5 100644 --- a/src/commands/enter.rs +++ b/src/commands/enter.rs @@ -7,7 +7,11 @@ use std::path::{Path, PathBuf}; pub fn enter(args: CommandArgs) -> Result { 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 diff --git a/src/commands/first.rs b/src/commands/first.rs index afc1ff8f5..89bb4ff9c 100644 --- a/src/commands/first.rs +++ b/src/commands/first.rs @@ -5,15 +5,11 @@ use crate::prelude::*; pub fn first(args: CommandArgs) -> Result { if args.positional.len() == 0 { - if let Some(span) = args.name_span { - return Err(ShellError::labeled_error( - "First requires an amount", - "needs parameter", - span, - )); - } else { - return Err(ShellError::string("first requires an amount.")); - } + return Err(ShellError::maybe_labeled_error( + "First requires an amount", + "needs parameter", + args.name_span, + )); } let amount = args.positional[0].as_i64(); diff --git a/src/commands/from_xml.rs b/src/commands/from_xml.rs index 83e964e09..5d82d2657 100644 --- a/src/commands/from_xml.rs +++ b/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 { let out = args.input; + let span = args.name_span; Ok(out - .map(|a| match a { - Value::Primitive(Primitive::String(s)) => ReturnValue::Value(from_xml_string_to_value(s)), - _ => ReturnValue::Value(Value::Error(Box::new(ShellError::string( + .map(move |a| match a { + Value::Primitive(Primitive::String(s)) => { + 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()) diff --git a/src/commands/get.rs b/src/commands/get.rs index 3939e9fde..60b835d62 100644 --- a/src/commands/get.rs +++ b/src/commands/get.rs @@ -11,7 +11,7 @@ fn get_member(path: &str, span: Span, obj: &Value) -> Option { 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 { pub fn get(args: CommandArgs) -> Result { if args.positional.len() == 0 { - if let Some(span) = args.name_span { - return Err(ShellError::labeled_error( - "Get requires a field or field path", - "needs parameter", - span, - )); - } else { - return Err(ShellError::string("get requires fields.")); - } + return Err(ShellError::maybe_labeled_error( + "Get requires a field or field path", + "needs parameter", + args.name_span, + )); } let amount = args.positional[0].as_i64(); diff --git a/src/commands/ls.rs b/src/commands/ls.rs index bfa86218e..102cf7b9c 100644 --- a/src/commands/ls.rs +++ b/src/commands/ls.rs @@ -31,7 +31,11 @@ pub fn ls(args: CommandArgs) -> Result { 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, diff --git a/src/commands/open.rs b/src/commands/open.rs index 18b8e242d..b28067132 100644 --- a/src/commands/open.rs +++ b/src/commands/open.rs @@ -6,7 +6,11 @@ use std::path::{Path, PathBuf}; pub fn open(args: CommandArgs) -> Result { 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 diff --git a/src/commands/pick.rs b/src/commands/pick.rs index 73e953ad0..9de5e8068 100644 --- a/src/commands/pick.rs +++ b/src/commands/pick.rs @@ -5,15 +5,11 @@ use crate::prelude::*; pub fn pick(args: CommandArgs) -> Result { if args.positional.len() == 0 { - if let Some(span) = args.name_span { - return Err(ShellError::labeled_error( - "Pick requires fields", - "needs parameter", - span, - )); - } else { - return Err(ShellError::string("pick requires fields.")); - } + return Err(ShellError::maybe_labeled_error( + "Pick requires fields", + "needs parameter", + args.name_span, + )); } let fields: Result, _> = args.positional.iter().map(|a| a.as_string()).collect(); diff --git a/src/commands/reject.rs b/src/commands/reject.rs index 372adcf7c..ec5703738 100644 --- a/src/commands/reject.rs +++ b/src/commands/reject.rs @@ -5,15 +5,11 @@ use crate::prelude::*; pub fn reject(args: CommandArgs) -> Result { if args.positional.len() == 0 { - if let Some(span) = args.name_span { - return Err(ShellError::labeled_error( - "Reject requires fields", - "needs parameter", - span, - )); - } else { - return Err(ShellError::string("reject requires fields.")); - } + return Err(ShellError::maybe_labeled_error( + "Reject requires fields", + "needs parameter", + args.name_span, + )); } let fields: Result, _> = args.positional.iter().map(|a| a.as_string()).collect(); diff --git a/src/commands/save.rs b/src/commands/save.rs index ea16f5f0b..b5e61e4a0 100644 --- a/src/commands/save.rs +++ b/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 diff --git a/src/commands/size.rs b/src/commands/size.rs index fb9ff767e..6fdf78cfa 100644 --- a/src/commands/size.rs +++ b/src/commands/size.rs @@ -6,8 +6,12 @@ use std::fs::File; use std::io::prelude::*; pub fn size(args: CommandArgs) -> Result { - if args.positional.is_empty() { - return Err(ShellError::string("size requires at least one file")); + if args.positional.len() == 0 { + return Err(ShellError::maybe_labeled_error( + "Size requires a filepath", + "needs path", + args.name_span, + )); } let cwd = args .env diff --git a/src/commands/skip.rs b/src/commands/skip.rs index 38af0c1a7..d55615105 100644 --- a/src/commands/skip.rs +++ b/src/commands/skip.rs @@ -3,15 +3,11 @@ use crate::prelude::*; pub fn skip(args: CommandArgs) -> Result { if args.positional.len() == 0 { - if let Some(span) = args.name_span { - return Err(ShellError::labeled_error( - "Skip requires an amount", - "needs parameter", - span, - )); - } else { - return Err(ShellError::string("skip requires an amount.")); - } + return Err(ShellError::maybe_labeled_error( + "Skip requires an amount", + "needs parameter", + args.name_span, + )); } let amount = args.positional[0].as_i64(); diff --git a/src/commands/view.rs b/src/commands/view.rs index 043b30f6a..51e29cdba 100644 --- a/src/commands/view.rs +++ b/src/commands/view.rs @@ -4,15 +4,11 @@ use prettyprint::PrettyPrinter; pub fn view(args: CommandArgs) -> Result { if args.positional.len() == 0 { - if let Some(span) = args.name_span { - return Err(ShellError::labeled_error( - "View requires a filename", - "needs parameter", - span, - )); - } else { - return Err(ShellError::string("view requires a filename.")); - } + return Err(ShellError::maybe_labeled_error( + "View requires a filename", + "needs parameter", + args.name_span, + )); } let target = match args.positional[0].as_string() { diff --git a/src/commands/where_.rs b/src/commands/where_.rs index 45ac0c7fe..045c6f7df 100644 --- a/src/commands/where_.rs +++ b/src/commands/where_.rs @@ -25,8 +25,12 @@ impl Command for Where { } pub fn r#where(args: CommandArgs) -> Result { - if args.positional.is_empty() { - return Err(ShellError::string("select requires a field")); + if args.positional.len() == 0 { + return Err(ShellError::maybe_labeled_error( + "Where requires a condition", + "needs condition", + args.name_span, + )); } let block = args.positional[0].as_block()?; diff --git a/src/errors.rs b/src/errors.rs index 1023474d5..62e7534db 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -53,6 +53,20 @@ impl ShellError { ) } + crate fn maybe_labeled_error( + msg: impl Into, + label: impl Into, + span: Option, + ) -> 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) -> ShellError { ShellError::String(StringError::new(title.into(), Value::nothing())) }