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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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@xxxxxxxxx>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]