Skip to content

Commit

Permalink
non-HTTP(s) URLs now works with start (#14370)
Browse files Browse the repository at this point in the history
# Description
this PR should close #14315
This PR enhances the start command in Nushell to handle both files and
URLs more effectively, including support for custom URL schemes.
Previously, the start command only reliably opened HTTP and HTTPS URLs,
and custom schemes like Spotify and Obsidian which were not handled
earlier.

1. **Custom URL Schemes Support:**
- Added support for opening custom URL schemes

2. **Detailed Error Messages:**
- Improved error reporting for failed external commands.

- Captures and displays error output from the system to aid in
debugging.

**Example**

**Opening a custom URL scheme (e.g., Spotify):**

```bash
start spotify:track:4PTG3Z6ehGkBFwjybzWkR8?si=f9b4cdfc1aa14831
```

Opens the specified track in the Spotify application.

**User-Facing Changes**

- **New Feature:** The start command now supports opening URLs with
custom schemes
  • Loading branch information
anomius authored Jan 24, 2025
1 parent cdbb3ee commit fd684a2
Show file tree
Hide file tree
Showing 2 changed files with 180 additions and 85 deletions.
145 changes: 60 additions & 85 deletions crates/nu-command/src/filesystem/start.rs
Original file line number Diff line number Diff line change
@@ -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,
};

Expand All @@ -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> {
Expand All @@ -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)
}

Expand All @@ -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<nu_protocol::Example> {
vec![
Example {
Expand All @@ -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,
},
]
}
}
Expand All @@ -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::<OsString>()
.to_string_lossy()
.into_owned()
Expand Down
120 changes: 120 additions & 0 deletions crates/nu-command/tests/commands/start.rs
Original file line number Diff line number Diff line change
@@ -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");
}
}

0 comments on commit fd684a2

Please sign in to comment.