Bikeshedding advice on the ab/i18n-scripts series

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

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.

Ævar Arnfjörð Bjarmason wrote:

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.

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

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.

On Mon, May 16, 2011 at 22:45, Junio C Hamano <gitster@pobox.com> wrote:

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?

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

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.

On Mon, May 16, 2011 at 22:58, Junio C Hamano <gitster@pobox.com> 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.

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

Ævar Arnfjörð Bjarmason wrote:

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

On Mon, May 16, 2011 at 23:26, Jonathan Nieder <jrnieder@gmail.com> wrote:

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.

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

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.