From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mon, 16 May 2011 20:53:44 GMT
Subject: Re: Bikeshedding advice on the ab/i18n-scripts series
Message-ID: <BANLkTimtofA7xVBW-W+20zqUt98GLSPHdw@mail.gmail.com>
In-Reply-To: <7vboz2l6h1.fsf@alter.siamese.dyndns.org>

On Mon, May 16, 2011 at 22:45, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> Oh, and for all the convertion of:
>>
>>     echo >&2 "$(gettext "foobar")"
>>
>> I've already done:
>>
>>     (
>>         gettext "foobar" &&
>>         echo
>>     ) >&2
>
> Sorry, but what problem are you trying to solve?  The output from
>
>    $ gettext "foobar"
>
> may have 0, 1 or more LF at the end, but wouldn't
>
>    echo >&2 "$(gettext "foobar")"
>
> terminate with a single LF in any case?
>
> Ahh, perhaps I was the one who said something stupid like:
>
>        echo >&2 "$(cmd)"
>
> should be equivalent to
>
>        cmd >&2
>
> which is not the case when output from cmd does not end with a single LF
> (i.e. either an incomplete line, or with trailing blank lines).
>
> Sorry, if that is what you are trying to address, please let me take that
> back.

Yeah, you said:

    >> +  echo >&2 "$(eval_gettext "Warning: fetch updated the current
branch head.
    >> +Warning: fast-forwarding your working tree from
    >> +Warning: commit \$orig_head.")"
    >
    > echo "$(...)"
    >
    > sounds quite wasteful and harder to read than necessary.
    > The same also happens in later patches.

    A very good point.  Just letting eval_gettext write out to >&2 should be
    both sufficient and inifinitely more readable.

    -- http://www.spinics.net/lists/git/msg157376.html

Which is why I rewrote all the relevant patches to look like:

    diff --git a/git-pull.sh b/git-pull.sh
    index 06dcd81..a10b129 100755
    --- a/git-pull.sh
    +++ b/git-pull.sh
    @@ -217,9 +217,12 @@ then
            # $orig_head commit, but we are merging into $curr_head.
            # First update the working tree to match $curr_head.

    -       echo >&2 "Warning: fetch updated the current branch head."
    -       echo >&2 "Warning: fast-forwarding your working tree from"
    -       echo >&2 "Warning: commit $orig_head."
    +       (
    +               eval_gettext "Warning: fetch updated the current
branch head.
    +Warning: fast-forwarding your working tree from
    +Warning: commit \$orig_head." &&
    +               echo
    +       ) >&2
            git update-index -q --refresh
            git read-tree -u -m "$orig_head" "$curr_head" ||
                    die "$(eval_gettext "Cannot fast-forward your working tree.

I wrote them using echo "$(gettext "foo")" initially because I thought
bending over backwards like this was silly.

You want me to go and change it back now?