Re: [PATCH v3 2/2] mergetools: Simplify how we handle "vim" and "defaults"

From: David Aguilar <davvid@gmail.com>
Date: Jan 27, 2013, 03:56 UTC
Message-ID: <CAJDDKr7E1NkMV0_G+6oY9O3iCS9OCEzR1HYssKpwh77swDUcig@mail.gmail.com>
In-Reply-To: <7vobgbz58a.fsf@alter.siamese.dyndns.org>

On Sat, Jan 26, 2013 at 7:15 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 22 quoted lines

> David Aguilar <davvid@gmail.com> writes:
>
>> @@ -44,19 +46,9 @@ valid_tool () {
>>  }
>>
>>  setup_tool () {
>> -     case "$1" in
>> -     vim*|gvim*)
>> -             tool=vim
>> -             ;;
>> -     *)
>> -             tool="$1"
>> -             ;;
>> -     esac
>
> This part was an eyesore every time I looked at mergetools scripts.
> Good riddance.
>
> Is there still other special case like this, or was this the last
> one?
>
> Thanks, both of you, for working on this.

I believe that was the last special case. :-) It should be easier to auto-generate a list of tools for use in the documentation now. That'll be the the next topic I look into.

I have a question. John mentioned that we can replace the use of "$(..)" with $(..) in shell.

I have a trivial style patches to replace "$(..)" with $(..) sitting uncommitted in my tree for mergetool--lib.

Grepping the rest of the tree shows that there are many occurrences of the "$(..)" idiom in the shell code.

Is this a general rule that should be in CodingStyle, or is it better left as-is? Are there cases where DQ should be used around $(..)? My understanding is "no", but I don't know whether that is correct.

Doing the documentation stuff is more important IMO so I probably shouldn't let myself get too distracted by it, though I am curious.

-- 
David