Allow false in compose CLI argument

[ChihweiLHBird]

Allow compose to be passed as a false.

• --compose=false (plus boolish spellings like 0, no, off) to actually select ComposeMode::Skip, which is previously unreachable from the CLI
• Bare --compose still works, defaulting to true via default_missing_value
• require_equals = true prevents --compose false from swallowing the positional registry reference as the flag's value

[itowlson]

I am confused. What is the purpose here?

[ChihweiLHBird]

I am confused. What is the purpose here?

This CLI argument is always true regardless how a user configures it, even if they try to configure it as a false. This change allows users to set it to false if they want.

[itowlson]

But they can just leave it out. Flags like --build usually don't take a value, it's presence or non-presence. You express false by non-presence. What's the use case in which someone needs it to be present but false?

[ChihweiLHBird]

But they can just leave it out. Flags like --build usually don't take a value, it's presence or non-presence. You express false by non-presence. What's the use case in which someone needs it to be present but false?

It defaults to true so when non-presence it's still true... This is controlled by default_value_t.

I can also remove the default_value_t to achieve the behavior that presence represents true or non-presence representing false, but this will be a breaking change for most people who don't use this argument.

The current PR can avoid breaking change, but the alternative might be more correct and simple. Either way works, what do you think?

[itowlson]

Oh! I think I have confused myself. Sorry! And thanks for patiently clarifying.

@fibonacci1729 I think this was a flag that you added. I recall choosing this over --skip-compose to avoid awkward double negatives, and that we wanted to default to pre-composing so it had to default true, but is Zhiwei correct that it can't currently be turned off (i.e. push an app non-pre-composed)? Could this be a regression from the Clap 4 migration?

[ChihweiLHBird]

@itowlson no problem, and thanks for looking into this!

[fibonacci1729]

Thanks for catching this @ChihweiLHBird ! Indeed your assessment is correct that the flag was never settable (AFAICT this wasn't due to the clap4 migration). In practice this isn't a huge concern because most downstream vendors require precomposing (i.e. SpinKube) while others that I know of programmatically call the registry APIs with ComposeMode::Skip.

This PR LGTM. One thing that would be nice to see is a parse test in registry.rs -- Push::try_parse_from (following the pattern in up.rs) that covers bare --compose -> true with reference intact; --compose=false/=0/=no/=off -> false; --compose false <ref> -> Err; and omitted -> true.