Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

From: Jacob Keller <jacob.keller@gmail.com>
Date: Nov 17, 2015, 16:27 UTC
Message-ID: <CA+P7+xp5x+WhQ418aDEHdVv=GWvEpzsvOGbKpNMZ8ko5xcFr8w@mail.gmail.com>
In-Reply-To: <20151117132644.Horde.0sLxtm6pFWvFfyP7Yzukk0F@webmail.informatik.kit.edu>

On Tue, Nov 17, 2015 at 4:26 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:

Show 18 quoted lines

>
> Quoting Eric Sunshine <sunshine@sunshineco.com>:
>
>> On Tue, Nov 17, 2015 at 12:10:35AM +0000, Keller, Jacob E wrote:
>>>
>>> On Mon, 2015-11-16 at 18:50 -0500, Eric Sunshine wrote:
>>> > It should be possible to extract the alias within the shell itself
>>> > without a separate process. For instance:
>>> >
>>> >     read alias rest
>>> >
>>> > will leave the first token in $alias and the remainder of the line in
>>> > $rest, and it's all done within the shell process.
>
>
> Actually, putting this read in a while loop and feeding 'git send-email's
> output into that does fork() a subshell:
>

Yea, I realized this. One extra subshell isn't a big deal but if we can eliminate that it is good.

Show 18 quoted lines

>   $ echo "outside: $BASH_SUBSHELL" |while read line ; do echo "$line
> inside: $BASH_SUBSHELL" ; done
>   outside: 0  inside: 1
>
>>> I'll look into this :)
>>
>>
>> My reason for asking is concern about scripts possibly breaking if
>> someone comes along and wants to "fix" --dump-aliases to also dump
>> the alias expansions. One possibility is just to punt today and say
>> that when that feature is needed in the future, then that someone can
>> add a --verbose option to complement --dump-aliases which would emit
>> the alias expansions as well. One nice thing about punting at this
>> point is that we don't (today) have to define a format for the output
>> of the expansions.
>
>
> I think we should cross the bridge when we get to it.

Agreed.

Show 6 quoted lines

>
> However, we could still be nice to that brave soul who might want to cross
> it in the future, and since at this point we are interested in listing only
> alias names, perhaps we should not appropriate the broader '--dump-alias'
> option, but go with the more specific '--dump-alias-names' instead.
>

Disagree. I think the best approach for expanding --dump-aliases output is turning it into a keyword-option argument such as:

--dump-aliases=format

and we can say "historic reasons the format is always names only", then we can extend the support that way. I don't really think that "--dump-alias-names" is nice since then we'd need separate option if this ever does change, and it's rather long if it never changes.

I suspect it won't be something people need either.

Regards, Jake

>
> Gábor
>