Re: [PATCH] t3703, t4208: add test cases for magic pathspec

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

 



2011/5/9 Junio C Hamano <gitster@xxxxxxxxx>:
>> +test_expect_failure 'colon alone magic can only used alone' '
>> + Â Â test_must_fail git add -n sub/foo : &&
>> + Â Â test_must_fail git add -n : sub/foo
>> +'
>
> I don't care too much about this case (it is a user error), but if you
> promise you will turn this expect-failure to expect-success in a follow-up
> patch, why not ;-)?

failed tests are (annoying enough) to be fixed, right? :)

>> +cat >expected <<EOF
>> +add 'anothersub/foo'
>> +add 'expected'
>> +add 'sub/actual'
>> +add 'sub/foo'
>> +EOF
>> +
>> +test_expect_success 'add :' '
>> + Â Â (cd sub && git add -n : >actual) &&
>> + Â Â test_cmp expected sub/actual
>> +'
>
> Shouldn't
>
> Â Â Â Â$ git anycmd :
>
> be equivalent to
>
> Â Â Â Â$ (cd $(git rev-parse --show-cdup)/. && git anycmd)
>
> for any command? ÂI doubt this test is expecting the right outcome.
> Shouldn't it result in "Nothing specified, nothing added."?

It's gray area. Yes I'd rather see that behavior, but then a lot of
code needs to be audited. Perhaps we should delay introducing ":"
until get_pathspec() learns to modify argc. IOW die() for now when
users write ":".

>> +test_expect_success 'add :/non-existent' '
>> + Â Â (cd sub && test_must_fail git add -n :/non-existent)
>> +'
>
> Just being curious. What should the error message say? ÂCan we make it to
> say "fatal: pathspec 'non-ex' from root did not match any files"?

My opinion is in the next test. It should show exactly what users type
in, ie. "pathspec ':/non-existent' did not match any files". "'non-ex'
from root" is nice when there's only one magic. If users specify a few
more in one pathspec, the text may become too verbose.

>> +test_expect_failure 'show pathspecs exactly what are typed in' '
>> + Â Â test_cmp expected error
>> +'
>
> Will this break under gettext-poison?

Never really known what that poison is. Will grep ":/non-existent" instead.

>> +test_expect_failure 'git log :/ ambiguous with [ref]:/path' '
>> + Â Â test_must_fail git log :/ 2>error &&
>> + Â Â grep ambiguous error
>> +'
>> +test_expect_failure 'git log :' '
>> + Â Â git log :
>> +'
>
> These two should expect exactly the same error, I think. ':', ':/' or
> anything magic will not satisify verify_filename(), and needs a
> double-dash before it.

Yes. Error from the latter "fatal: Path '' does not exist (neither on
disk nor in the index)." makes me wonder, why does log accept a sha1
syntax that may resolve to non-commit object?

> We could improve the disambiguation heuristics so that when we do not have
> a '--' on the command line:
>
> Â- make sure all the earlier ones are refs and they cannot be a path on
> Â the filesystem (otherwise we need a disambiguator "--").
>
> Â- the first non-ref argument and everything that follows must be either a
> Â ':' magic, a string with globbing character, or a path on the
> Â filesystem, and none of them can be a ref.
>
> Do you want to take a stab at it?

A bit busy these days. If you're interested, go ahead.
-- 
Duy
--
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]