From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sun, 03 Apr 2011 15:25:13 GMT
Subject: Re: [PATCH/RFC 1/2] git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext()
Message-ID: <BANLkTikf0s=tyGSnmgf1zmRAe3WnzB4JUA@mail.gmail.com>
In-Reply-To: <7vpqq99i11.fsf@alter.siamese.dyndns.org>

On Thu, Mar 3, 2011 at 01:14, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> In a previous incarnation of the gettext series I implemented the
>> eval_gettext() fallback like this:
>>
>>     eval_gettext() {
>>         gettext_out=$(gettext "$1")
>>         gettext_eval="printf '%s' \"$gettext_out\""
>>         printf "%s" "`eval \"$gettext_eval\"`"
>>     }
>>
>> This was clever, but would incorrectly handle cases where the variable
>> being interpolated contained spaces. E.g.:
>>
>>     cmd="git foo"; eval_gettext "command: \$cmd"
>>
>> Would emit "command: gitfoo", instead of the correct "command: git
>> foo".
>
> Hmm, are you sure you got your quoting right?
>
>    $ cat >1.sh <<\EOF
>    #!/bin/sh
>    gettext () {
>            case "$1" in
>            'command: $cmd') echo 'dcomman: $cmd' ;;
>            *) echo "GETTEXT POISON" ;;
>            esac
>    }
>
>    eval_gettext() {
>            gettext_out=$(gettext "$1") &&
>            gettext_eval="printf '%s' \"$gettext_out\"" &&
>            gettext_cmd=$(eval "$gettext_eval") &&
>            printf "%s" "$gettext_cmd"
>    }
>
>    cmd="git foo"
>    eval_gettext "command: \$cmd"
>    EOF
>    $ sh -x 1.sh
>    + cmd='git foo'
>    + eval_gettext 'command: $cmd'
>    ++ gettext 'command: $cmd'
>    ++ case "$1" in
>    ++ echo 'dcomman: $cmd'
>    + gettext_out='dcomman: $cmd'
>    + gettext_eval='printf '\''%s'\'' "dcomman: $cmd"'
>    ++ eval 'printf '\''%s'\'' "dcomman: $cmd"'
>    +++ printf %s 'dcomman: git foo'
>    + gettext_cmd='dcomman: git foo'
>    + printf %s 'dcomman: git foo'
>    dcomman: git foo
>
> Am I grossly missing something from what you are trying to do here?

(CC-ing the list again, so there's a record of this in the mail
archive).

Your eval_gettext() is better, but it still fails in cases where the
string contains "'s. E.g.:

    test_expect_success C_LOCALE_OUTPUT 'eval_gettext: our
eval_gettext() fallback can interpolate variables with spaces in them'
'
        cmdline="git am" &&
        export cmdline;
        printf "When you have resolved this problem run \"git am
--resolved\"." >expect &&
        eval_gettext "When you have resolved this problem run
\"\$cmdline --resolved\"." >actual
        test_cmp expect actual
    '

If this were just:

    test_expect_success C_LOCALE_OUTPUT 'eval_gettext: our
eval_gettext() fallback can interpolate variables with spaces in them'
'
        cmdline="git am" &&
        export cmdline;
        printf "When you have resolved this problem run git am
--resolved." >expect &&
        eval_gettext "When you have resolved this problem run
\$cmdline --resolved." >actual
        test_cmp expect actual
    '

Your version would work.

Maybe there's some portable version of doing this that works with
variables with spaces mixed with quotes. But unless someone can point
it out I'll submit a series with git-sh-i18n--envsubst and tests
demonstrating why it's needed soon.q