Re: [PATCH] "git branch -M @{-1}"

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Earlier, we've taught "git branch -d <name>" that <name> is supposed to be
> a branch name and not a random extended SHA-1.  "git branch -m <name> [<name>]" 
> needs to be taught the same.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin-branch.c |   14 ++++++++++----
>  1 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/builtin-branch.c b/builtin-branch.c
> index c34af27..3f97838 100644
> --- a/builtin-branch.c
> +++ b/builtin-branch.c
> @@ -425,21 +425,27 @@ static void rename_branch(const char *oldname, const char *newname, int force)
>  	if (!oldname)
>  		die("cannot rename the current branch while not on any.");
>  
> -	strbuf_addf(&oldref, "refs/heads/%s", oldname);
> +	if (interpret_nth_last_branch(oldname, &oldref) == strlen(oldname))
> +		strbuf_splice(&oldref, 0, 0, "refs/heads/", 11);
> +	else
> +		strbuf_addf(&oldref, "refs/heads/%s", oldname);

A few comments on this patch.

 (1) I am actually very reluctant to queue this, or any unproven "fix"
     that adds obscure codepath, for 1.6.2, this close to the final.  Most
     likely I won't be queueing any more @{-n] fixes until 1.6.2 final and
     defer them to 1.6.2.X series if necessary.

 (2) If we are to have full @{-n} support, and if we do not mind small
     leaks, it would make more sense to introduce a leaky helper function
     like this and use it everywhere:

	const char *handle_branch_arg(const char *name)
        {
        	struct strbuf buf = STRBUF_INIT;
		size_t len = strlen(name);
                if (interpret_nth_last_branch(name, &buf) == len)
			return strbuf_detach(&buf);
		return name;
	}

 (3) I think this "The argument could be previous branch name" conversion
     should be done only when the command is about local branch names
     (i.e. when run without -r option), and the above codepath needs to be
     protected with if (kinds == REF_LOCAL_BRANCH)