Bikeshedding advice on the ab/i18n-scripts series

Bikeshedding advice on the ab/i18n-scripts series

[Ævar Arnfjörð Bjarmason]

On Wed, May 11, 2011 at 22:24, Junio C Hamano <gitster@pobox.com> wrote:

> Will merge to "master" by the end of week #3.
>
> * ab/i18n-scripts (2011-05-08) 48 commits
>  - i18n: git-bisect bisect_next_check "You need to" message
>  - i18n: git-bisect [Y/n] messages
>  - i18n: git-bisect bisect_replay + $1 messages

I'm now mostly done solving all the bugs noted in this series, aside
from the case insensitive env vars on win32 issue.

I know how to fix these, but some people may have issues with my style
of doing so, so I'd like to ask in advance to save me extra work.

Basically for code like this:

        while [ $# -gt 0 ]; do
            arg="$1"
            case "$arg" in
            --)
                shift
                break
                ;;
            *)
                rev=$(git rev-parse -q --verify "$arg^{commit}") || {
                    test $has_double_dash -eq 1 &&
                        die "$(eval_gettext "'\$arg' does not appear
to be a valid revision")"
                    break

I was thinking of just doing:

    WHATEVER_arg=$arg
    die "$(eval_gettext "'\$WHATEVER_arg' does not appear to be a
valid revision")"

Where WHATEVER is a sufficiently unique prefix. E.g.:

    WINDOWS_ME_HARDER
    GIT_I18N_VARIABLE
    YOUR_MOM
    DUDE_WHERES_MY_POSIX_COMPLIANCE

I'll just pick one at my discretion (not necessarily from that list)
unless someone has some strong preference. I don't care, I just don't
want to re-send it over that.

I could also change all occurances of "arg" (including arg="$1" etc.)
to WHATEVER_arg, but that seems like overkill, and would make the code
hard to read, since you'd have WHATEVER all over the place.

Oh, and for all the convertion of:

    echo >&2 "$(gettext "foobar")"

I've already done:

    (
        gettext "foobar" &&
        echo
    ) >&2

Is that OK, or does someone have a strong preference (and wants me to
waste time on converting it) for:

    gettext "foobar" >&2
    echo >&2

Or something else?

Your friendly neighborhood i18n maintainer.

Re: Bikeshedding advice on the ab/i18n-scripts series

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> Oh, and for all the convertion of:
>
>     echo >&2 "$(gettext "foobar")"
>
> I've already done:
>
>     (
>         gettext "foobar" &&
>         echo
>     ) >&2

Some day we might want to introduce a gettext_ln function or something
similar.  But that doesn't seem urgent; I wouldn't be appalled by
having to read 'gettext "foobar" && echo' in a block surrounded by
braces (or even a subshell, even though it makes shells waste a
fork()).

As for what to call the envvars, I have no strong preference there,
either.  If you don't care about compatibility with the real gettext.sh
then it should be possible to do the variable munging automatically,
using a loop:

	for var in $(git sh-i18n--envsubst --variables "$fmt")
	do
		eval "sh_i18n_$var"=\$var
		export "sh_i18n_$var"
	done

with an understanding that git sh-i18n--envsubst substitutes in
${sh_i18n_$var} where it sees '$var' in the format string.

But if we do want to use the real envsubst some day then that is not
possible.  I dunno.

Re: Bikeshedding advice on the ab/i18n-scripts series

[Junio C Hamano]

Æ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.

Re: Bikeshedding advice on the ab/i18n-scripts series

[Ævar Arnfjörð Bjarmason]

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?

Re: Bikeshedding advice on the ab/i18n-scripts series

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> 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.

Having said that, depending on how the strings are distributed, I have a
feeling that we might be better off having two variants:

	say >&2 "<message string>"
	gettext >&2 "<message string>"

The former would be

	say () {
		gettext >&2 "$1"; echo
	}

and minority of callers (they may be an empty set) that care about
trailing blank lines they output can include their own terminating LFs in
the message to be translated and call gettext directly, letting it output
the translation without stripping trailing LFs they (or their translation)
produce.

Re: Bikeshedding advice on the ab/i18n-scripts series

[Ævar Arnfjörð Bjarmason]

On Mon, May 16, 2011 at 22:58, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> 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.
>
> Having said that, depending on how the strings are distributed, I have a
> feeling that we might be better off having two variants:
>
>        say >&2 "<message string>"
>        gettext >&2 "<message string>"
>
> The former would be
>
>        say () {
>                gettext >&2 "$1"; echo
>        }
>
> and minority of callers (they may be an empty set) that care about
> trailing blank lines they output can include their own terminating LFs in
> the message to be translated and call gettext directly, letting it output
> the translation without stripping trailing LFs they (or their translation)
> produce.

That would require us to start extracting strings from all "say"
functions. The reason we have only "gettext" and "eval_gettext" is so
xgettext can extract them.

I'd like to keep that simplicity. Let's not go and wrap these
functions for trivial aesthetics at the cost of complexity.

Re: Bikeshedding advice on the ab/i18n-scripts series

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> That would require us to start extracting strings from all "say"
> functions. The reason we have only "gettext" and "eval_gettext" is so
> xgettext can extract them.

Isn't that what the xgettext -k parameter is for?

Re: Bikeshedding advice on the ab/i18n-scripts series

[Ævar Arnfjörð Bjarmason]

On Mon, May 16, 2011 at 23:26, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>
>> That would require us to start extracting strings from all "say"
>> functions. The reason we have only "gettext" and "eval_gettext" is so
>> xgettext can extract them.
>
> Isn't that what the xgettext -k parameter is for?

I'm not saying we can't use xgettext to extract these things. I'm
saying I don't think it's a good idea.

    $ git --no-pager grep say git-submodule.sh
    git-submodule.sh:                       say "$(eval_gettext
"Entering '\$prefix\$path'")"
    git-submodule.sh:               say "$(eval_gettext "Submodule
