On 13/10/13 10:12, Andrew Gregory wrote:
> On 10/13/13 at 09:55am, Allan McRae wrote:
>> On 13/10/13 03:32, Andrew Gregory wrote:
>>> On invalid combinations of flags we were only printing the unhelpfully
>>> vague message "invalid option".
>>>
>>> Signed-off-by: Andrew Gregory <[email protected]>
>>> ---
>>>  src/pacman/conf.h   | 3 ++-
>>>  src/pacman/pacman.c | 7 ++++++-
>>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/pacman/conf.h b/src/pacman/conf.h
>>> index e263d7c..45e48c0 100644
>>> --- a/src/pacman/conf.h
>>> +++ b/src/pacman/conf.h
>>> @@ -128,7 +128,8 @@ enum {
>>>  
>>>  /* Long Operations */
>>>  enum {
>>> -   OP_NOCONFIRM = 1000,
>>> +   OP_LONG_FLAG_MIN = 1000,
>>> +   OP_NOCONFIRM,
>>>     OP_CONFIG,
>>>     OP_IGNORE,
>>>     OP_DEBUG,
>>> diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
>>> index 93baa44..308ff38 100644
>>> --- a/src/pacman/pacman.c
>>> +++ b/src/pacman/pacman.c
>>> @@ -828,7 +828,12 @@ static int parseargs(int argc, char *argv[])
>>>             result = parsearg_global(opt);
>>>             if(result != 0) {
>>>                     /* global option parsing failed, abort */
>>> -                   pm_printf(ALPM_LOG_ERROR, _("invalid option\n"));
>>> +                   if(opt < OP_LONG_FLAG_MIN) {
>>> +                           pm_printf(ALPM_LOG_ERROR, _("invalid option 
>>> '-%c'\n"), opt);
>>> +                   } else {
>>> +                           pm_printf(ALPM_LOG_ERROR, _("invalid option 
>>> '--%s'\n"),
>>> +                                           opts[option_index].name);
>>> +                   }
>>>                     return result;
>>
>>
>> I spent some time trying to get to this error.  I found tow ways to get
>> there and both do not seem like this is actually the right error message:
>>
>> allan@arya ~
>>> pacman --color=foo
>> error: invalid argument 'foo' for --color
>> error: invalid option
>>
>> allan@arya ~
>>> pacman --debug=4
>> error: '4' is not a valid debug level
>> error: invalid option
>>
>> How else can we get there?
> 
> Use a valid option with an operation that doesn't understand it such
> as `pacman -T --quiet`.
> 

I like this patch, so accepted with the note that this output is bad
(but was bad in the first place...)

allan@arya /home/arch/code/pacman (working)
> ./src/pacman/pacman --color=foo
error: invalid argument 'foo' for --color
error: invalid option '--color'




Reply via email to