[run] improve UX by not requiring '--'

[mikeland73]

Summary

Inspired by @jimmidyson's awesome post 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

[LucilleH]

Can we move this clause up first, given that it is only applicable to run? Something like:

if len(args) <= 2 || args[0] != "run" || slices.Contains(args, "--") {
    return args
}

[LucilleH]

I think this logic make sense. Can we simplify a bit? Took me a while to understand what is going on. like:

for i < len(args) {
  arg := args[i]
  if !strings.HasPrefix(arg, "-") {
    // finally found it!
    return append(args[:i+1], append([]string{"--"}, args[i+1:]...)...)
  }

  if strings.Contains(arg, "=") {
    i++
    continue
  }
  
  var flag *pflag.Flag
  if strings.HasPrefix(arg, "--") {
    flag = runFlags.Lookup(strings.TrimLeft(arg, "-"))
  } else {
    flag = runFlags.ShorthandLookup(strings.TrimLeft(arg, "-"))
  }
  if flag != nil {
    if flag.NoOptDefVal == "" {
	i := i+2
    } else {
        i++
    }
}

[ipince]

Yeah. I would also add a comment in english stating what the loop is doing.

[ipince]

Something like: "adds a -- after all flags that come before the command that will be run. Thus, it transforms something like run --flag1 val1 command --flag2 val2 into run --flag1 val1 -- command --flag2 val2".

That would save a code reader quite a bit of time I think.

[ipince]

Approving to unblock!

[ipince]

Something like: "adds a -- after all flags that come before the command that will be run. Thus, it transforms something like run --flag1 val1 command --flag2 val2 into run --flag1 val1 -- command --flag2 val2".

That would save a code reader quite a bit of time I think.

[ipince]

what happens if they do?

[mikeland73]

It will fail if you don't include -- and have non-devbox flags in there, e.g.

devbox -c path/to/conf run python --version

Will say --version is not a valid flag.

This can be fixed, it just requires finding the run command and ensuring everything before it is a flag and not a command.

[jimmidyson]

Just FYI the comment re devbox run in my blog post wasn't about the need for --, I actually like that too demarcate the command, but about needing devbox run at all - I couldn't figure out a way to massage the environment so PATH, etc was configured properly to use the devbox installed tools.

[mikeland73]

thanks for the clarification @jimmidyson!

We chatted about that as well and I think @LucilleH has some ideas to make the action better.

Not sure if it helps your particular use case, but if you do:

eval "$(devbox shellenv)" in a directory with a devbox.json it's the equivalent of turning the current shell into the devbox environment. I think in Github actions the environment is not persisted between steps so not sure if this is helpful, we may need to make a chance to the action itself.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
devbox-docs	❌ Failed (Inspect)			Aug 1, 2023 7:18pm