Re: [v2 PATCH 1/2] reset: add '-' shorthand for '@{-1}'

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 10, 2015, 17:25 UTC
Message-ID: <CAPig+cS9t6gWdf+2A1MX7tfkS_Eb+MAdNn_Zgo6+oG4PCjP77w@mail.gmail.com>
In-Reply-To: <1426001883-6423-1-git-send-email-dyoucme@gmail.com>

On Tue, Mar 10, 2015 at 11:38 AM, Sundararajan R <dyoucme@gmail.com> wrote:

> Teaching reset the - shorthand involves checking if any file named '-' exists
> because it then becomes ambiguous as to whether the user wants to reset the
> file '-' or if he wants to reset the working tree to the previous branch.

For clarity, I'd probably mention that the ambiguity arises only in the absence of explicit '--' disambiguation.

> check_filename() is used to perform this check. A similar ambiguity occurs
> when the file @{-1} exits. Therefore, when the files '-' or '@{-1}' exist
> then the program dies with a message about the ambiguous argument.

Why single out @{-1} as a potential file name? Has @{-1} ever been considered a filename rather than a treeish? Is this patch changing the treatment of @{-1} so that it might be interpreted as a filename?

Show 52 quoted lines

> Helped-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Sundararajan R <dyoucme@gmail.com>
> ---
> Have made the modifications suggest by you, Eric.
> Removed the part where the user is told that he can use ./- instead.
>
>  builtin/reset.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 4c08ddc..88ce0c5 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -192,6 +192,7 @@ static void parse_args(struct pathspec *pathspec,
>  {
>         const char *rev = "HEAD";
>         unsigned char unused[20];
> +       int file_named_minus = 0;
>         /*
>          * Possible arguments are:
>          *
> @@ -205,6 +206,12 @@ static void parse_args(struct pathspec *pathspec,
>          */
>
>         if (argv[0]) {
> +               if (!strcmp(argv[0], "-") && !argv[1]) {
> +                       if (!check_filename(prefix, "-"))
> +                               argv[0] = "@{-1}";
> +                       else
> +                               file_named_minus = 1;
> +               }
>                 if (!strcmp(argv[0], "--")) {
>                         argv++; /* reset to HEAD, possibly with paths */
>                 } else if (argv[1] && !strcmp(argv[1], "--")) {
> @@ -226,7 +233,13 @@ static void parse_args(struct pathspec *pathspec,
>                         rev = *argv++;
>                 } else {
>                         /* Otherwise we treat this as a filename */
> -                       verify_filename(prefix, argv[0], 1);
> +                       if (file_named_minus) {
> +                               die(_("ambiguous argument '-': both revision and filename\n"
> +                                       "Use '--' to separate paths from revisions, like this:\n"
> +                                       "'git <command> [<revision>...] -- [<file>...]'"));
> +                       }
> +                       else
> +                               verify_filename(prefix, argv[0], 1);
>                 }
>         }
>         *rev_ret = rev;
> --
> 2.1.0