provide env to commands and try to start provided path#10302
provide env to commands and try to start provided path#10302sholderbach merged 2 commits intonushell:mainfrom
Conversation
because your changes only touch a single command, the documentation that is rendered on the website will be updated automatically next release 😋 |
| Ok(status) => Err(format!("\nCommand {cmd:?} failed with {:?}", status)), | ||
| Err(err) => Err(format!("\nCommand {cmd:?} failed with {:?}", err)), |
There was a problem hiding this comment.
Debug formatting is debug formatting and shouldn't necessarily be used for regular user output.
This behavior did not change. It is exactly what open crate is doing under the hood: |
Oh sorry, yes you are right. I stand corrected. What prompted me with the comments around error handling is that in the output above, the command is not represented in a form you could run:
It would be great if this could represent a form that would be valid to execute in nushell. |
I agree. Using debug formatter was a poor choice, will change that. |
|
Awesome, thanks! |
|
This is how exapamle from OP looks like now: |
|
Awesome looks good! That empty string in the example still looks a bit weird but as it is a failure... hey) |
fixes nushell#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::shell::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.
fixes #8551
Description
Use
open::commandsfunction 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:
User-Facing Changes
startcommand now provides environment to the external command. This is how it worked innu 0.72, see linked issue.Tests + Formatting
startcommand 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 testtry_commandsfunction. But is still runs external commands, and robust test will require apriori knowledge which commands are necessary successful to run and which are not.Do docs need updating after this PR?