When we parse a padding directive ("%<" or "%>"), we might populate a
few of the struct's fields before bailing. This can result in such
half-parsed information being used to actually introduce some
padding/truncation.
When parsing a "%<" or "%>", only store the parsed data after parsing
successfully. The added test would have failed before this commit. It
also shows how the existing behavior is hardly something someone can
rely on since the non-consumed modifier ("%<(10,bad)") shows up verbatim
in the pretty output.
We could let the caller use a temporary struct and only copy the data on
success. Let's instead make our parsing function easy to use correctly
by letting it only touch the output struct in the success case.
While setting up a temporary struct for parsing into, we might as well
initialize it to a well-defined state. It's unnecessary for the current
implementation since it always writes to all three fields in a
successful case, but some future-proofing shouldn't hurt.
Note that the test relies on first using a correct placeholder
"%<(4,trunc)" where "trunc" (`trunc_right`) lingers in our struct until
it's then used instead of the invalid "bad". The next commit will teach
us to clean up any remnants of "%<(4,trunc)" after handling it.
Signed-off-by: Martin Ågren <martin.agren@xxxxxxxxx>
---
pretty.c | 18 ++++++++++++------
t/t4205-log-pretty-formats.sh | 6 ++++++
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/pretty.c b/pretty.c
index e5e8ef24fa..a4fa052f8b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1121,6 +1121,11 @@ static size_t parse_padding_placeholder(const char *placeholder,
const char *ch = placeholder;
enum flush_type flush_type;
int to_column = 0;
+ struct padding_args ans = {
+ .flush_type = no_flush,
+ .truncate = trunc_none,
+ .padding = 0,
+ };
switch (*ch++) {
case '<':
@@ -1171,8 +1176,8 @@ static size_t parse_padding_placeholder(const char *placeholder,
if (width < 0)
return 0;
}
- p->padding = to_column ? -width : width;
- p->flush_type = flush_type;
+ ans.padding = to_column ? -width : width;
+ ans.flush_type = flush_type;
if (*end == ',') {
start = end + 1;
@@ -1180,16 +1185,17 @@ static size_t parse_padding_placeholder(const char *placeholder,
if (!end || end == start)
return 0;
if (starts_with(start, "trunc)"))
- p->truncate = trunc_right;
+ ans.truncate = trunc_right;
else if (starts_with(start, "ltrunc)"))
- p->truncate = trunc_left;
+ ans.truncate = trunc_left;
else if (starts_with(start, "mtrunc)"))
- p->truncate = trunc_middle;
+ ans.truncate = trunc_middle;
else
return 0;
} else
- p->truncate = trunc_none;
+ ans.truncate = trunc_none;
+ *p = ans;
return end - placeholder + 1;
}
return 0;
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index f81e42a84d..26987ecd77 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -1130,6 +1130,12 @@ test_expect_success 'log --pretty with invalid padding format' '
test_cmp expect actual
'
+test_expect_success 'semi-parseable padding format does not get semi-applied' '
+ git log -1 --pretty="format:%<(4,trunc)%H%%<(10,bad)%H" >expect &&
+ git log -1 --pretty="format:%<(4,trunc)%H%<(10,bad)%H" >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'log --pretty with magical wrapping directives' '
commit_id=$(git commit-tree HEAD^{tree} -m "describe me") &&
git tag describe-me $commit_id &&
--
2.49.0.472.ge94155a9ec
