Re: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom

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

 



Karthik Nayak <karthik.188@xxxxxxxxx> writes:

> --- a/t/t6302-for-each-ref-filter.sh
> +++ b/t/t6302-for-each-ref-filter.sh
> @@ -149,4 +149,25 @@ test_expect_success 'check `colornext` format option' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'check `ifexists` format option' '
> +	cat >expect <<-\EOF &&
> +	[foo1.10]
> +	[foo1.3]
> +	[foo1.6]
> +	EOF
> +	git for-each-ref --format="%(ifexists:[%s])%(refname:short)" | grep "foo" >actual &&
> +	test_cmp expect actual
> +'

You're testing only the positive case of ifexists, right? You need a
test for the negative case (i.e. the attribute does not exist) too.
Ideally, check that the ifexist actually applies to the right attribute,
like

--format '%(ifexist:<%s>)%(attribute1) %(ifexist:[%s])%(attribute2)'

and manage to get an output like

<foo>
 [bar]
<foobar> [barfoo]

> +cat >expect <<EOF &&
> +[$(get_color green)foo1.10$(get_color reset)]||foo1.10
> +[$(get_color green)foo1.3$(get_color reset)]||foo1.3
> +[$(get_color green)foo1.6$(get_color reset)]||foo1.6
> +EOF
> +
> +test_expect_success 'check `ifexists` with `colornext` format option' '
> +	git for-each-ref --format="%(ifexists:[%s])%(colornext:green)%(refname:short)||%(refname:short)" | grep "foo" >actual &&
> +	test_cmp expect actual
> +'

You have a double || that is not useful. Not really harmful either, but
it may distract the reader. You may want to s/||/|/.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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]