On 7/25/2022 8:38 AM, SZEDER Gábor wrote:

Several Git commands have subcommands to implement mutually exclusive
"operation modes", and they usually parse their subcommand argument
with a bunch of if-else if statements.

Teach parse-options to handle subcommands as well, which will result
in shorter and simpler code with consistent error handling and error
messages on unknown or missing subcommand, and it will also make
possible for our Bash completion script to handle subcommands
programmatically in a follow-up series [1].

Since this has become an increasingly common pattern, I appreciate
that you have standardized it here.

Patches 1-8 are a mix of preparatory cleanups, documentation updates, and
test coverage improvements.

Patch 9 is the most important one, which teaches parse-options to handle
subcommands.

The remaining 10-20 convert most builtin commands with subcommands one by
one to use parse-options to handle their subcommand parameters.

I focused on reading the changes to the builtins I have experience with
(commit-graph, maintenance, multi-pack-index, sparse-checkout, worktree)
and found the adaptation to the new model very clean.

The one common thing I saw was that you are updating a function pointer
that you name "fn" but it could be more informative on first reading if
it was named something like "subcommand_fn".

This patch series has two conflicts with 'seen' (but none with 'next'):

An upcoming conflict is going to be my bundle URI topic which is going
to replace 'git fetch --bundle-uri' with 'git bundle fetch' in its next
version. I'll wait to see how Junio applies this series and I'll think
about splitting the current series into "design doc" and "implementation"
just so I can build on your work here instead of colliding.

Thanks,
-Stolee