Re: [RFC/PATCH] reset: accept "git reset <removed file>"
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 18, 2010, 23:56 UTC
- Message-ID
- <7viq0z2gxj.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20101018224840.GA9729@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
Show 18 quoted lines
> Suppose I try to use "git reset" to un-add an new, unwanted file: > > echo hello >foo.c > git add foo.c > rm foo.c; # bad file! bad! > git reset foo.c > > The file foo.c does not exist on disk, so "git reset" rejects the > request with > > fatal: ambiguous argument 'foo.c': unknown revision or path not in the working tree. > Use '--' to separate paths from revisions > > Git can do better: since foo.c is not a revision and has an entry in > the index, it is clear the request refers to a path and not a rev. > > Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> > ---
This changes the definition of path/rev disambiguation only for "reset" and makes things inconsistent. Is it a good thing?
If a token is not a filename in the working tree, but is a path in the index, and at the same time is a valid ref, wouldn't it make the token ambiguous with the updated definition of disambiguation code here?
Show 18 quoted lines
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 0037be4..7d23d75 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -295,7 +295,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
> rev = argv[i++];
> } else {
> /* Otherwise we treat this as a filename */
> - verify_filename(prefix, argv[i]);
> + const char *name = argv[i];
> + if (prefix)
> + name = prefix_filename(prefix, strlen(prefix), name);
> + if (read_cache() < 0)
> + die("Could not read index");
> + if (cache_name_pos(name, strlen(name)) < 0)
> + verify_filename(prefix, argv[i]);
> }
> }Makes me wonder
- if we can/want to have a logic like this inside verify_filename();
- if we need a corresponding logic in either the previous else/if cascade that calls verify_non_filename(), or in verify_non_filename() itself.