Early flags check before "PersistentPreRunE"

[vax-r]

Summary

Utilize Cobra's "MarkFlagsMutuallyExclusive()" on root command. This shifts validation to flag parsing and remove the manual check from "PersistentPreRunE", which can be applied to all subcommands before hooks fire.

Test

Simple manual test:

$ limactl start --tty --yes
FATA[0000] if any flags in the group [tty yes] are set none of the others can be; [tty yes] were all set

Reference

• MarkFlagsMutuallyExclusive()

[jandubois]

I just wish the Cobra message was a bit more readable by the enduser:

FATA[0000] if any flags in the group [tty yes] are set none of the others can be; [tty yes] were all set

[vax-r]

I just wish the Cobra message was a bit more readable by the enduser:

FATA[0000] if any flags in the group [tty yes] are set none of the others can be; [tty yes] were all set

Yes I agree, though I don't think we can change cobra message from our end. I can try to send patches to them for user-friendly message if you think this is worthy enough.

[jandubois]

I can try to send patches to them for user-friendly message if you think this is worthy enough.

I don't know if this is going to be worth it.

I think the first step should be to catalog all conflicting options throughout all subcommands, and see how they are being handled (if at all). And then come up with a comprehensive strategy that can cover all situations. Just saving 3 lines of code for a singular instance doesn't seem to be worth it if it makes the error message less readable.

Just off the top of my head:

• limactl create --list-templates is incompatible with most (all?) other options. I think we don't check, but ignore everything if --list-templates is specified.
• limactl list --list-fields is similar.
• limactl list has a bunch of special cases that Cobra probably won't be able to handle:
  • --quiet can only be used with --format=table (I thought I had changed this, but don't see it right now)
  • --yq can only be used with --format=yaml, --format=json, and --json.
  • --json cannot be used with --format

There are probably more. So I think it would be more useful to define a common error message for this situation, and make sure we use the same words everywhere instead of creating ad-hoc error strings each time.

I don't think Cobra is flexible enough to handle this, so we should have our own support functions, if that is possible.

[AkihiroSuda]

I don't think Cobra is flexible enough to handle this, so we should have our own support functions, if that is possible.

If we can extend the upstream cobra that would be better

[vax-r]

@jandubois - Thanks for so much detail, I'll look into them and try to come up with some strategy on how to handle them with a consistent way and also make the error message readable.

@AkihiroSuda - No problem, I'll try to reach out to them at the same time, to see if we can extend the log/error message to be specified by ourselves.