Re: [PATCH v3 0/5] Skip 'cache_tree_update()' when 'prime_cache_tree()' is called immediate after

[Victoria Dye <vdye@github.com>]

SZEDER Gábor wrote:
> On Thu, Nov 10, 2022 at 07:06:00PM +0000, Victoria Dye via GitGitGadget wrote:
>> Changes since V2
>> ================
>>
>>  * Cleaned up option handling & provided more informative error messages in
>>    'test-tool cache-tree'. The changes don't affect any behavior in the
>>    added tests & 'test-tool cache-tree' won't be used outside of
>>    development, but the improvements here will help future readers avoid
>>    propagating error-prone implementations.
>>    * Note that the suggestion to change the "unknown subcommand" error to a
>>      'usage()' error was not taken, as it would be somewhat cumbersome to
>>      use a formatted string with it.
> 
> I'm not sure I understand what's cumbersome.  It's as simple as:
> 
>    if (...) {
>        error(_("unknown subcommand: `%s'"), argv[0]);
>        usage_with_options(test_cache_tree_usage, options);
>    }

To be honest, the cumbersome approach I was thinking of was 'sprintf()'-ing
the subcommand into the string and calling 'usage()' with that - your
suggestion is certainly much simpler. However, as a matter of personal
preference, I still think the 'die()' is sufficient in the context of this
test helper (especially given that other test helpers do the same).

> 
>>      This is in line with other custom
>>      subcommand parsing in Git, such as in 'fsmonitor--daemon.c'.
> 
> The option parsing in 'fsmonitor--daemon.c' is broken, please don't
> consider it as an example to follow.

While I understand your desire to helpfully guide users, I don't see
anything to suggest that particular example is "broken." The error conveys
the cause of the problem to a user, who could then run without arguments (or
with -h) to see what the valid subcommands are. And, in the case of this
test helper, I'm not particularly concerned with perfecting the (already
subjective) user experience, given that it's an internal-only tool.

If there are examples of proper usage patterns that future commands should
follow, I'd recommend updating 'CodingGuidelines' and/or
'MyFirstContribution' to mention them. Codifying recommendations like that
can help avoid churn in reviews and, long-term, push Git to align on a
uniform style.