On Wed, Apr 21, 2021 at 8:55 PM Josh Soref via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> Usually, it is easier to read a message if it makes its primary
> point first, before giving a parenthetical note.
> [...]
> Signed-off-by: Josh Soref <jsoref@xxxxxxxxx>
> ---
> Changes since v2:
>
> * finish_up_to_date now figures out the answer on its own to address
> feedback from Eric Sunshine
>
> -- Yes, I'm well aware of localization rules. But as Eric Sunshine
> noted, the code was already making a mess of things. I didn't want to
> make invasive changes. I actually wrote roughly what Eric proposed as an
> earlier implementation, but the various complexities, including trying
> to figure out what the yeah was for and who cared about it, made me
> really wary of proposing such a significant change.
Understandable. I also was curious as to whether "Yeeah!" had any
significance, thus checked the project history before responding to
your v2. As far as I can tell, "Yeeah!" has no particular
significance. It materialized out of thin air with 1c7b76be7d (Build
in merge, 2008-07-07) and simply hasn't been touched since then (in
any meaningful way). Delving into the list archive doesn't shed any
additional light on "Yeeah!"; none of the reviews mentioned it at all.
So, it doesn't seem to serve any genuine purpose, and I don't mind
seeing it go away, especially since its removal simplifies things for
translators.
> diff --git a/builtin/merge.c b/builtin/merge.c
> @@ -380,10 +380,14 @@ static void restore_state(const struct object_id *head,
> /* This is called when no merge was necessary. */
> -static void finish_up_to_date(const char *msg)
> +static void finish_up_to_date(void)
> {
> - if (verbosity >= 0)
> - printf("%s%s\n", squash ? _(" (nothing to squash)") : "", msg);
> + if (verbosity >= 0) {
> + if (squash)
> + puts(_("Already up to date (nothing to squash)."));
> + else
> + puts(_("Already up to date."));
> + }
> remove_merge_branch_state(the_repository);
> }
> @@ -1482,7 +1486,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> - finish_up_to_date(_("Already up to date."));
> + finish_up_to_date();
> @@ -1566,7 +1570,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> - finish_up_to_date(_("Already up to date. Yeeah!"));
> + finish_up_to_date();
Perhaps not surprisingly, I find this version of the patch much easier
to understand.
Thanks for re-rolling.
