Re: [PATCH] config: Use parseopt.

[Felipe Contreras <felipe.contreras@gmail.com>]

On Sat, Feb 14, 2009 at 11:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>>> +    OPT_STRING(0, "get-color", &get_color_name, "name", "find the color configured: [default]"),
>>
>> get-color is used to get the defined color for a given slot.  Please do not
>> rename it to "name" (see the original).
>>
>>> +    OPT_STRING(0, "get-colorbool", &get_colorbool_name, "name", "find the color setting: [stdout-is-tty]"),
>>
>> get-colorbool is used to get the boolean setting for a given configuration
>> variable.  Please do not rename it to "name" (see the original).
>>
>
> By the way, I think it might be more appropriate if you categorize the
> above two (especially the "colorbool" one) in the "Types" section, as that
> really is what is happening.  Instead of doing the usual "print the value
> as a string", it does something magical.

All right.

I haven't used most of the config options, so I had to learn and try
them; color/colorbool are the ones I still don't get.

>> Overall, with the same number of lines, we lost a lot of error checking in
>> exchange for an ability to say "git config --remove-sec" instead of "git
>> config --remove-section", and "git config --help" may give an easier to
>> read message.
>
> And I forgot to mention the "option categorization" merit.
>
>> Given that "git config" is primarily meant for script use, this particular
>> round does not look like a good tradeoff to me.
>
> Don't take this too negatively.  The tradeoff will improve if there aren't
> these obvious bugs you can spot without even running the code.

No problem, I probably should have explained that this was RFC, I knew
there were issues in the code.

-- 
Felipe Contreras