provide env to commands and try to start provided path (#10302)

fixes #8551

# Description
Use `open::commands` function to get list of command available for
starting given path. run commands directly, providing environment, until
one of them is successful.

example of output if start was not successful:
```
~\code\nushell> start ..\nustart\a.myext                                                                    09/12/2023 01:37:55 PM
Error: nu:🐚:external_command

  × External command failed
   ╭─[entry #1:1:1]
 1 │ start ..\nustart\a.myext
   ·       ─────────┬────────
   ·                ╰── No command found to start with this path
   ╰────
  help: Try different path or install appropriate command

        Command `cmd /c start "" "..\nustart\a.myext"` failed with exit code: 1
```

# User-Facing Changes
`start` command now provides environment to the external command. This
is how it worked in `nu 0.72`, see linked issue.

# Tests + Formatting
`start` command didn't have any tests and this PR does not add any.
Integration-level tests will require setup specific to OS and
potentially change global environment on testing machine. For unit-level
test it is possible to test `try_commands` function. But is still runs
external commands, and robust test will require apriori knowledge which
commands are necessary successful to run and which are not.
This commit is contained in:
Eugeen Sablin 2023-09-12 14:03:41 +02:00 committed by GitHub
parent d53b0a99d0
commit 9e1e2a4320
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -1,11 +1,15 @@
use itertools::Itertools;
use nu_engine::env_to_strings;
use nu_engine::CallExt; use nu_engine::CallExt;
use nu_path::canonicalize_with; use nu_path::canonicalize_with;
use nu_protocol::ast::Call; use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{ use nu_protocol::{
Category, Example, PipelineData, ShellError, Signature, Spanned, SyntaxShape, Type, Category, Example, PipelineData, ShellError, Signature, Span, Spanned, SyntaxShape, Type,
}; };
use std::ffi::{OsStr, OsString};
use std::path::Path; use std::path::Path;
use std::process::Stdio;
#[derive(Clone)] #[derive(Clone)]
pub struct Start; pub struct Start;
@ -46,7 +50,7 @@ impl Command for Start {
// only check if file exists in current current directory // only check if file exists in current current directory
let file_path = Path::new(path_no_whitespace); let file_path = Path::new(path_no_whitespace);
if file_path.exists() { if file_path.exists() {
open::that(path_no_whitespace)?; open_path(path_no_whitespace, engine_state, stack, path.span)?;
} else if file_path.starts_with("https://") || file_path.starts_with("http://") { } else if file_path.starts_with("https://") || file_path.starts_with("http://") {
let url = url::Url::parse(&path.item).map_err(|_| { let url = url::Url::parse(&path.item).map_err(|_| {
ShellError::GenericError( ShellError::GenericError(
@ -57,13 +61,13 @@ impl Command for Start {
Vec::new(), Vec::new(),
) )
})?; })?;
open::that(url.as_str())?; open_path(url.as_str(), engine_state, stack, path.span)?;
} else { } else {
// try to distinguish between file not found and opening url without prefix // try to distinguish between file not found and opening url without prefix
if let Ok(path) = if let Ok(canon_path) =
canonicalize_with(path_no_whitespace, std::env::current_dir()?.as_path()) canonicalize_with(path_no_whitespace, std::env::current_dir()?.as_path())
{ {
open::that(path)?; open_path(canon_path, engine_state, stack, path.span)?;
} else { } else {
// open crate does not allow opening URL without prefix // open crate does not allow opening URL without prefix
let path_with_prefix = Path::new("https://").join(&path.item); let path_with_prefix = Path::new("https://").join(&path.item);
@ -83,7 +87,7 @@ impl Command for Start {
let ext = Path::new(&domain).extension().and_then(|s| s.to_str()); let ext = Path::new(&domain).extension().and_then(|s| s.to_str());
if let Some(url_ext) = ext { if let Some(url_ext) = ext {
if common_domains.contains(&url_ext) { if common_domains.contains(&url_ext) {
open::that(url.as_str())?; open_path(url.as_str(), engine_state, stack, path.span)?;
} }
} }
} }
@ -130,3 +134,65 @@ impl Command for Start {
] ]
} }
} }
fn open_path(
path: impl AsRef<OsStr>,
engine_state: &EngineState,
stack: &Stack,
span: Span,
) -> Result<(), ShellError> {
try_commands(open::commands(path), engine_state, stack, span)
}
fn try_commands(
commands: Vec<std::process::Command>,
engine_state: &EngineState,
stack: &Stack,
span: Span,
) -> Result<(), ShellError> {
let env_vars_str = env_to_strings(engine_state, stack)?;
commands
.into_iter()
.map(|mut cmd| {
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
)),
}
})
.take_while_inclusive(|result| result.is_err())
.fold(Err("".to_string()), |combined_result, next_result| {
combined_result.or_else(|combined_message| {
next_result.map_err(|next_message| combined_message + &next_message)
})
})
.map_err(|message| ShellError::ExternalCommand {
label: "No command found to start with this path".to_string(),
help: "Try different path or install appropriate command\n".to_string() + &message,
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())
.collect::<OsString>()
.to_string_lossy()
.into_owned()
}