cmd/analyze: use exit status 1 and 2 for errors, improve error messages for invalid cli arguments

[maxfisher-g]

1. print errors for missing flags
2. better use of exit codes
3. [internal] consoliate context object initialisation

[adg]

I think the headline news of this change is that you're now returning a nonzero exit code if the command fails; that should probably be in the commit subject line.

[adg]

Is there a compelling reason to distinguish the usage errors vs other failures? The code gets a lot simpler if you just return either status 0 or 1.

[calebbrown]

Also, you could just return error and leave the exit code decision up to main()

[maxfisher-g]

I was just thinking of matching other unix utilities in this way. Since the options are quite complex for this program (and there is also an intermediate docker layer whose options are handled by a helper script), I think it helps for usability to distinguish user errors from program errors.

[adg]

Is anything checking for the difference though?
If I saw "exit code 1" or "exit code 2" I'd still need to look at the other logs to figure out what went wrong.
I think unless another program is going to interpret your exit code in some meaningful way, it's better to err on the side of simplicity and just return 0 or 1.

[maxfisher-g]

Fair enough. I'll implement the error type as you suggested in the other comment but just return exit code 1 on any error until there's a clear need to return distinct error codes.

[maxfisher-g]

I think the headline news of this change is that you're now returning a nonzero exit code if the command fails; that should probably be in the commit subject line.

fair point, I'll add this to the final commit message and/or PR heading

[adg]

The more idiomatic way to do this in Go is to return an error and check for its type to determine the kind of error. In this case, you can declare a usageError type and then return that, check for it in main, and return exit code 2 in that case. (for all other errors just return error code 1)

type usageError struct {
  error
}

func usagef(format string, args ...any) error {
  return usageError{fmt.Errorf(format, args...)}
}

func run() error {
  ...
  if err := featureflags.Update(*features); err != nil {
    return usagef("failed to parse flags: %w", err)
  }
  ...
  return nil
}

func main() {
  if err := run(); err !=  nil {
    fmt.Fprintln(os.Stderr, err)
    if errors.As(err, &usageError{}) {
      os.Exit(2)
    }
    os.Exit(1)
  }
}

[maxfisher-g]

Thanks for the suggestion, this idiom is really nice! I'll implement it.

[adg]

Use %q

[adg]

LGTM modulo my last comments