[PATCH] t6300: values containing ')' are broken in ref formats

[Kousik Sanagavarapu <five231003@gmail.com>]

Document that values containing ')' in formats are not parsed correctly
in ref-filter.

The problem here is that ref-filter, while parsing the format string,
looks for the first occurence of ')' and marks it to be the end of _that_
particular atom - which is obviously wrong in cases where the format is
of type

	atom:key=value

where "value" has ')' somewhere in it.

However formats having a '(' instead in "value" will parse correctly
because in a general format string we also mark start of the format by
making note of '%(' instead of just '('.

For example, in

	%(if:equals=somere)f)%(refname:short)...

the string that ref-filter should compare against is "somere)f", although
since the parsing behavior in these cases is broken, we instead compare
against "somere".

While in

	%(if:equals=somere(f)%(refname:short)...

ref-filter rightly compares against "somere(f" as expected.

As a side note it should be mentioned that values containing ')' are
legit in %(refname) (and other atoms like %(upstream)).  Meaning
the parser wouldn't err out such values as they are legal, which is also
confirmed by - say the refname coming from

	$ git branch '1)branch'

or a remote name coming from

	$ git remote add 'up)stream' 'some.url'
	$ git push --set-upstream 'up)stream' '1)branch'

since none of these fail.

Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---

This raises the question of what can be done to parse ')' in values of
the format correctly.  It seems to me like a clean solution would
involve a huge refactoring involving a large portion of ref-filter but I
maybe wrong.

So this patch also hopes to open up discussion on not only solving this
bug but also how in general ref-filter currently parses and formats
atoms and if it is the way in which we would like to do things in the
future, which would in turn also be helpful in the long term goal of
merging both pretty and ref-filter.

Here is also a simple script to demonstrate the difference between '('
and ')' in values in formats - as described in the commit msg -

#!/bin/sh

rm -rf /tmp/atom-test-dir &&

# create env
git init /tmp/atom-test-dir 1>/tmp/init-dump 2>>/tmp/init-dump &&
cd /tmp/atom-test-dir &&
echo "smtg" >file &&
git add file &&
git commit -s -m "initial revision" >/tmp/commit-dump &&

# using "(" in refname works good
echo "test with refname as \"bran(ch\"" &&
git branch "bran(ch" &&
printf "bran(ch\n\n" >expect &&
git for-each-ref --format="%(if:equals=bran(ch)%(refname:short)%(then)%(refname:short)%(end)" refs/heads/ >actual &&
if ! diff -u expect actual; then
	echo "		actual is different from expect"
else
	echo "		actual is the same as expect"
fi

echo "" &&

# using ")" in refname will parse wrong in ref-filter code
echo "test with refname as \"bran()ch\"" &&
git branch "bran()ch" &&
printf "bran()ch\n\n\n" >expect &&
git for-each-ref --format="%(if:equals=bran()ch)%(refname:short)%(then)%(refname:short)%(end)" refs/heads/ >actual &&
if ! diff -u expect actual; then
	echo "		actual is different from expect"
else
	echo "		actual is the same as expect"
fi

 t/t6300-for-each-ref.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index c39d4e7e9c..ce5c607193 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -2141,4 +2141,19 @@ test_expect_success GPG 'show lack of signature with custom format' '
 	test_cmp expect actual
 '
 
+test_expect_failure 'format having values containing ) parse correctly' '
+	git branch "1)feat" &&
+	cat >expect <<-\EOF &&
+	refs/heads/1)feat
+	not equals
+	not equals
+	not equals
+	not equals
+	not equals
+	EOF
+	git for-each-ref --format="%(if:equals=1)feat)%(refname:short)%(then)%(refname)%(else)not equals%(end)" \
+		refs/heads/ >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.47.0.230.g0cf584699a