diff --git a/crates/nu-command/src/filesystem/start.rs b/crates/nu-command/src/filesystem/start.rs index b295b5f368..619865daed 100644 --- a/crates/nu-command/src/filesystem/start.rs +++ b/crates/nu-command/src/filesystem/start.rs @@ -1,9 +1,8 @@ use itertools::Itertools; use nu_engine::{command_prelude::*, env_to_strings}; -use nu_path::canonicalize_with; +use nu_protocol::ShellError; use std::{ ffi::{OsStr, OsString}, - path::Path, process::Stdio, }; @@ -16,7 +15,7 @@ impl Command for Start { } fn description(&self) -> &str { - "Open a folder, file or website in the default application or viewer." + "Open a folder, file, or website in the default application or viewer." } fn search_terms(&self) -> Vec<&str> { @@ -26,7 +25,7 @@ impl Command for Start { fn signature(&self) -> nu_protocol::Signature { Signature::build("start") .input_output_types(vec![(Type::Nothing, Type::Any)]) - .required("path", SyntaxShape::String, "Path to open.") + .required("path", SyntaxShape::String, "Path or URL to open.") .category(Category::FileSystem) } @@ -42,59 +41,29 @@ impl Command for Start { item: nu_utils::strip_ansi_string_unlikely(path.item), span: path.span, }; - let path_no_whitespace = &path.item.trim_end_matches(|x| matches!(x, '\x09'..='\x0d')); - // only check if file exists in current current directory - let file_path = Path::new(path_no_whitespace); - if file_path.exists() { - open_path(path_no_whitespace, engine_state, stack, path.span)?; - } else if file_path.starts_with("https://") || file_path.starts_with("http://") { - let url = url::Url::parse(&path.item).map_err(|_| ShellError::GenericError { - error: format!("Cannot parse url: {}", &path.item), - msg: "".to_string(), - span: Some(path.span), - help: Some("cannot parse".to_string()), - inner: vec![], - })?; + let path_no_whitespace = path.item.trim_end_matches(|x| matches!(x, '\x09'..='\x0d')); + // Attempt to parse the input as a URL + if let Ok(url) = url::Url::parse(path_no_whitespace) { open_path(url.as_str(), engine_state, stack, path.span)?; - } else { - // try to distinguish between file not found and opening url without prefix - let cwd = engine_state.cwd(Some(stack))?; - if let Ok(canon_path) = canonicalize_with(path_no_whitespace, cwd) { - open_path(canon_path, engine_state, stack, path.span)?; - } else { - // open crate does not allow opening URL without prefix - let path_with_prefix = Path::new("https://").join(&path.item); - let common_domains = ["com", "net", "org", "edu", "sh"]; - if let Some(url) = path_with_prefix.to_str() { - let url = url::Url::parse(url).map_err(|_| ShellError::GenericError { - error: format!("Cannot parse url: {}", &path.item), - msg: "".into(), - span: Some(path.span), - help: Some("cannot parse".into()), - inner: vec![], - })?; - if let Some(domain) = url.host() { - let domain = domain.to_string(); - let ext = Path::new(&domain).extension().and_then(|s| s.to_str()); - if let Some(url_ext) = ext { - if common_domains.contains(&url_ext) { - open_path(url.as_str(), engine_state, stack, path.span)?; - } - } - } - return Err(ShellError::GenericError { - error: format!("Cannot find file or url: {}", &path.item), - msg: "".into(), - span: Some(path.span), - help: Some("Use prefix https:// to disambiguate URLs from files".into()), - inner: vec![], - }); - } - }; + return Ok(PipelineData::Empty); } - Ok(PipelineData::Empty) + // If it's not a URL, treat it as a file path + let cwd = engine_state.cwd(Some(stack))?; + let full_path = cwd.join(path_no_whitespace); + // Check if the path exists or if it's a valid file/directory + if full_path.exists() { + open_path(full_path, engine_state, stack, path.span)?; + return Ok(PipelineData::Empty); + } + // If neither file nor URL, return an error + Err(ShellError::GenericError { + error: format!("Cannot find file or URL: {}", &path.item), + msg: "".into(), + span: Some(path.span), + help: Some("Ensure the path or URL is correct and try again.".into()), + inner: vec![], + }) } - fn examples(&self) -> Vec { vec![ Example { @@ -113,15 +82,20 @@ impl Command for Start { result: None, }, Example { - description: "Open a pdf with the default pdf viewer", + description: "Open a PDF with the default PDF viewer", example: "start file.pdf", result: None, }, Example { - description: "Open a website with default browser", + description: "Open a website with the default browser", example: "start https://www.nushell.sh", result: None, }, + Example { + description: "Open an application-registered protocol URL", + example: "start obsidian://open?vault=Test", + result: None, + }, ] } } @@ -142,47 +116,48 @@ fn try_commands( span: Span, ) -> Result<(), ShellError> { let env_vars_str = env_to_strings(engine_state, stack)?; - let cmd_run_result = commands.into_iter().map(|mut cmd| { + let mut last_err = None; + + for mut cmd in commands { let status = cmd .envs(&env_vars_str) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()) .status(); - match status { - Ok(status) if status.success() => Ok(()), - Ok(status) => Err(format!( - "\nCommand `{}` failed with {}", - format_command(&cmd), - status - )), - Err(err) => Err(format!( - "\nCommand `{}` failed with {}", - format_command(&cmd), - err - )), - } - }); - for one_result in cmd_run_result { - if let Err(err_msg) = one_result { - return Err(ShellError::ExternalCommand { - label: "No command found to start with this path".to_string(), - help: "Try different path or install appropriate command\n".to_string() + &err_msg, - span, - }); - } else if one_result.is_ok() { - break; + match status { + Ok(status) if status.success() => return Ok(()), + Ok(status) => { + last_err = Some(format!( + "Command `{}` failed with exit code: {}", + format_command(&cmd), + status.code().unwrap_or(-1) + )); + } + Err(err) => { + last_err = Some(format!( + "Command `{}` failed with error: {}", + format_command(&cmd), + err + )); + } } } - Ok(()) + + Err(ShellError::ExternalCommand { + label: "Failed to start the specified path or URL".to_string(), + help: format!( + "Try a different path or install the appropriate application.\n{}", + last_err.unwrap_or_default() + ), + span, + }) } fn format_command(command: &std::process::Command) -> String { - let parts_iter = std::iter::repeat(command.get_program()) - .take(1) - .chain(command.get_args()); - Itertools::intersperse(parts_iter, " ".as_ref()) + let parts_iter = std::iter::once(command.get_program()).chain(command.get_args()); + Itertools::intersperse(parts_iter, OsStr::new(" ")) .collect::() .to_string_lossy() .into_owned() diff --git a/crates/nu-command/tests/commands/start.rs b/crates/nu-command/tests/commands/start.rs new file mode 100644 index 0000000000..60f8f057b1 --- /dev/null +++ b/crates/nu-command/tests/commands/start.rs @@ -0,0 +1,120 @@ +use super::*; +use nu_protocol::{ + ast::Call, + engine::{EngineState, Stack, StateWorkingSet}, + PipelineData, Span, Spanned, Type, Value, +}; +use nu_engine::test_help::{convert_single_value_to_cmd_args, eval_block_with_input}; +use nu_engine::{current_dir, eval_expression}; +use std::path::PathBuf; + +/// Create a minimal test engine state and stack to run commands against. +fn create_test_context() -> (EngineState, Stack) { + let mut engine_state = EngineState::new(); + let mut stack = Stack::new(); + + // A working set is needed for storing definitions in the engine state. + let _working_set = StateWorkingSet::new(&mut engine_state); + + // Add the `Start` command to the engine state so we can run it. + let start_cmd = Start; + engine_state.add_cmd(Box::new(start_cmd)); + + (engine_state, stack) +} + +#[test] +fn test_start_valid_url() { + let (engine_state, mut stack) = create_test_context(); + + // For safety in tests, we won't actually open anything, + // but we can still check that the command resolves as a URL + // and attempts to run. Typically, you'd mock `open::commands` if needed. + + // Create call for: `start https://www.example.com` + let path = "https://www.example.com".to_string(); + let span = Span::test_data(); + let call = Call::test( + "start", + // The arguments for `start` are just the path in this case + vec![Value::string(path, span)], + ); + + let result = Start.run( + &engine_state, + &mut stack, + &call, + PipelineData::Empty, + ); + + assert!( + result.is_ok(), + "Expected successful run with a valid URL, got error: {:?}", + result.err() + ); +} + +#[test] +fn test_start_valid_local_path() { + let (engine_state, mut stack) = create_test_context(); + + // Here we'll simulate opening the current directory (`.`). + let path = ".".to_string(); + let span = Span::test_data(); + let call = Call::test( + "start", + vec![Value::string(path, span)], + ); + + let result = Start.run( + &engine_state, + &mut stack, + &call, + PipelineData::Empty, + ); + + // If the environment is correctly set, it should succeed. + // If you're running in a CI environment or restricted environment + // this might fail, so you may need to mock `open` calls. + assert!( + result.is_ok(), + "Expected successful run opening current directory, got error: {:?}", + result.err() + ); +} + +#[test] +fn test_start_nonexistent_local_path() { + let (engine_state, mut stack) = create_test_context(); + + // Create an obviously invalid path + let path = "this_file_does_not_exist_hopefully.txt".to_string(); + let span = Span::test_data(); + let call = Call::test( + "start", + vec![Value::string(path, span)], + ); + + let result = Start.run( + &engine_state, + &mut stack, + &call, + PipelineData::Empty, + ); + + // We expect an error since the file does not exist + assert!( + result.is_err(), + "Expected an error for a non-existent file path" + ); + + if let Err(ShellError::GenericError { error, .. }) = result { + assert!( + error.contains("Cannot find file or URL"), + "Expected 'Cannot find file or URL' in error, found: {}", + error + ); + } else { + panic!("Unexpected error type, expected ShellError::GenericError"); + } +} \ No newline at end of file