On do, 2016-09-01 at 17:17 +0200, Johannes Schindelin wrote:
> 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?
Kinda makes sense. It's not how I use fixup/squash as a user of rebase
-i though. But we can't go there, that's bikeshed country :)
> > > +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.
Hmm, not sure if I agree with that. I'd see it as a regression to lose
the i18n there.
D.