From: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Date: Thu, 01 Sep 2016 08:33:28 GMT
Subject: Re: [PATCH 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
Message-ID: <1472718808.4680.19.camel@kaarsemaker.net>
In-Reply-To: <5488a031ffe14373b7434d497b7fd2f2e5fe55bd.1472633606.git.johannes.schindelin@gmx.de>

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?

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

D.