[PATCH] Allow "-" as a short-hand for "@{-1}" in "git branch -d @{-1}"

[PATCH] Allow "-" as a short-hand for "@{-1}" in "git branch -d @{-1}"

[Dinesh Polathula]

From: Dinesh <dpdineshp2@gmail.com>

This patch allows the usage of "-" as a short-hand for "@{-1}" in 
"git branch -d  <at> {-1}".

Note : This is a microproject that is part of the Google Summer of 
Code application process.
I am interested in working on the git Beginner mode implementation as 
part of Google Summer of Code. The mentor details for this particular 
project are not available on the Ideas page. The mentors are likely on 
this mailing list, so I request the mentors to drop me a mail so I can 
get in contact with you to further discuss the git Beginner mode project.

Dinesh (1):
  Allow "-" as a short-hand for "@{-1}" in "git branch -d @{-1}"

 builtin/branch.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

-- 
2.8.0.rc0

[PATCH] Allow "-" as a short-hand for "@{-1}" in "git branch -d @{-1}"

[Dinesh Polathula]

From: Dinesh <dpdineshp2@gmail.com>

The "-" shorthand can be used as a replacement for "@{-1}" to refer
to the previous branch the user was on in the "git branch -d @{-1}"
command.
Replace "-" argument with "@{-1}" when the command line arguments
are parsed.

Signed-off-by: Dinesh Polathula<dpdineshp2@gmail.com>
---
 builtin/branch.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 7b45b6b..98d2c4b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -24,7 +24,7 @@
 static const char * const builtin_branch_usage[] = {
 	N_("git branch [<options>] [-r | -a] [--merged | --no-merged]"),
 	N_("git branch [<options>] [-l] [-f] <branch-name> [<start-point>]"),
-	N_("git branch [<options>] [-r] (-d | -D) <branch-name>..."),
+	N_("git branch [<options>] [-r] (-d | -D) [-] <branch-name>..."),
 	N_("git branch [<options>] (-m | -M) [<old-branch>] <new-branch>"),
 	N_("git branch [<options>] [-r | -a] [--points-at]"),
 	NULL
@@ -658,8 +658,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	filter.abbrev = -1;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage_with_options(builtin_branch_usage, options);
-
+	{
+		usage_with_options(builtin_branch_usage, options);	
+	}
+	if (argc == 3 && !strcmp(argv[2], "-"))
+	{
+	    argv[2] = "@{-1}";	
+	}
 	git_config(git_branch_config, NULL);
 
 	track = git_branch_track;
-- 
2.8.0.rc0

Re: [PATCH] Allow "-" as a short-hand for "@{-1}" in "git branch -d @{-1}"

[Eric Sunshine]

On Sun, Mar 6, 2016 at 7:48 AM, Dinesh Polathula <dpdineshp2@gmail.com> wrote:
> From: Dinesh <dpdineshp2@gmail.com>

You can drop this line and let git-am pick up your name and address
from the email envelope.

> Allow "-" as a short-hand for "@{-1}" in "git branch -d @{-1}"

    branch: -d/-D: recognize "-" as short-hand for @{-1}

> The "-" shorthand can be used as a replacement for "@{-1}" to refer
> to the previous branch the user was on in the "git branch -d @{-1}"
> command.

Does/should this also apply to -D?

> Replace "-" argument with "@{-1}" when the command line arguments
> are parsed.

This final sentence isn't really needed, as it's just repeating what
the patch itself already says.

> Signed-off-by: Dinesh Polathula<dpdineshp2@gmail.com>
> ---
>  builtin/branch.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)

Does this need a documentation update? Is "-" documented for other
commands which recognize it specially or is knowledge of "-" implicit?

This change probably does deserve a new test or two.

> diff --git a/builtin/branch.c b/builtin/branch.c
> index 7b45b6b..98d2c4b 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -24,7 +24,7 @@
>  static const char * const builtin_branch_usage[] = {
>         N_("git branch [<options>] [-r | -a] [--merged | --no-merged]"),
>         N_("git branch [<options>] [-l] [-f] <branch-name> [<start-point>]"),
> -       N_("git branch [<options>] [-r] (-d | -D) <branch-name>..."),
> +       N_("git branch [<options>] [-r] (-d | -D) [-] <branch-name>..."),
>         N_("git branch [<options>] (-m | -M) [<old-branch>] <new-branch>"),
>         N_("git branch [<options>] [-r | -a] [--points-at]"),
>         NULL
> @@ -658,8 +658,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>         filter.abbrev = -1;
>
>         if (argc == 2 && !strcmp(argv[1], "-h"))
> -               usage_with_options(builtin_branch_usage, options);
> -
> +       {
> +               usage_with_options(builtin_branch_usage, options);
> +       }

Why add unnecessary braces (making the diff noisier, as well)?

> +       if (argc == 3 && !strcmp(argv[2], "-"))

The commit message talks about this applying only to -d (and
presumably -D), however, there doesn't seem to be any constraint
enforcing that.

Won't this logic fail if the user passes other options accepted by
git-branch, such as --quite (for instance, "git branch --quiet -d -")?
How do other Git commands which recognize "-" as alias for @{-1} deal
with this? Have you checked their implementations?

To address these issues, it seems like a more correct place to
recognize "-" as an alias would be somewhere within
builtin/branch.d:delete_branches().

> +       {
> +           argv[2] = "@{-1}";
> +       }

Style: Unnecessary braces.

>         git_config(git_branch_config, NULL);
>
>         track = git_branch_track;
> --
> 2.8.0.rc0

Re: [PATCH] Allow "-" as a short-hand for "@{-1}" in "git branch -d @{-1}"

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> +       if (argc == 3 && !strcmp(argv[2], "-"))
> ...
> To address these issues, it seems like a more correct place to
> recognize "-" as an alias would be somewhere within
> builtin/branch.d:delete_branches().

I agree with all your review comments, and the above would also
cover another issue you did not mention above: "git branch -d/-D"
actually takes one or more branches to delete.

Another issue to consider is if we get a sensible error message.

What should the error message say if a user did these in a freshly
created empty repository?

    $ git commit --allow-empty -m initial
    $ git commit --allow-empty -m second
    $ git branch side master^
    $ git branch -d -

Currently, you would get

    $ git branch -d -
    error: branch '-' not found.

Is it OK to say

    error: branch '@{-1}' not found.

when the user never said '@{-1}' from the command line?

I am not saying it _is_ a problem--it may very well be acceptable,
as long as the users understand that '-' is merely a short-hand for
'@{-1}'.  Either way, the log message needs to have some evidence
that the patch author thought about the issue, the resolution the
patch author chose for the issue, and the reason why a particular
resolution was chosen.

Thanks.