Skip to content

Commit 767becb

Browse files
authored
[run] improve UX by not requiring '--' (#1326)
## Summary Inspired by @jimmidyson's awesome [post](https://eng.d2iq.com/blog/a-long-journey-to-cross-platform-developer-tooling-utopia-for-now/) I thought we could remove the need for `--` every time you want to run something with devbox that has flags. This change assumes that when a user runs `devbox run` the first non-flag arg after "run" is part of the command or the script name the user wants to run. This relies on `run` never having subcommands (which I think will be true forever). So we transform: `devbox run python --version` into `devbox run -- python --version` If command already has `--` then we do nothing. This change is fully backward compatible. @LucilleH I'm somewhat sure I fixed the issues from demo. PTAL ## How was it tested? ``` devbox add python devbox run python --version Python 3.11.4 devbox run -c examples/development/python/pip python --version ```
1 parent bd2ea97 commit 767becb

File tree

4 files changed

+78
-3
lines changed

4 files changed

+78
-3
lines changed

examples/development/python/pip/devbox.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@
1212
"version": "23.0.1"
1313
}
1414
}
15-
}
15+
}

internal/boxcli/midcobra/midcobra.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ func (ex *midcobraExecutable) Execute(ctx context.Context, args []string) int {
5454
m.preRun(ex.cmd, args)
5555
}
5656

57+
// set args (needed in case caller transforms args in any way)
58+
ex.cmd.SetArgs(args)
59+
5760
// Execute the cobra command:
5861
err := ex.cmd.Execute()
5962

internal/boxcli/root.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,12 @@ func RootCmd() *cobra.Command {
9595

9696
func Execute(ctx context.Context, args []string) int {
9797
defer debug.Recover()
98-
exe := midcobra.New(RootCmd())
98+
rootCmd := RootCmd()
99+
exe := midcobra.New(rootCmd)
99100
exe.AddMiddleware(traceMiddleware)
100101
exe.AddMiddleware(midcobra.Telemetry())
101102
exe.AddMiddleware(debugMiddleware)
102-
return exe.Execute(ctx, args)
103+
return exe.Execute(ctx, wrapArgsForRun(rootCmd, args))
103104
}
104105

105106
func Main() {

internal/boxcli/run.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@ package boxcli
55

66
import (
77
"fmt"
8+
"strings"
89

10+
"github.com/samber/lo"
911
"github.com/spf13/cobra"
12+
"github.com/spf13/pflag"
13+
"golang.org/x/exp/slices"
1014

1115
"go.jetpack.io/devbox"
1216
"go.jetpack.io/devbox/internal/boxcli/usererr"
@@ -113,3 +117,70 @@ func parseScriptArgs(args []string, flags runCmdFlags) (string, string, []string
113117

114118
return flags.config.path, script, scriptArgs, nil
115119
}
120+
121+
func wrapArgsForRun(rootCmd *cobra.Command, args []string) []string {
122+
// if the first argument is not "run", we don't need to do anything. If there
123+
// are 2 or fewer arguments, we also don't need to do anything because there
124+
// are no flags after a non-run non-flag arg.
125+
// IMPROVEMENT: technically users can pass a flag before the subcommand "run"
126+
if len(args) <= 2 || args[0] != "run" || slices.Contains(args, "--") {
127+
return args
128+
}
129+
130+
cmd, found := lo.Find(
131+
rootCmd.Commands(),
132+
func(item *cobra.Command) bool { return item.Name() == "run" },
133+
)
134+
if !found {
135+
return args
136+
}
137+
_ = cmd.InheritedFlags() // bug in cobra requires this to be called to ensure flags contains inherited flags.
138+
runFlags := cmd.Flags()
139+
// typical args can be of the form:
140+
// run --flag1 val1 -f val2 --flag3=val3 --bool-flag python --version
141+
// We handle each different type of flag
142+
// (flag with equals, long-form, short-form, and defaulted flags)
143+
// Note that defaulted does not mean initial value, it only means flags
144+
// that don't require a value.
145+
// For example, --bool-flag has NoOptDefVal set to "true".
146+
i := 1
147+
for i < len(args) {
148+
arg := args[i]
149+
if !strings.HasPrefix(arg, "-") {
150+
// We found and argument that is not part of the flags, so we can stop
151+
// This inserts a "--" before the first non-flag argument
152+
// Turning
153+
// run --flag1 val1 command --flag2 val2
154+
// into
155+
// run --flag1 val1 -- command --flag2 val2
156+
return append(args[:i+1], append([]string{"--"}, args[i+1:]...)...)
157+
}
158+
159+
if strings.HasPrefix(arg, "-") && strings.Contains(arg, "=") {
160+
// This is a flag with an equals sign, so we can skip it
161+
i++
162+
continue
163+
}
164+
165+
var flag *pflag.Flag
166+
if strings.HasPrefix(arg, "--") {
167+
flag = runFlags.Lookup(strings.TrimLeft(arg, "-"))
168+
} else {
169+
flag = runFlags.ShorthandLookup(strings.TrimLeft(arg, "-"))
170+
}
171+
if flag == nil {
172+
// found an invalid flag, just return args as-is
173+
return args
174+
}
175+
if flag.NoOptDefVal == "" {
176+
// This is a non-boolean flag, e.g. --flag1 val1
177+
i += 2
178+
} else {
179+
// This is a boolean flag, e.g. --bool-flag
180+
i++
181+
}
182+
}
183+
184+
// This means there is no non-flag command. Just return as is.
185+
return args
186+
}

0 commit comments

Comments
 (0)