Re: [PATCH] Allow "-" as a short-hand for "@{-1}" in "git branch -d @{-1}"
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.
Show 22 quoted lines
> 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.
Show 5 quoted lines
> git_config(git_branch_config, NULL);
>
> track = git_branch_track;
> --
> 2.8.0.rc0
