On Thu, Nov 2, 2017 at 2:54 AM, Kaartic Sivaraam
<kaartic.sivaraam@xxxxxxxxx> wrote:
> From: Kaartic Sivaraam <kaarticsivaraam91196@xxxxxxxxx>
>
> When trying to rename an inexistent branch to with a name of a branch
> that already exists the rename failed specifying the new branch name
> exists rather than specifying that the branch trying to be renamed
> doesn't exist.
>
> $ git branch -m tset master
> fatal: A branch named 'master' already exists.
>
> It's conventional to report that 'tset' doesn't exist rather than
> reporting that 'master' exists, the same way the 'mv' command does.
>
> (hypothetical)
> $ git branch -m tset master
> fatal: branch 'tset' doesn't exist.
>
> That has the problem that the error about an existing branch is shown
> only after the user corrects the error about inexistent branch.
>
> $ git branch -m test master
> fatal: A branch named 'master' already exists.
>
> This isn't useful either because the user would have corrected this error in
> a single go if he had been told this alongside the first error. So, give
> more useful error messages by giving errors about old branch name and new
> branch name at the same time. This is possible as the branch name validation
> functions now return the reason they were about to die, when requested.
>
> $ git branch -m tset master
> fatal: branch 'tset' doesn't exist, and branch 'master' already exists
Nicely explained; easily understood.
> Note: Thanks to the strbuf API that made it possible to easily construct
> the composite error message strings!
This may be a problem. See below...
> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@xxxxxxxxx>
> ---
> diff --git a/builtin/branch.c b/builtin/branch.c
> +static void get_error_msg(struct strbuf* error_msg, const char* oldname, unsigned old_branch_exists,
> + const char* newname, enum branch_validation_result res)
> +{
> + const char* connector_string = _(", and ");
> +
> + if (!old_branch_exists) {
> + strbuf_addf(error_msg, _("branch '%s' doesn't exist"), oldname);
> + }
> +
> + switch (res) {
> + case BRANCH_EXISTS_NO_FORCE:
> + strbuf_addf(error_msg, "%s", (!old_branch_exists) ? connector_string : "");
> + strbuf_addf(error_msg,_("branch '%s' already exists"), newname);
> + break;
> + case CANNOT_FORCE_UPDATE_CURRENT_BRANCH:
> + strbuf_addf(error_msg, "%s", (!old_branch_exists) ? connector_string : "");
> + strbuf_addstr(error_msg, _("cannot force update the current branch"));
> + break;
> + case INVALID_BRANCH_NAME:
> + strbuf_addf(error_msg, "%s", (!old_branch_exists) ? connector_string : "");
> + strbuf_addf(error_msg, _("branch name '%s' is invalid"), newname);
> + break;
> + /* not necessary to handle success cases */
> + case BRANCH_EXISTS:
> + case BRANCH_DOESNT_EXIST:
> + break;
> + }
> +}
Translators can correct me, but this smells like "sentence lego"[1],
which we'd like to avoid. Translators lack full context when presented
with bits and pieces of a sentence like this, thus the translation may
be of poor quality; it may even be entirely incorrect since they don't
necessarily know how you will be composing the various pieces.
You _might_ be able to able to resolve this by dropping "and" from:
"foo is moo, and bar is boo"
to turn the error messages into independent clauses:
"foo is moo; bar is boo"
but I'm no translator, and even that may fail the lego sniff test.
A sure-fire way to avoid lego construction would be simply to emit
each messages on its own line:
fatal: branch 'tset' doesn't exist
fatal: branch 'master' already exists
(though, you might consider that too ugly).
[1]: http://www.gnu.org/software/gettext/manual/gettext.html#Preparing-Strings
