Skip to content

Conversation

ipince
Copy link
Contributor

@ipince ipince commented Jan 24, 2023

Summary

TSIA

How was it tested?

devbox run echo "hi"
devbox run hello
devbox run cowsay -d hello      # fails because -d is interpreted as devbox flag
devbox run -- cowsay -d hello   # works!

@ipince ipince requested review from savil and mohsenari January 24, 2023 15:26
if script == "" || !slices.Contains(scripts, script) {
return errors.Errorf("no script found with name \"%s\". "+
"Here's a list of the existing scripts in devbox.json: %v", script, scripts)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when a user has a script in their devbox.json named "ls" and they run devbox run ls ?
Does the script alias take precedent over the command ls? or vice versa?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script takes precedence

return errors.New("attempted to run script but did not specify script name")
func RunScript(nixShellFilePath string, projectDir string, cmdWithArgs string, additionalEnv []string) error {
if cmdWithArgs == "" {
return errors.New("attempted to run a command but gave an empty command")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can be both a command or a script right? so can we say "attempted to run a command or a script but gave an empty one?" or something like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I was assuming that a "command" can be a script, but I can change the wording to make it clearer.

@ipince
Copy link
Contributor Author

ipince commented Jan 24, 2023

PTAL

Base automatically changed from rodrigo/script-args to main January 24, 2023 22:11
@ipince ipince merged commit 8073762 into main Jan 24, 2023
@ipince ipince deleted the rodrigo/run-cmd branch January 24, 2023 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants