Thread (51 messages) 51 messages, 4 authors, 2022-03-10

Re: [PATCH v2 3/4] commit-graph: start parsing generation v2 (again)

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-02-28 15:39:23

On Mon, Feb 28 2022, Derrick Stolee via GitGitGadget wrote:
From: Derrick Stolee <redacted>
[...]
+	GENERATION_VERSION=2
+	if test ! -z "$3"
TIL this works somewhere :)

I thought it *might* be unportable behavior (but didn't check at
first), but it's not! We have a few such cases already.

But IMO much less puzzling would be at least:

    if ! test -z "$3"

Or in this case, more plainly:

    if test -n "$3"
+	then
+		GENERATION_VERSION=$3
+	fi
+	OPTIONS=
+	if test $GENERATION_VERSION -gt 1
+	then
+		OPTIONS=" read_generation_data"
+	fi
Or actually, since we don't use $GENERATION_VERSION further down setting
it to a default etc. here seems a bit odd. Perhaps something closer to:

    if test $# -eq 3 && test $3 -gt 1

It's also possible to be more clever as e.g.:

    test "${3:-2}" -gt 1

But that hardly seems worth it...
quoted hunk ↗ jump to hunk
 NUM_COMMITS=9
diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
index 778fa418de2..669ddc645fa 100755
--- a/t/t5324-split-commit-graph.sh
+++ b/t/t5324-split-commit-graph.sh
@@ -30,11 +30,16 @@ graph_read_expect() {
 	then
 		NUM_BASE=$2
 	fi
+	OPTIONS=
+	if test -z "$3"
+	then
+		OPTIONS=" read_generation_data"
+	fi
 	cat >expect <<- EOF
 	header: 43475048 1 $(test_oid oid_version) 4 $NUM_BASE
 	num_commits: $1
 	chunks: oid_fanout oid_lookup commit_metadata generation_data
-	options:
+	options:$OPTIONS
 	EOF
 	test-tool read-graph >output &&
 	test_cmp expect output
Not a new issue, but it would be nice to have the mostly copy/pasted
code in a lib-commit-graph.sh or something, but probably too distracting
for this small series...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help