From: Bert Wesarg <bert.wesarg@googlemail.com>
Date: Tue, 12 Aug 2008 05:14:51 GMT
Subject: Re: [TopGit PATCH v2] tg-create.sh: Support for multiple {to,cc,bcc} options
Message-ID: <36ca99e90808112214hb8f01f0j30cff74e4f6c1ead@mail.gmail.com>
In-Reply-To: <20080811204723.GF10151@machine.or.cz>

On Mon, Aug 11, 2008 at 22:47, Petr Baudis <pasky@suse.cz> wrote:
>  Hi,
>
> On Sat, Aug 09, 2008 at 08:48:56PM +0200, Bert Wesarg wrote:
>> Git config supports multiple values for the same config key, so support it
>> for these TopGit config options, too.
>>
>> New in v2:
>> Print a RFC2822 compliant header.
>>
>> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>
>  oops, I'm really sorry! I thought I already commented on this while
> apparently, I forgot to.
>
>> ---
>>  tg-create.sh |   35 ++++++++++++++++++++++++++++++++---
>>  1 files changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/tg-create.sh b/tg-create.sh
>> index 6cce7ed..d7ee1d2 100644
>> --- a/tg-create.sh
>> +++ b/tg-create.sh
>> @@ -100,13 +100,42 @@ git checkout -b "$name"
>>  echo "$deps" | sed 's/ /\n/g' >"$root_dir/.topdeps"
>>  git add "$root_dir/.topdeps"
>>
>> +# Print a RFC2822 compliant header ($2) with values from the config option
>> +# ($1 without the topgit. prefix)
>> +get_multi_config()
>> +{
>> +     # Do we need to escape it for awk double quotes?
>> +     prefix="$2"
>> +     prefix_align="$(printf "%*s  " "${#2}" "")"
>> +
>> +     git config --get-all topgit.$1 |
>> +             awk '
>> +                     BEGIN {
>> +                             line = ""
>> +                             prefix = "'"$prefix"': "
>> +                     }
>> +                             {
>> +                                     if (line != "") {
>> +                                             print prefix line ","
>> +                                             prefix = "'"$prefix_align"'"
>> +                                     }
>> +                                     line = $0
>> +                             }
>> +                     END {
>> +                             if (line != "") {
>> +                                     print prefix line
>> +                             }
>> +                     }
>> +             '
>> +}
>> +
>
> I'm not too happy about this, for several reasons:
>
>        (i) This code is so awfully complicated.
For my first awk script, its very clean and not that complicated. I think ;-)

>
>        (ii) It would be simpler to just prefix all the further lines
> with a tab; wouldn't something like
>
>                sed '2,$s/^/\t/'
No objections with this.

>
> actually work?
>
>        (iii) This is troublesome because now header values can span
> multiple lines. Until now, we were just blisfully ignorant about this
> possibility. At least tg export needs to be adjusted to account for this
> now, and I fear dealing with this will be pretty annoying when
> prototyping new features.
Than we should go back to v1. As I asked in a previous line: "But do
we generate a valid mail with tg patch, or just a patch file with some
special looking lines?"

Bert
> --
>                                Petr "Pasky" Baudis