Thread (1 message) 1 message, 1 author, 2020-07-09

Re: [PATCH v3 4/4] ref-filter: add support for %(contents:size)

From: Junio C Hamano <hidden>
Date: 2020-07-09 13:48:07

Christian Couder [off-list ref] writes:
quoted hunk
Yeah, I have been working on that and will send a new patch series soon.
The current test_atom() change looks like this:
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 371e45e5ad..e514d98574 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -52,6 +52,26 @@ test_atom() {
                sanitize_pgp <actual >actual.clean &&
                test_cmp expected actual.clean
        "
+       # Automatically test "contents:size" atom after testing "contents"
+       if test "$2" = "contents"
+       then
+               case "$1" in
+               refs/tags/signed-*)
+                       # We cannot use $3 as it expects sanitize_pgp to run
+                       git cat-file tag $ref | tail -n +6 | \
+                               wc -c | sed -e 's/^ *//' >expected ;;
+               refs/mytrees/*)
+                       echo >expected ;;
+               refs/myblobs/*)
+                       echo >expected ;;
+               *)
+                       printf '%s' "$3" | wc -c | sed -e 's/^ *//' >expected ;;
+               esac
+               test_expect_${4:-success} $PREREQ "basic atom: $1 $2:size" "
+                       git for-each-ref --format='%($2:size)' $ref >actual &&
+                       test_cmp expected actual
+               "
+       fi
 }
I am wondering if it's worth adding a preparatory patch to introduce
an helper function like the following in test-lib-functions.sh:

+# test_byte_count outputs the number of bytes in files or stdin
+#
+# It is like wc -c but without portability issues, as on macOS and
+# possibly other platforms leading whitespaces are emitted before the
+# number.
+
+test_byte_count () {
+       wc -c "$@" | sed -e 's/^ *//'
+}

Not sure about the name of this helper function as it works
differently than test_line_count().
Yeah, if I were writing it, I'd call it "sane_wc_c" or something.

But more importantly, I think the invocation of "|sed" is overkill.
If I were writing it, I would go more like...

	if test $2 = contents
	then
		case "$1" in 
		...)
			expect=$(git cat-file ... | wc -c)
			;;
		refs/mytrees/* | refs/myblobs/*)
			expect=0
			;;
		*)
			expect=$(printf ... | wc -c)
			;;
		esac

		# leave $expect unquoted to lose possible leading whitespaces
	        echo $expect >expect
		test_expect_success "..." '
			...
			test_cmp expect actual
		'
	fi
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help