'\$name' (\$url) registered for path '\$path'")"
    git-submodule.sh:                       say "$(eval_gettext
"Submodule path '\$path' not initialized
    git-submodule.sh:
say_msg="$(eval_gettext "Submodule path '\$path': rebased into
'\$sha1'")"
    git-submodule.sh:
say_msg="$(eval_gettext "Submodule path '\$path': merged in
'\$sha1'")"
    git-submodule.sh:
say_msg="$(eval_gettext "Submodule path '\$path': checked out
'\$sha1'")"
    git-submodule.sh:                       say $say_msg
    git-submodule.sh:                       say "U$sha1 $displaypath"
    git-submodule.sh:                       say "-$sha1 $displaypath"
    git-submodule.sh:                       say " $sha1 $displaypath$revname"
    git-submodule.sh:                       say "+$sha1 $displaypath$revname"
    git-submodule.sh:               say "$(eval_gettext "Synchronizing
submodule url for '\$name'")"

I want to mark *some* of these *manually* as translatable, I don't
want to go and extract strings from all invocations to functions like
printf & die, instead I want to add a printf(_()) wrapper to
everything.

Re: Bikeshedding advice on the ab/i18n-scripts series

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Mon, May 16, 2011 at 23:26, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Ævar Arnfjörð Bjarmason wrote:
>>
>>> That would require us to start extracting strings from all "say"
>>> functions. The reason we have only "gettext" and "eval_gettext" is so
>>> xgettext can extract them.
>>
>> Isn't that what the xgettext -k parameter is for?
>
> I'm not saying we can't use xgettext to extract these things. I'm
> saying I don't think it's a good idea.
>
>     $ git --no-pager grep say git-submodule.sh
>     git-submodule.sh:                       say "$(eval_gettext
> "Entering '\$prefix\$path'")"

Oh, I didn't mean to say "we _must_ call that _say_".

The discussion suggested to me that we may want two variants, one that
takes an incomplete single line and outputs a single complete line, and
the other that takes whatever the caller gives and outputs the translated
lines without further butchering the tail end.  I missed that Jonathan
already mentioned gettext_nl and that is fine as well, but if we use a
function other than gettext I thought it would be better to use short and
sweet name for common cases.