Hi Dennis,
On Thu, 1 Sep 2016, Dennis Kaarsemaker wrote:
> On wo, 2016-08-31 at 10:54 +0200, Johannes Schindelin wrote:
>
> > +static int is_fixup(enum todo_command command)
> > +{
> > + return command == TODO_FIXUP || command == TODO_SQUASH;
> > +}
>
> It sounds wrong to have a function named is_fixup return true when the
> command isn't a fixup but a squash. Maybe name it
> changes_previous_commit or something?
I can see how that may sound confusing, unless you understand that a
squash is a fixup that lets the user edit the commit message, too. So
essentially squash = fixup + edit, if you will.
Maybe the name is more appropriate in that light?
> > +static const char *nth_for_number(int n)
> > +{
> > + int n1 = n % 10, n10 = n % 100;
> > +
> > + if (n1 == 1 && n10 != 11)
> > + return "st";
> > + if (n1 == 2 && n10 != 12)
> > + return "nd";
> > + if (n1 == 3 && n10 != 13)
> > + return "rd";
> > + return "th";
> > +}
>
> >8---
>
> > + if (command == TODO_SQUASH) {
> > + unlink(rebase_path_fixup_msg());
> > + strbuf_addf(&buf, "\n%c This is the %d%s commit message:\n\n%s",
> > + comment_line_char,
> > + count, nth_for_number(count), body);
> > + }
> > + else if (command == TODO_FIXUP) {
> > + strbuf_addf(&buf,
> > + "\n%c The %d%s commit message will be skipped:\n\n",
> > + comment_line_char, count, nth_for_number(count));
> > + strbuf_add_commented_lines(&buf, body, strlen(body));
> > + }
>
> This way of handling numbers is not translatable, and I really think we
> should mark these strings for translation, like they are in the .sh
> version.
Ah, this is the risk of working on something as big as rebase--helper.
Back when I started with it, the relevant code in git-rebase--interactive
read like this:
nth_string () {
case "$1" in
*1[0-9]|*[04-9]) echo "$1"th;;
*1) echo "$1"st;;
*2) echo "$1"nd;;
*3) echo "$1"rd;;
esac
}
I merely did a faithful translation of that...
Now, I see that git-rebase--interactive was switched to use eval_gettext,
which in turn is handled in git-sh-i18n whose code is quite convoluted. In
the absence of gettext, it uses git-sh-i18n--envsubst, which has no C API
whatsoever.
And I see that the beautiful ordinal computation was given up in favor of
a lousy "#1", "#2", "#3", etc (it used to be "1st", "2nd", "3rd" etc).
In any case, translation is not my main concern until v2.10.0, so I'll
take care of this after that release.
Ciao,
Dscho