Subject: Re: [PATCH 00/20] parse-options: handle subcommands

From: Derrick Stolee <[email-masked]>
Date: Mon, 25 Jul 2022 09:15:45 -0400

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