Re: [PATCH v4 1/2] diff: move no-index detection to builtin/diff.c

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

 



Thomas Gummerer <t.gummerer@xxxxxxxxx> writes:

>> What happens if I run 'git diff --no-index /tmp git.c git.c' from
>> within a git repository?  With this, I fear I will get
>
> Thanks, I've missed that one.  It only happens when run outside a git
> repository, but the same  comments still apply.  Will fix and send a
> re-roll.

Please don't, as the last round has already been pushed on 'next'.

An incremental change on top would also illustrate more clearly what
breakage needed to be fixed, which would be another good thing. It
could even come with a new test that makes sure that the above
command line is diagnosed correctly as a mistake ;-).

Thanks.

>
>> 	Not a git repository
>> 	To compare two paths outside a working tree:
>> 	usage: git diff [--no-index] <path> <path>
>>
>> instead of the expected
>>
>> 	usage: git diff --no-index <path> <path>
>>
>> Something like
>>
>> 	if (no_index)
>> 		;
>> 	else if (nongit)
>> 		no_index = DIFF_NO_INDEX_IMPLICIT;
>> 	else if (argc != i + 2)
>> 		;
>> 	else if (!path_inside_repo(prefix, argv[i]) ||
>> 		 !path_inside_repo(prefix, argv[i + 1]))
>> 		no_index = DIFF_NO_INDEX_IMPLICIT;
>>
>> should work.  Or:
>>
>> 	if (!no_index) {
>> 		/*
>> 		 * Treat git diff with ...
>> 		 */
>> 		if (nongit || ...)
>> 			no_index = DIFF_NO_INDEX_IMPLICIT;
>> 	}
>>
>> Or the '!no_index &&' condition inserted some other way.
>>
>>> -	/* If this is a no-index diff, just run it and exit there. */
>>> -	diff_no_index(&rev, argc, argv, nongit, prefix);
>>> +	if (no_index) {
>>> +		if (argc != i + 2) {
>> [...]
>>> +			/* Give the usage message for non-repository usage and exit. */
>>> +			usagef("git diff %s <path> <path>",
>>> +			       no_index == DIFF_NO_INDEX_EXPLICIT ?
>>> +					"--no-index" : "[--no-index]");
>>> +
>>> +		}
>>> +		/* If this is a no-index diff, just run it and exit there. */
>>> +		diff_no_index(&rev, argc, argv, prefix);
>>> +	}
>>
>> Perhaps, to avoid some nesting:
>>
>> 	/* A no-index diff takes exactly two path arguments. */
>> 	if (no_index && argc != i + 2) {
>> 		...
>> 		usagef(...);
>> 	}
>>
>> 	if (no_index)
>> 		/* Just run the diff and exit. */
>> 		diff_no_index(...);
>>
>> Jonathan
>
> Thanks, will change in the re-roll.
>
> --
> Thomas
--
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]