Re: [RESEND PATCH] git-checkout: fix argument parsing to detect ambiguous arguments.

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

 



On Wed, Jul 23, 2008 at 01:27:51AM +0000, Pierre Habouzit wrote:
> Note that it also fix a bug, git checkout -- <path> would be badly
> understood as git checkout <branch> if <path> is ambiguous with a branch.
> 
> Testcases included.
> 
> Signed-off-by: Pierre Habouzit <madcoder@xxxxxxxxxx>
> ---
> 
>   This is a resend with proper patches that made me realize that my
> previous patch had silly mistakes and wasn't testing thigns properly.
> 
>  builtin-checkout.c            |   23 +++++++++++++++++------
>  t/t2010-checkout-ambiguous.sh |   35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+), 6 deletions(-)
>  create mode 100755 t/t2010-checkout-ambiguous.sh
> 
> diff --git a/builtin-checkout.c b/builtin-checkout.c
> index fbd5105..97321e6 100644
> --- a/builtin-checkout.c
> +++ b/builtin-checkout.c
> @@ -438,12 +438,17 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  
>  	opts.track = git_branch_track;
>  
> -	argc = parse_options(argc, argv, options, checkout_usage, 0);
> -	if (argc) {
> +	argc = parse_options(argc, argv, options, checkout_usage,
> +			     PARSE_OPT_KEEP_DASHDASH);
> +
> +	if (argc && strcmp(argv[0], "--")) {
> +		int may_be_ambiguous = argc == 1 || strcmp(argv[1], "--");
> +
>  		arg = argv[0];
> -		if (get_sha1(arg, rev))
> -			;
> -		else if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
> +		if (get_sha1(arg, rev)) {
> +			if (may_be_ambiguous)
> +				verify_filename(NULL, arg);

  Dscho mentionned on irc that this bit is superfluous, we already know
that 'arg' cannot be a refspec here, so in the worse case, if it's not a
file at all, this will generate a curious error message about
"ambiguous" arguments, whereas it should fail with an "unknown file"
error or sth similar. This two lines should probably remain ';' like it
previously was.

  The rest looks correct to me, but it's late...

> +		} else if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
>  			new.name = arg;
>  			setup_branch_path(&new);
>  			if (resolve_ref(new.path, rev, 1, NULL))
> @@ -454,10 +459,16 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  			source_tree = new.commit->tree;
>  			argv++;
>  			argc--;
> +			if (may_be_ambiguous)
> +				verify_non_filename(NULL, arg);
>  		} else if ((source_tree = parse_tree_indirect(rev))) {
>  			argv++;
>  			argc--;
> -		}
> +			if (may_be_ambiguous)
> +				verify_non_filename(NULL, arg);
> +		} else
> +			if (may_be_ambiguous)
> +				verify_filename(NULL, arg);
>  	}
>  
>  	if (argc && !strcmp(argv[0], "--")) {
> diff --git a/t/t2010-checkout-ambiguous.sh b/t/t2010-checkout-ambiguous.sh
> new file mode 100755
> index 0000000..389ba8c
> --- /dev/null
> +++ b/t/t2010-checkout-ambiguous.sh
> @@ -0,0 +1,35 @@
> +#!/bin/sh
> +
> +test_description='checkout and pathspecs/refspecs ambiguities'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +	echo hello >world &&
> +	echo hello >all &&
> +	git add all world &&
> +	git commit -m initial &&
> +	git branch world
> +'
> +
> +test_expect_success 'branch switching' '
> +	test "refs/heads/master" = "$(git symbolic-ref HEAD)" &&
> +	git checkout world -- &&
> +	test "refs/heads/world" = "$(git symbolic-ref HEAD)"
> +'
> +
> +test_expect_success 'checkout world from the index' '
> +	echo bye > world &&
> +	git checkout -- world &&
> +	git diff --exit-code --quiet
> +'
> +
> +test_expect_success 'non ambiguous call' '
> +	git checkout all
> +'
> +
> +test_expect_success 'ambiguous call' '
> +	test_must_fail git checkout world
> +'
> +
> +test_done
> -- 
> 1.6.0.rc0.154.ge2a39.dirty
> 


Attachment: pgpZodLwOKMUZ.pgp
Description: PGP signature


[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]

  Powered by Linux