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?
