Junio C Hamano <gits...@pobox.com> writes:

> Paul-Sebastian Ungureanu <ungureanupaulsebast...@gmail.com> writes:
>> Basically, the option parser only parses strings that represent
>> commits and the ref-filter performs the commit look-up. If an
>> error occurs during the option parsing, then it must be an invalid
>> argument and the user should be informed of usage, but if a error
>> occurs during ref-filtering, then it is a problem with the
>> argument.

After staring the code a bit longer, I started to dislike the
approach taken by this patch quite a lot.  Isn't the problem that we
let parse-options machinery to show the usage help, which is
designed to be shown when the user does not know what options the
command supports, even when we recognised the option perfectly fine?

That is, if we added a mechanism for a callback function given to
OPT_CALLBACK to tell the calling parse-options machinery "I
recognise the option; but the value given to the option is wrong and
that is why I am returning an error", and made the caller in the
parse-options machinery to refrain from showing the usage help,
would it solve the issue with minimum fuss and stop the execution at
the very first error we detect?

Stepping back even further, I wonder if any error detected in a
custom callback handler given to OPT_CALLBACK even wants to show the
usage help.  I offhand do not think of any situation--- the callback
was called only because OPT_CALLBACK item in the options[] list
matched what the user gave us, so at that point we know the user
gave us one of the valid options.  The error message from the
callback may say "Hey I only take commit object name", or it could
(theoretically) even be "Sorry I do not take any values", but in any
case, I do not think there is a reason for a failure detected in the
callback should lead to the usage help.

So perhaps "if we added a machanism...to tell..." part in the
previous paragraph is not even needed, and the only thing we need to
do is to make the caller in parse-options that calls a custom
callback function given to OPT_CALLBACK to stop giving the usage
help.  Wouldn't that automatically fix the "branch --points-at
garbage" issue that is not addressed by this patch, too?

Reply via email to