Re: [PATCH] test_cmp: diagnose incorrect arguments

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

 



Hello Eric,

>  test_cmp() {
> -	eval "$GIT_TEST_CMP" '"$@"'
> +	test $# -eq 2 || BUG "test_cmp requires two arguments"
> +	if ! eval "$GIT_TEST_CMP" '"$@"'
> +	then
> +		test -e "$1" || BUG "test_cmp 'expect' file missing"
> +		test -e "$2" || BUG "test_cmp 'actual' file missing"
> +		return 1
> +	fi
>  }

I reckon we could be just a little bit more precise here by bugging out
with the exact filename which is missing instead of 'expect' or 'actual'
so that the user has more idea as to what happened. What do you think?

BTW, I looked up the 'test_i18ncmp' function as well and if we have
this small loophole you mentioned above, I think maybe we could make a
similar fix for it too. What I mean is that in case of absence of the
required locale, it should error out kind of like what we did above

    BUG "locale missing"


so that the user it is clear to the user what was the failure point.
Though I will be honest that I have not really encountered a locale related
error or seen what the error looks like, so maybe we can ignore this
suggestion.

Regards,
Shourya Shukla




[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