Re: [PATCH v4 2/7] pretty: allow %(trailers) options with explicit value
Оля Тележная writes: >> Oh my. I wasn't aware that there was a totally separate string >> interpolation implementation used for ref filters. That one has >> separated parsing, making it more amenable to good error handling. >> I wonder if that could be generalized and reused for pretty formats. >> >> However I doubt I will have time to dig deeper into that in near time. >> > > Sorry, I haven't read your patch in details. If you will be at Git Merge > tomorrow, you could ask me any questions, I can explain how for-each-ref > formatting works amd maybe even give you some ideas how to use its logic in > pretty, I was thinking about it a bit. No, unfortunately I'm not at Git Merge. I think I got how the formatting work. But if you have ideas on how to reuse the logic I'm all ears.
Re: [PATCH v4 2/7] pretty: allow %(trailers) options with explicit value
Jeff King writes: > There's some small value in leaving > %X alone if we do not understand "X" (not to mention the backwards > %compatibility you mentioned), but I think %() is a pretty > deliberate indication that a placeholder was meant there. Good point. > We already do this for ref-filter expansions: > > $ git for-each-ref --format='%(foo)' > fatal: unknown field name: foo > > We don't for "--pretty" formats, but I do wonder if anybody would be > really mad (after all, we have declared ourselves free to add new > placeholders, so such formats are not future-proof). Oh my. I wasn't aware that there was a totally separate string interpolation implementation used for ref filters. That one has separated parsing, making it more amenable to good error handling. I wonder if that could be generalized and reused for pretty formats. However I doubt I will have time to dig deeper into that in near time.
Re: [PATCH v5 2/7 update] pretty: allow %(trailers) options with explicit value
In addition to old %(trailers:only) it is now allowed to write
%(trailers:only=yes)
By itself this only gives (the not quite so useful) possibility to have
users change their mind in the middle of a formatting
string (%(trailers:only=true,only=false)). However, it gives users the
opportunity to override defaults from future options.
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 14 ++---
pretty.c | 52 +++-
t/t4205-log-pretty-formats.sh| 18 +++
3 files changed, 73 insertions(+), 11 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 86d804fe97..d33b072eb2 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -225,10 +225,16 @@ endif::git-rev-list[]
linkgit:git-interpret-trailers[1]. The
`trailers` string may be followed by a colon
and zero or more comma-separated options:
-** 'only': omit non-trailer lines from the trailer block.
-** 'unfold': make it behave as if interpret-trailer's `--unfold`
- option was given. E.g., `%(trailers:only,unfold)` unfolds and
- shows all trailer lines.
+** 'only[=val]': select whether non-trailer lines from the trailer
+ block should be included. The `only` keyword may optionally be
+ followed by an equal sign and one of `true`, `on`, `yes` to omit or
+ `false`, `off`, `no` to show the non-trailer lines. If option is
+ given without value it is enabled. If given multiple times the last
+ value is used.
+** 'unfold[=val]': make it behave as if interpret-trailer's `--unfold`
+ option was given. In same way as to for `only` it can be followed
+ by an equal sign and explicit value. E.g.,
+ `%(trailers:only,unfold=true)` unfolds and shows all trailer lines.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index b83a3ecd23..4dfbd38cf6 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1056,13 +1056,26 @@ static size_t parse_padding_placeholder(struct strbuf
*sb,
return 0;
}
-static int match_placeholder_arg(const char *to_parse, const char *candidate,
-const char **end)
+static int match_placeholder_arg_value(const char *to_parse, const char
*candidate,
+ const char **end, const char
**valuestart,
+ size_t *valuelen)
{
const char *p;
if (!(skip_prefix(to_parse, candidate, &p)))
return 0;
+ if (valuestart) {
+ if (*p == '=') {
+ *valuestart = p + 1;
+ *valuelen = strcspn(*valuestart, ",)");
+ p = *valuestart + *valuelen;
+ } else {
+ if (*p != ',' && *p != ')')
+ return 0;
+ *valuestart = NULL;
+ *valuelen = 0;
+ }
+ }
if (*p == ',') {
*end = p + 1;
return 1;
@@ -1074,6 +1087,34 @@ static int match_placeholder_arg(const char *to_parse,
const char *candidate,
return 0;
}
+static int match_placeholder_bool_arg(const char *to_parse, const char
*candidate,
+ const char **end, int *val)
+{
+ const char *argval;
+ char *strval;
+ size_t arglen;
+ int v;
+
+ if (!match_placeholder_arg_value(to_parse, candidate, end, &argval,
&arglen))
+ return 0;
+
+ if (!argval) {
+ *val = 1;
+ return 1;
+ }
+
+ strval = xstrndup(argval, arglen);
+ v = git_parse_maybe_bool(strval);
+ free(strval);
+
+ if (v == -1)
+ return 0;
+
+ *val = v;
+
+ return 1;
+}
+
static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -1318,11 +1359,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (*arg == ':') {
arg++;
for (;;) {
- if (match_placeholder_arg(arg, "only", &arg))
- opts.only_trailers = 1;
- else if (match_placeholder_arg(arg, "unfold",
&arg))
- opts.unfold = 1;
- else
+ if (!match_placeholder_bool_arg(arg, "only",
&arg, &opts.only_trailers) &&
+
Re: [PATCH v5 2/7] pretty: Allow %(trailers) options with explicit value
Junio C Hamano writes:
>> ...
>> +static int match_placeholder_bool_arg(const char *to_parse, const char
>> *candidate,
>> + const char **end, int *val)
>> +{
>> +char buf[8];
>> +const char *strval;
>> +size_t len;
>> +int v;
>> +
>> +if (!match_placeholder_arg_value(to_parse, candidate, end, &strval,
>> &len))
>> +return 0;
>> +
>> +if (!strval) {
>> +*val = 1;
>> +return 1;
>> +}
>> +
>> +strlcpy(buf, strval, sizeof(buf));
>> +if (len < sizeof(buf))
>> +buf[len] = 0;
>
> Doesn't strlcpy() terminate buf[len] if len is short enough?
> Even if the strval is longer than buf[], strlcpy() would truncate
> and make sure buf[] is NUL terminated, no?
Yes, but no. strval is not NUL-terminated at len. E.g strval would point
to "false,something=true". `buf[len] = 0` makes sure it becomes "false".
> Instead of using "char buf[8]", just using a strbuf and avoidng
> strlcpy() would make the code much better, I would think.
Yes, taking the heap allocation hit would most likely make the intent
clearer.
[PATCH v5 6/7] strbuf: separate callback for strbuf_expand:ing literals
Expanding '%n' and '%xNN' is generic functionality, so extract that from
the pretty.c formatter into a callback that can be reused.
No functional change intended
Signed-off-by: Anders Waldenborg
---
pretty.c | 16 +---
strbuf.c | 21 +
strbuf.h | 8
3 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/pretty.c b/pretty.c
index ed25845c98..7baa4c1c26 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1137,9 +1137,13 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
const char *msg = c->message;
struct commit_list *p;
const char *arg;
- int ch;
+ size_t res;
/* these are independent of the commit */
+ res = strbuf_expand_literal_cb(sb, placeholder, NULL);
+ if (res)
+ return res;
+
switch (placeholder[0]) {
case 'C':
if (starts_with(placeholder + 1, "(auto)")) {
@@ -1158,16 +1162,6 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
*/
return ret;
}
- case 'n': /* newline */
- strbuf_addch(sb, '\n');
- return 1;
- case 'x':
- /* %x00 == NUL, %x0a == LF, etc. */
- ch = hex2chr(placeholder + 1);
- if (ch < 0)
- return 0;
- strbuf_addch(sb, ch);
- return 3;
case 'w':
if (placeholder[1] == '(') {
unsigned long width = 0, indent1 = 0, indent2 = 0;
diff --git a/strbuf.c b/strbuf.c
index f6a6cf78b9..78eecd29f7 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -380,6 +380,27 @@ void strbuf_expand(struct strbuf *sb, const char *format,
expand_fn_t fn,
}
}
+size_t strbuf_expand_literal_cb(struct strbuf *sb,
+ const char *placeholder,
+ void *context)
+{
+ int ch;
+
+ switch (placeholder[0]) {
+ case 'n': /* newline */
+ strbuf_addch(sb, '\n');
+ return 1;
+ case 'x':
+ /* %x00 == NUL, %x0a == LF, etc. */
+ ch = hex2chr(placeholder + 1);
+ if (ch < 0)
+ return 0;
+ strbuf_addch(sb, ch);
+ return 3;
+ }
+ return 0;
+}
+
size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
void *context)
{
diff --git a/strbuf.h b/strbuf.h
index fc40873b65..52e44c9ab8 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -320,6 +320,14 @@ void strbuf_expand(struct strbuf *sb,
expand_fn_t fn,
void *context);
+/**
+ * Used as callback for `strbuf_expand` to only expand literals
+ * (i.e. %n and %xNN). The context argument is ignored.
+ */
+size_t strbuf_expand_literal_cb(struct strbuf *sb,
+ const char *placeholder,
+ void *context);
+
/**
* Used as callback for `strbuf_expand()`, expects an array of
* struct strbuf_expand_dict_entry as context, i.e. pairs of
--
2.17.1
[PATCH v5 3/7] pretty: single return path in %(trailers) handling
No functional change intended.
This change may not seem useful on its own, but upcoming commits will do
memory allocation in there, and a single return path makes deallocation
easier.
Signed-off-by: Anders Waldenborg
---
pretty.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/pretty.c b/pretty.c
index b8d71a57c9..65a1b9bd82 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1353,6 +1353,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts =
PROCESS_TRAILER_OPTIONS_INIT;
+ size_t ret = 0;
opts.no_divider = 1;
@@ -1366,8 +1367,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
}
if (*arg == ')') {
format_trailers_from_commit(sb, msg + c->subject_off,
&opts);
- return arg - placeholder + 1;
+ ret = arg - placeholder + 1;
}
+ return ret;
}
return 0; /* unknown placeholder */
--
2.17.1
[PATCH v5 1/7] doc: group pretty-format.txt placeholders descriptions
The placeholders can be grouped into three kinds:
* literals
* affecting formatting of later placeholders
* expanding to information in commit
Also change the list to a definition list (using '::')
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 235 ---
1 file changed, 125 insertions(+), 110 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 417b638cd8..86d804fe97 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -102,118 +102,133 @@ The title was >>t4119: test autocomputing -p for
traditional diff input.<<
+
The placeholders are:
-- '%H': commit hash
-- '%h': abbreviated commit hash
-- '%T': tree hash
-- '%t': abbreviated tree hash
-- '%P': parent hashes
-- '%p': abbreviated parent hashes
-- '%an': author name
-- '%aN': author name (respecting .mailmap, see linkgit:git-shortlog[1]
- or linkgit:git-blame[1])
-- '%ae': author email
-- '%aE': author email (respecting .mailmap, see
- linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- '%ad': author date (format respects --date= option)
-- '%aD': author date, RFC2822 style
-- '%ar': author date, relative
-- '%at': author date, UNIX timestamp
-- '%ai': author date, ISO 8601-like format
-- '%aI': author date, strict ISO 8601 format
-- '%cn': committer name
-- '%cN': committer name (respecting .mailmap, see
- linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- '%ce': committer email
-- '%cE': committer email (respecting .mailmap, see
- linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- '%cd': committer date (format respects --date= option)
-- '%cD': committer date, RFC2822 style
-- '%cr': committer date, relative
-- '%ct': committer date, UNIX timestamp
-- '%ci': committer date, ISO 8601-like format
-- '%cI': committer date, strict ISO 8601 format
-- '%d': ref names, like the --decorate option of linkgit:git-log[1]
-- '%D': ref names without the " (", ")" wrapping.
-- '%e': encoding
-- '%s': subject
-- '%f': sanitized subject line, suitable for a filename
-- '%b': body
-- '%B': raw body (unwrapped subject and body)
+- Placeholders that expand to a single literal character:
+'%n':: newline
+'%%':: a raw '%'
+'%x00':: print a byte from a hex code
+
+- Placeholders that affect formatting of later placeholders:
+'%Cred':: switch color to red
+'%Cgreen':: switch color to green
+'%Cblue':: switch color to blue
+'%Creset':: reset color
+'%C(...)':: color specification, as described under Values in the
+"CONFIGURATION FILE" section of linkgit:git-config[1]. By
+default, colors are shown only when enabled for log output
+(by `color.diff`, `color.ui`, or `--color`, and respecting
+the `auto` settings of the former if we are going to a
+terminal). `%C(auto,...)` is accepted as a historical
+synonym for the default (e.g., `%C(auto,red)`). Specifying
+`%C(always,...) will show the colors even when color is
+not otherwise enabled (though consider just using
+`--color=always` to enable color for the whole output,
+including this format and anything else git might color).
+`auto` alone (i.e. `%C(auto)`) will turn on auto coloring
+on the next placeholders until the color is switched
+again.
+'%m':: left (`<`), right (`>`) or boundary (`-`) mark
+'%w([[,[,]]])':: switch line wrapping, like the -w option of
+linkgit:git-shortlog[1].
+'%<([,trunc|ltrunc|mtrunc])':: make the next placeholder take at
+ least N columns, padding spaces on
+ the right if necessary. Optionally
+ truncate at the beginning (ltrunc),
+ the middle (mtrunc) or the end
+ (trunc) if the output is longer than
+ N columns. Note that truncating
+ only works correctly with N >= 2.
+'%<|()':: make the next placeholder take at least until Nth
+ columns, padding spaces on the right if necessary
+'%>()', '%>|()':: similar to '%<()', '%<|()' respectively,
+but padding spaces on the left
+'%>>()', '%>>|()':: similar
[PATCH v5 5/7] pretty: add support for "valueonly" option in %(trailers)
With the new "key=" option to %(trailers) it often makes little sense to
show the key, as it by definition already is knows which trailer is
printed there. This new "valueonly" option makes it omit the key when
printing trailers.
E.g.:
$ git show -s --pretty='%s%n%(trailers:key=Signed-off-by,valueonly)' 88182
will show:
> upload-pack: fix broken if/else chain in config callback
> Jeff King
> Junio C Hamano
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 2 ++
pretty.c | 3 ++-
t/t4205-log-pretty-formats.sh| 6 ++
trailer.c| 6 --
trailer.h| 1 +
5 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d6add831c0..a920dd15b1 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -243,6 +243,8 @@ endif::git-rev-list[]
option was given. In same way as to for `only` it can be followed
by an equal sign and explicit value. E.g.,
`%(trailers:only,unfold=true)` unfolds and shows all trailer lines.
+** 'valueonly[=val]': skip over the key part of the trailer line and only
+ show the value part. Also this optionally allows explicit value.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index d3dd2d6254..ed25845c98 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1391,7 +1391,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
opts.filter_data = &filter_list;
opts.only_trailers = 1;
} else if (!match_placeholder_bool_arg(arg,
"only", &arg, &opts.only_trailers) &&
- !match_placeholder_bool_arg(arg,
"unfold", &arg, &opts.unfold))
+ !match_placeholder_bool_arg(arg,
"unfold", &arg, &opts.unfold) &&
+ !match_placeholder_bool_arg(arg,
"valueonly", &arg, &opts.value_only))
break;
}
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index d87201afbe..1ad6834781 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -673,6 +673,12 @@ test_expect_success '%(trailers:key) without value is
error' '
test_cmp expect actual
'
+test_expect_success '%(trailers:key=foo,valueonly) shows only value' '
+ git log --no-walk --pretty="format:%(trailers:key=Acked-by,valueonly)"
>actual &&
+ echo "A U Thor " >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'trailer parsing not fooled by --- line' '
git commit --allow-empty -F - <<-\EOF &&
this is the subject
diff --git a/trailer.c b/trailer.c
index d6da555cd7..d0d9e91631 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1150,8 +1150,10 @@ static void format_trailer_info(struct strbuf *out,
if (!opts->filter || opts->filter(&tok,
opts->filter_data)) {
if (opts->unfold)
unfold_value(&val);
-
- strbuf_addf(out, "%s: %s\n", tok.buf, val.buf);
+ if (!opts->value_only)
+ strbuf_addf(out, "%s: ", tok.buf);
+ strbuf_addbuf(out, &val);
+ strbuf_addch(out, '\n');
}
strbuf_release(&tok);
strbuf_release(&val);
diff --git a/trailer.h b/trailer.h
index 5255b676de..06d417fe93 100644
--- a/trailer.h
+++ b/trailer.h
@@ -72,6 +72,7 @@ struct process_trailer_options {
int only_input;
int unfold;
int no_divider;
+ int value_only;
int (*filter)(const struct strbuf *, void *);
void *filter_data;
};
--
2.17.1
[PATCH v5 7/7] pretty: add support for separator option in %(trailers)
By default trailer lines are terminated by linebreaks ('\n'). By
specifying the new 'separator' option they will instead be separated by
user provided string and have separator semantics rather than terminator
semantics. The separator string can contain the literal formatting codes
%n and %xNN allowing it to be things that are otherwise hard to type
such as %x00, or comma and end-parenthesis which would break parsing.
E.g:
$ git log --pretty='%(trailers:key=Reviewed-by,valueonly,separator=%x00)'
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 9
pretty.c | 10 +
t/t4205-log-pretty-formats.sh| 36
trailer.c| 15 +++--
trailer.h| 1 +
5 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index a920dd15b1..ce087dee80 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -239,6 +239,15 @@ endif::git-rev-list[]
`false`, `off`, `no` to show the non-trailer lines. If option is
given without value it is enabled. If given multiple times the last
value is used.
+** 'separator=': specify a separator inserted between trailer
+ lines. When this option is not given each trailer line is
+ terminated with a line feed character. The string SEP may contain
+ the literal formatting codes described above. To use comma as
+ separator one must use `%x2C` as it would otherwise be parsed as
+ next option. If separator option is given multiple times only the
+ last one is used. E.g., `%(trailers:key=Ticket,separator=%x2C )`
+ shows all trailer lines whose key is "Ticket" separated by a comma
+ and a space.
** 'unfold[=val]': make it behave as if interpret-trailer's `--unfold`
option was given. In same way as to for `only` it can be followed
by an equal sign and explicit value. E.g.,
diff --git a/pretty.c b/pretty.c
index 7baa4c1c26..99b66ccf5a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1361,6 +1361,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts =
PROCESS_TRAILER_OPTIONS_INIT;
struct string_list filter_list = STRING_LIST_INIT_NODUP;
+ struct strbuf sepbuf = STRBUF_INIT;
size_t ret = 0;
opts.no_divider = 1;
@@ -1384,6 +1385,14 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
opts.filter = format_trailer_match_cb;
opts.filter_data = &filter_list;
opts.only_trailers = 1;
+ } else if (match_placeholder_arg_value(arg,
"separator", &arg, &argval, &arglen)) {
+ char *fmt;
+
+ strbuf_reset(&sepbuf);
+ fmt = xstrndup(argval, arglen);
+ strbuf_expand(&sepbuf, fmt,
strbuf_expand_literal_cb, NULL);
+ free(fmt);
+ opts.separator = &sepbuf;
} else if (!match_placeholder_bool_arg(arg,
"only", &arg, &opts.only_trailers) &&
!match_placeholder_bool_arg(arg,
"unfold", &arg, &opts.unfold) &&
!match_placeholder_bool_arg(arg,
"valueonly", &arg, &opts.value_only))
@@ -1396,6 +1405,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
}
trailer_out:
string_list_clear(&filter_list, 0);
+ strbuf_release(&sepbuf);
return ret;
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 1ad6834781..99f50fa401 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -679,6 +679,42 @@ test_expect_success '%(trailers:key=foo,valueonly) shows
only value' '
test_cmp expect actual
'
+test_expect_success 'pretty format %(trailers:separator) changes separator' '
+ git log --no-walk --pretty=format:"X%(trailers:separator=%x00,unfold)X"
>actual &&
+ printf "XSigned-off-by: A U Thor \0Acked-by: A U
Thor \0[ v2 updated patch description ]\0Signed-off-by: A U
Thor X" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'pretty format %(trailers) combining
separator/key/valueonly' '
+
[PATCH v5 4/7] pretty: allow showing specific trailers
Adds a new "key=X" option to "%(trailers)" which will cause it to only
print trailer lines which match any of the specified keys.
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 8 +
pretty.c | 36 ++--
t/t4205-log-pretty-formats.sh| 57
trailer.c| 10 +++---
trailer.h| 2 ++
5 files changed, 107 insertions(+), 6 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d33b072eb2..d6add831c0 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -225,6 +225,14 @@ endif::git-rev-list[]
linkgit:git-interpret-trailers[1]. The
`trailers` string may be followed by a colon
and zero or more comma-separated options:
+** 'key=': only show trailers with specified key. Matching is done
+ case-insensitively and trailing colon is optional. If option is
+ given multiple times trailer lines matching any of the keys are
+ shown. This option automatically enables the `only` option so that
+ non-trailer lines in the trailer block are hidden. If that is not
+ desired it can be disabled with `only=false`. E.g.,
+ `%(trailers:key=Reviewed-by)` shows trailer lines with key
+ `Reviewed-by`.
** 'only[=val]': select whether non-trailer lines from the trailer
block should be included. The `only` keyword may optionally be
followed by an equal sign and one of `true`, `on`, `yes` to omit or
diff --git a/pretty.c b/pretty.c
index 65a1b9bd82..d3dd2d6254 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1115,6 +1115,19 @@ static int match_placeholder_bool_arg(const char
*to_parse, const char *candidat
return 1;
}
+static int format_trailer_match_cb(const struct strbuf *key, void *ud)
+{
+ const struct string_list *list = ud;
+ const struct string_list_item *item;
+
+ for_each_string_list_item (item, list) {
+ if (key->len == (uintptr_t)item->util &&
+ !strncasecmp(item->string, key->buf, key->len))
+ return 1;
+ }
+ return 0;
+}
+
static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -1353,6 +1366,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts =
PROCESS_TRAILER_OPTIONS_INIT;
+ struct string_list filter_list = STRING_LIST_INIT_NODUP;
size_t ret = 0;
opts.no_divider = 1;
@@ -1360,8 +1374,24 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (*arg == ':') {
arg++;
for (;;) {
- if (!match_placeholder_bool_arg(arg, "only",
&arg, &opts.only_trailers) &&
- !match_placeholder_bool_arg(arg, "unfold",
&arg, &opts.unfold))
+ const char *argval;
+ size_t arglen;
+
+ if (match_placeholder_arg_value(arg, "key",
&arg, &argval, &arglen)) {
+ uintptr_t len = arglen;
+
+ if (!argval)
+ goto trailer_out;
+
+ if (len && argval[len - 1] == ':')
+ len--;
+ string_list_append(&filter_list,
argval)->util = (char *)len;
+
+ opts.filter = format_trailer_match_cb;
+ opts.filter_data = &filter_list;
+ opts.only_trailers = 1;
+ } else if (!match_placeholder_bool_arg(arg,
"only", &arg, &opts.only_trailers) &&
+ !match_placeholder_bool_arg(arg,
"unfold", &arg, &opts.unfold))
break;
}
}
@@ -1369,6 +1399,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
format_trailers_from_commit(sb, msg + c->subject_off,
&opts);
ret = arg - placeholder + 1;
}
+ trailer_out:
+ string_list_clear(&filter_list, 0);
return ret;
}
diff --git a/t/t420
[PATCH v5 2/7] pretty: Allow %(trailers) options with explicit value
In addition to old %(trailers:only) it is now allowed to write
%(trailers:only=yes)
By itself this only gives (the not quite so useful) possibility to have
users change their mind in the middle of a formatting
string (%(trailers:only=true,only=false)). However, it gives users the
opportunity to override defaults from future options.
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 14 ++---
pretty.c | 52 +++-
t/t4205-log-pretty-formats.sh| 18 +++
3 files changed, 73 insertions(+), 11 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 86d804fe97..d33b072eb2 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -225,10 +225,16 @@ endif::git-rev-list[]
linkgit:git-interpret-trailers[1]. The
`trailers` string may be followed by a colon
and zero or more comma-separated options:
-** 'only': omit non-trailer lines from the trailer block.
-** 'unfold': make it behave as if interpret-trailer's `--unfold`
- option was given. E.g., `%(trailers:only,unfold)` unfolds and
- shows all trailer lines.
+** 'only[=val]': select whether non-trailer lines from the trailer
+ block should be included. The `only` keyword may optionally be
+ followed by an equal sign and one of `true`, `on`, `yes` to omit or
+ `false`, `off`, `no` to show the non-trailer lines. If option is
+ given without value it is enabled. If given multiple times the last
+ value is used.
+** 'unfold[=val]': make it behave as if interpret-trailer's `--unfold`
+ option was given. In same way as to for `only` it can be followed
+ by an equal sign and explicit value. E.g.,
+ `%(trailers:only,unfold=true)` unfolds and shows all trailer lines.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index b83a3ecd23..b8d71a57c9 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1056,13 +1056,25 @@ static size_t parse_padding_placeholder(struct strbuf
*sb,
return 0;
}
-static int match_placeholder_arg(const char *to_parse, const char *candidate,
-const char **end)
+static int match_placeholder_arg_value(const char *to_parse, const char
*candidate,
+ const char **end, const char
**valuestart, size_t *valuelen)
{
const char *p;
if (!(skip_prefix(to_parse, candidate, &p)))
return 0;
+ if (valuestart) {
+ if (*p == '=') {
+ *valuestart = p + 1;
+ *valuelen = strcspn(*valuestart, ",)");
+ p = *valuestart + *valuelen;
+ } else {
+ if (*p != ',' && *p != ')')
+ return 0;
+ *valuestart = NULL;
+ *valuelen = 0;
+ }
+ }
if (*p == ',') {
*end = p + 1;
return 1;
@@ -1074,6 +1086,35 @@ static int match_placeholder_arg(const char *to_parse,
const char *candidate,
return 0;
}
+static int match_placeholder_bool_arg(const char *to_parse, const char
*candidate,
+ const char **end, int *val)
+{
+ char buf[8];
+ const char *strval;
+ size_t len;
+ int v;
+
+ if (!match_placeholder_arg_value(to_parse, candidate, end, &strval,
&len))
+ return 0;
+
+ if (!strval) {
+ *val = 1;
+ return 1;
+ }
+
+ strlcpy(buf, strval, sizeof(buf));
+ if (len < sizeof(buf))
+ buf[len] = 0;
+
+ v = git_parse_maybe_bool(buf);
+ if (v == -1)
+ return 0;
+
+ *val = v;
+
+ return 1;
+}
+
static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -1318,11 +1359,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (*arg == ':') {
arg++;
for (;;) {
- if (match_placeholder_arg(arg, "only", &arg))
- opts.only_trailers = 1;
- else if (match_placeholder_arg(arg, "unfold",
&arg))
- opts.unfold = 1;
- else
+ if (!match_placeholder_bool_arg(arg, "only",
&arg, &opts.only_trailers) &&
+
[PATCH v5 0/7] %(trailers) improvements in pretty format
Updates since v4: * Coding style fixes * Reuse git_parse_maybe_bool for bool parsing Anders Waldenborg (7): doc: group pretty-format.txt placeholders descriptions pretty: Allow %(trailers) options with explicit value pretty: single return path in %(trailers) handling pretty: allow showing specific trailers pretty: add support for "valueonly" option in %(trailers) strbuf: separate callback for strbuf_expand:ing literals pretty: add support for separator option in %(trailers) Documentation/pretty-formats.txt | 260 ++- pretty.c | 113 +++--- strbuf.c | 21 +++ strbuf.h | 8 + t/t4205-log-pretty-formats.sh| 117 ++ trailer.c| 25 ++- trailer.h| 4 + 7 files changed, 415 insertions(+), 133 deletions(-) -- 2.17.1
Re: [PATCH v4 2/7] pretty: allow %(trailers) options with explicit value
Junio C Hamano writes: > That way, we can handle %(trailers:only=bogo) more sensibly, > no? Syntactically we can recognize that the user wanted to give > 'bogo' as the value to 'only', and say "'bogo' is not a boolean" if > we did so. I agree that proper error reporting for the pretty formatting strings would be great. But that would depart from the current extremely crude error handling where incorrect formatting placeholders are just left unexpanded. How would such change in error handling be done safely, wrt backwards compatibility changes? To get good diagnostics for incorrect formatting strings I think the way forward is to have the formatting strings parsed once into some kind of AST or machine (as also mentioned by Jeff) that is just executed many times, instead of parsed each time like today. anders
[PATCH v4 4/7] pretty: allow showing specific trailers
Adds a new "key=X" option to "%(trailers)" which will cause it to only
print trailer lines which match any of the specified keys.
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 8 +
pretty.c | 47 ++---
t/t4205-log-pretty-formats.sh| 51
trailer.c| 10 ---
trailer.h| 2 ++
5 files changed, 110 insertions(+), 8 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d33b072eb2..d6add831c0 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -225,6 +225,14 @@ endif::git-rev-list[]
linkgit:git-interpret-trailers[1]. The
`trailers` string may be followed by a colon
and zero or more comma-separated options:
+** 'key=': only show trailers with specified key. Matching is done
+ case-insensitively and trailing colon is optional. If option is
+ given multiple times trailer lines matching any of the keys are
+ shown. This option automatically enables the `only` option so that
+ non-trailer lines in the trailer block are hidden. If that is not
+ desired it can be disabled with `only=false`. E.g.,
+ `%(trailers:key=Reviewed-by)` shows trailer lines with key
+ `Reviewed-by`.
** 'only[=val]': select whether non-trailer lines from the trailer
block should be included. The `only` keyword may optionally be
followed by an equal sign and one of `true`, `on`, `yes` to omit or
diff --git a/pretty.c b/pretty.c
index 07e6c0..541a553ccc 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1056,13 +1056,20 @@ static size_t parse_padding_placeholder(struct strbuf
*sb,
return 0;
}
-static int match_placeholder_arg(const char *to_parse, const char *candidate,
-const char **end)
+static int match_placeholder_arg_value(const char *to_parse, const char
*candidate,
+ const char **end, const char
**valuestart, size_t *valuelen)
{
const char *p;
if (!(skip_prefix(to_parse, candidate, &p)))
return 0;
+ if (valuestart) {
+ if (*p != '=')
+ return 0;
+ *valuestart = p + 1;
+ *valuelen = strcspn(*valuestart, ",)");
+ p = *valuestart + *valuelen;
+ }
if (*p == ',') {
*end = p + 1;
return 1;
@@ -1074,6 +1081,12 @@ static int match_placeholder_arg(const char *to_parse,
const char *candidate,
return 0;
}
+static int match_placeholder_arg(const char *to_parse, const char *candidate,
+const char **end)
+{
+ return match_placeholder_arg_value(to_parse, candidate, end, NULL,
NULL);
+}
+
static int match_placeholder_bool_arg(const char *to_parse, const char
*candidate,
const char **end, int *val)
{
@@ -1095,7 +1108,19 @@ static int match_placeholder_bool_arg(const char
*to_parse, const char *candidat
*val = 1;
return 1;
}
+ return 0;
+}
+static int format_trailer_match_cb(const struct strbuf *key, void *ud)
+{
+ const struct string_list *list = ud;
+ const struct string_list_item *item;
+
+ for_each_string_list_item (item, list) {
+ if (key->len == (uintptr_t)item->util &&
+ !strncasecmp (item->string, key->buf, key->len))
+ return 1;
+ }
return 0;
}
@@ -1337,6 +1362,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts =
PROCESS_TRAILER_OPTIONS_INIT;
+ struct string_list filter_list = STRING_LIST_INIT_NODUP;
size_t ret = 0;
opts.no_divider = 1;
@@ -1344,8 +1370,20 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (*arg == ':') {
arg++;
for (;;) {
- if (!match_placeholder_bool_arg(arg, "only",
&arg, &opts.only_trailers) &&
- !match_placeholder_bool_arg(arg, "unfold",
&arg, &opts.unfold))
+ const char *argval;
+ size_t arglen;
+
+ if (match_placeholder_arg_value(arg, "key",
&arg, &argval, &arglen)) {
+ uintptr_t len = arglen;
+ if (
[PATCH v4 3/7] pretty: single return path in %(trailers) handling
No functional change intended.
This change may not seem useful on its own, but upcoming commits will do
memory allocation in there, and a single return path makes deallocation
easier.
Signed-off-by: Anders Waldenborg
---
pretty.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/pretty.c b/pretty.c
index 26efdba73a..07e6c0 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1337,6 +1337,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts =
PROCESS_TRAILER_OPTIONS_INIT;
+ size_t ret = 0;
opts.no_divider = 1;
@@ -1350,8 +1351,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
}
if (*arg == ')') {
format_trailers_from_commit(sb, msg + c->subject_off,
&opts);
- return arg - placeholder + 1;
+ ret = arg - placeholder + 1;
}
+ return ret;
}
return 0; /* unknown placeholder */
--
2.17.1
[PATCH v4 0/7] %(trailers) improvements in pretty format
Updated since v3: * multiple 'key=' matches any * allow overriding implicit 'only' when using key * minor grammar and spelling fixes * documentation restructuring * Helper functions for parsing options Anders Waldenborg (7): doc: group pretty-format.txt placeholders descriptions pretty: allow %(trailers) options with explicit value pretty: single return path in %(trailers) handling pretty: allow showing specific trailers pretty: add support for "valueonly" option in %(trailers) strbuf: separate callback for strbuf_expand:ing literals pretty: add support for separator option in %(trailers) Documentation/pretty-formats.txt | 260 ++- pretty.c | 104 ++--- strbuf.c | 21 +++ strbuf.h | 8 + t/t4205-log-pretty-formats.sh| 111 + trailer.c| 25 ++- trailer.h| 4 + 7 files changed, 400 insertions(+), 133 deletions(-) -- 2.17.1
[PATCH v4 6/7] strbuf: separate callback for strbuf_expand:ing literals
Expanding '%n' and '%xNN' is generic functionality, so extract that from
the pretty.c formatter into a callback that can be reused.
No functional change intended
Signed-off-by: Anders Waldenborg
---
pretty.c | 16 +---
strbuf.c | 21 +
strbuf.h | 8
3 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/pretty.c b/pretty.c
index c508357606..50d0b5830d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1133,9 +1133,13 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
const char *msg = c->message;
struct commit_list *p;
const char *arg;
- int ch;
+ size_t res;
/* these are independent of the commit */
+ res = strbuf_expand_literal_cb(sb, placeholder, NULL);
+ if (res)
+ return res;
+
switch (placeholder[0]) {
case 'C':
if (starts_with(placeholder + 1, "(auto)")) {
@@ -1154,16 +1158,6 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
*/
return ret;
}
- case 'n': /* newline */
- strbuf_addch(sb, '\n');
- return 1;
- case 'x':
- /* %x00 == NUL, %x0a == LF, etc. */
- ch = hex2chr(placeholder + 1);
- if (ch < 0)
- return 0;
- strbuf_addch(sb, ch);
- return 3;
case 'w':
if (placeholder[1] == '(') {
unsigned long width = 0, indent1 = 0, indent2 = 0;
diff --git a/strbuf.c b/strbuf.c
index f6a6cf78b9..78eecd29f7 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -380,6 +380,27 @@ void strbuf_expand(struct strbuf *sb, const char *format,
expand_fn_t fn,
}
}
+size_t strbuf_expand_literal_cb(struct strbuf *sb,
+ const char *placeholder,
+ void *context)
+{
+ int ch;
+
+ switch (placeholder[0]) {
+ case 'n': /* newline */
+ strbuf_addch(sb, '\n');
+ return 1;
+ case 'x':
+ /* %x00 == NUL, %x0a == LF, etc. */
+ ch = hex2chr(placeholder + 1);
+ if (ch < 0)
+ return 0;
+ strbuf_addch(sb, ch);
+ return 3;
+ }
+ return 0;
+}
+
size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
void *context)
{
diff --git a/strbuf.h b/strbuf.h
index fc40873b65..52e44c9ab8 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -320,6 +320,14 @@ void strbuf_expand(struct strbuf *sb,
expand_fn_t fn,
void *context);
+/**
+ * Used as callback for `strbuf_expand` to only expand literals
+ * (i.e. %n and %xNN). The context argument is ignored.
+ */
+size_t strbuf_expand_literal_cb(struct strbuf *sb,
+ const char *placeholder,
+ void *context);
+
/**
* Used as callback for `strbuf_expand()`, expects an array of
* struct strbuf_expand_dict_entry as context, i.e. pairs of
--
2.17.1
[PATCH v4 5/7] pretty: add support for "valueonly" option in %(trailers)
With the new "key=" option to %(trailers) it often makes little sense to
show the key, as it by definition already is knows which trailer is
printed there. This new "valueonly" option makes it omit the key when
printing trailers.
E.g.:
$ git show -s --pretty='%s%n%(trailers:key=Signed-off-by,valueonly)' 88182
will show:
> upload-pack: fix broken if/else chain in config callback
> Jeff King
> Junio C Hamano
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 2 ++
pretty.c | 3 ++-
t/t4205-log-pretty-formats.sh| 6 ++
trailer.c| 6 --
trailer.h| 1 +
5 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d6add831c0..a920dd15b1 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -243,6 +243,8 @@ endif::git-rev-list[]
option was given. In same way as to for `only` it can be followed
by an equal sign and explicit value. E.g.,
`%(trailers:only,unfold=true)` unfolds and shows all trailer lines.
+** 'valueonly[=val]': skip over the key part of the trailer line and only
+ show the value part. Also this optionally allows explicit value.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index 541a553ccc..c508357606 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1383,7 +1383,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
opts.filter_data = &filter_list;
opts.only_trailers = 1;
} else if (!match_placeholder_bool_arg(arg,
"only", &arg, &opts.only_trailers) &&
- !match_placeholder_bool_arg(arg,
"unfold", &arg, &opts.unfold))
+ !match_placeholder_bool_arg(arg,
"unfold", &arg, &opts.unfold) &&
+ !match_placeholder_bool_arg(arg,
"valueonly", &arg, &opts.value_only))
break;
}
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 54239290cf..22336c5485 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -667,6 +667,12 @@ test_expect_success 'pretty format
%(trailers:key=foo,only=no) also includes non
test_cmp expect actual
'
+test_expect_success '%(trailers:key=foo,valueonly) shows only value' '
+ git log --no-walk --pretty="format:%(trailers:key=Acked-by,valueonly)"
>actual &&
+ echo "A U Thor " >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'trailer parsing not fooled by --- line' '
git commit --allow-empty -F - <<-\EOF &&
this is the subject
diff --git a/trailer.c b/trailer.c
index d6da555cd7..d0d9e91631 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1150,8 +1150,10 @@ static void format_trailer_info(struct strbuf *out,
if (!opts->filter || opts->filter(&tok,
opts->filter_data)) {
if (opts->unfold)
unfold_value(&val);
-
- strbuf_addf(out, "%s: %s\n", tok.buf, val.buf);
+ if (!opts->value_only)
+ strbuf_addf(out, "%s: ", tok.buf);
+ strbuf_addbuf(out, &val);
+ strbuf_addch(out, '\n');
}
strbuf_release(&tok);
strbuf_release(&val);
diff --git a/trailer.h b/trailer.h
index 5255b676de..06d417fe93 100644
--- a/trailer.h
+++ b/trailer.h
@@ -72,6 +72,7 @@ struct process_trailer_options {
int only_input;
int unfold;
int no_divider;
+ int value_only;
int (*filter)(const struct strbuf *, void *);
void *filter_data;
};
--
2.17.1
[PATCH v4 1/7] doc: group pretty-format.txt placeholders descriptions
The placeholders can be grouped into three kinds:
* literals
* affecting formatting of later placeholders
* expanding to information in commit
Also change the list to a definition list (using '::')
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 235 ---
1 file changed, 125 insertions(+), 110 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 417b638cd8..86d804fe97 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -102,118 +102,133 @@ The title was >>t4119: test autocomputing -p for
traditional diff input.<<
+
The placeholders are:
-- '%H': commit hash
-- '%h': abbreviated commit hash
-- '%T': tree hash
-- '%t': abbreviated tree hash
-- '%P': parent hashes
-- '%p': abbreviated parent hashes
-- '%an': author name
-- '%aN': author name (respecting .mailmap, see linkgit:git-shortlog[1]
- or linkgit:git-blame[1])
-- '%ae': author email
-- '%aE': author email (respecting .mailmap, see
- linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- '%ad': author date (format respects --date= option)
-- '%aD': author date, RFC2822 style
-- '%ar': author date, relative
-- '%at': author date, UNIX timestamp
-- '%ai': author date, ISO 8601-like format
-- '%aI': author date, strict ISO 8601 format
-- '%cn': committer name
-- '%cN': committer name (respecting .mailmap, see
- linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- '%ce': committer email
-- '%cE': committer email (respecting .mailmap, see
- linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- '%cd': committer date (format respects --date= option)
-- '%cD': committer date, RFC2822 style
-- '%cr': committer date, relative
-- '%ct': committer date, UNIX timestamp
-- '%ci': committer date, ISO 8601-like format
-- '%cI': committer date, strict ISO 8601 format
-- '%d': ref names, like the --decorate option of linkgit:git-log[1]
-- '%D': ref names without the " (", ")" wrapping.
-- '%e': encoding
-- '%s': subject
-- '%f': sanitized subject line, suitable for a filename
-- '%b': body
-- '%B': raw body (unwrapped subject and body)
+- Placeholders that expand to a single literal character:
+'%n':: newline
+'%%':: a raw '%'
+'%x00':: print a byte from a hex code
+
+- Placeholders that affect formatting of later placeholders:
+'%Cred':: switch color to red
+'%Cgreen':: switch color to green
+'%Cblue':: switch color to blue
+'%Creset':: reset color
+'%C(...)':: color specification, as described under Values in the
+"CONFIGURATION FILE" section of linkgit:git-config[1]. By
+default, colors are shown only when enabled for log output
+(by `color.diff`, `color.ui`, or `--color`, and respecting
+the `auto` settings of the former if we are going to a
+terminal). `%C(auto,...)` is accepted as a historical
+synonym for the default (e.g., `%C(auto,red)`). Specifying
+`%C(always,...) will show the colors even when color is
+not otherwise enabled (though consider just using
+`--color=always` to enable color for the whole output,
+including this format and anything else git might color).
+`auto` alone (i.e. `%C(auto)`) will turn on auto coloring
+on the next placeholders until the color is switched
+again.
+'%m':: left (`<`), right (`>`) or boundary (`-`) mark
+'%w([[,[,]]])':: switch line wrapping, like the -w option of
+linkgit:git-shortlog[1].
+'%<([,trunc|ltrunc|mtrunc])':: make the next placeholder take at
+ least N columns, padding spaces on
+ the right if necessary. Optionally
+ truncate at the beginning (ltrunc),
+ the middle (mtrunc) or the end
+ (trunc) if the output is longer than
+ N columns. Note that truncating
+ only works correctly with N >= 2.
+'%<|()':: make the next placeholder take at least until Nth
+ columns, padding spaces on the right if necessary
+'%>()', '%>|()':: similar to '%<()', '%<|()' respectively,
+but padding spaces on the left
+'%>>()', '%>>|()':: similar
[PATCH v4 2/7] pretty: allow %(trailers) options with explicit value
In addition to old %(trailers:only) it is now allowed to write
%(trailers:only=yes)
By itself this only gives (the not quite so useful) possibility to have
users change their mind in the middle of a formatting
string (%(trailers:only=true,only=false)). However, it gives users the
opportunity to override defaults from future options.
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 14 ++
pretty.c | 32 +++-
t/t4205-log-pretty-formats.sh| 18 ++
3 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 86d804fe97..d33b072eb2 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -225,10 +225,16 @@ endif::git-rev-list[]
linkgit:git-interpret-trailers[1]. The
`trailers` string may be followed by a colon
and zero or more comma-separated options:
-** 'only': omit non-trailer lines from the trailer block.
-** 'unfold': make it behave as if interpret-trailer's `--unfold`
- option was given. E.g., `%(trailers:only,unfold)` unfolds and
- shows all trailer lines.
+** 'only[=val]': select whether non-trailer lines from the trailer
+ block should be included. The `only` keyword may optionally be
+ followed by an equal sign and one of `true`, `on`, `yes` to omit or
+ `false`, `off`, `no` to show the non-trailer lines. If option is
+ given without value it is enabled. If given multiple times the last
+ value is used.
+** 'unfold[=val]': make it behave as if interpret-trailer's `--unfold`
+ option was given. In same way as to for `only` it can be followed
+ by an equal sign and explicit value. E.g.,
+ `%(trailers:only,unfold=true)` unfolds and shows all trailer lines.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index b83a3ecd23..26efdba73a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1074,6 +1074,31 @@ static int match_placeholder_arg(const char *to_parse,
const char *candidate,
return 0;
}
+static int match_placeholder_bool_arg(const char *to_parse, const char
*candidate,
+ const char **end, int *val)
+{
+ const char *p;
+ if (!skip_prefix(to_parse, candidate, &p))
+ return 0;
+
+ if (match_placeholder_arg(p, "=no", end) ||
+ match_placeholder_arg(p, "=off", end) ||
+ match_placeholder_arg(p, "=false", end)) {
+ *val = 0;
+ return 1;
+ }
+
+ if (match_placeholder_arg(p, "", end) ||
+ match_placeholder_arg(p, "=yes", end) ||
+ match_placeholder_arg(p, "=on", end) ||
+ match_placeholder_arg(p, "=true", end)) {
+ *val = 1;
+ return 1;
+ }
+
+ return 0;
+}
+
static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -1318,11 +1343,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (*arg == ':') {
arg++;
for (;;) {
- if (match_placeholder_arg(arg, "only", &arg))
- opts.only_trailers = 1;
- else if (match_placeholder_arg(arg, "unfold",
&arg))
- opts.unfold = 1;
- else
+ if (!match_placeholder_bool_arg(arg, "only",
&arg, &opts.only_trailers) &&
+ !match_placeholder_bool_arg(arg, "unfold",
&arg, &opts.unfold))
break;
}
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 978a8a66ff..63730a4ec0 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -578,6 +578,24 @@ test_expect_success '%(trailers:only) shows only "key:
value" trailers' '
test_cmp expect actual
'
+test_expect_success '%(trailers:only=yes) shows only "key: value" trailers' '
+ git log --no-walk --pretty=format:"%(trailers:only=yes)" >actual &&
+ grep -v patch.description expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '%(trailers:only=no) shows all trailers' '
+ git log --no-walk --pretty=for
[PATCH v4 7/7] pretty: add support for separator option in %(trailers)
By default trailer lines are terminated by linebreaks ('\n'). By
specifying the new 'separator' option they will instead be separated by
user provided string and have separator semantics rather than terminator
semantics. The separator string can contain the literal formatting codes
%n and %xNN allowing it to be things that are otherwise hard to type
such as %x00, or comma and end-parenthesis which would break parsing.
E.g:
$ git log --pretty='%(trailers:key=Reviewed-by,valueonly,separator=%x00)'
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 9
pretty.c | 10 +
t/t4205-log-pretty-formats.sh| 36
trailer.c| 15 +++--
trailer.h| 1 +
5 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index a920dd15b1..ce087dee80 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -239,6 +239,15 @@ endif::git-rev-list[]
`false`, `off`, `no` to show the non-trailer lines. If option is
given without value it is enabled. If given multiple times the last
value is used.
+** 'separator=': specify a separator inserted between trailer
+ lines. When this option is not given each trailer line is
+ terminated with a line feed character. The string SEP may contain
+ the literal formatting codes described above. To use comma as
+ separator one must use `%x2C` as it would otherwise be parsed as
+ next option. If separator option is given multiple times only the
+ last one is used. E.g., `%(trailers:key=Ticket,separator=%x2C )`
+ shows all trailer lines whose key is "Ticket" separated by a comma
+ and a space.
** 'unfold[=val]': make it behave as if interpret-trailer's `--unfold`
option was given. In same way as to for `only` it can be followed
by an equal sign and explicit value. E.g.,
diff --git a/pretty.c b/pretty.c
index 50d0b5830d..c7609493ee 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1357,6 +1357,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts =
PROCESS_TRAILER_OPTIONS_INIT;
struct string_list filter_list = STRING_LIST_INIT_NODUP;
+ struct strbuf sepbuf = STRBUF_INIT;
size_t ret = 0;
opts.no_divider = 1;
@@ -1376,6 +1377,14 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
opts.filter = format_trailer_match_cb;
opts.filter_data = &filter_list;
opts.only_trailers = 1;
+ } else if (match_placeholder_arg_value(arg,
"separator", &arg, &argval, &arglen)) {
+ char *fmt;
+
+ strbuf_reset(&sepbuf);
+ fmt = xstrndup(argval, arglen);
+ strbuf_expand(&sepbuf, fmt,
strbuf_expand_literal_cb, NULL);
+ free(fmt);
+ opts.separator = &sepbuf;
} else if (!match_placeholder_bool_arg(arg,
"only", &arg, &opts.only_trailers) &&
!match_placeholder_bool_arg(arg,
"unfold", &arg, &opts.unfold) &&
!match_placeholder_bool_arg(arg,
"valueonly", &arg, &opts.value_only))
@@ -1387,6 +1396,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
ret = arg - placeholder + 1;
}
string_list_clear (&filter_list, 0);
+ strbuf_release(&sepbuf);
return ret;
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 22336c5485..282369dac0 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -673,6 +673,42 @@ test_expect_success '%(trailers:key=foo,valueonly) shows
only value' '
test_cmp expect actual
'
+test_expect_success 'pretty format %(trailers:separator) changes separator' '
+ git log --no-walk --pretty=format:"X%(trailers:separator=%x00,unfold)X"
>actual &&
+ printf "XSigned-off-by: A U Thor \0Acked-by: A U
Thor \0[ v2 updated patch description ]\0Signed-off-by: A U
Thor X" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'pretty format %(trailers) combi
Re: [PATCH v3 2/5] pretty: allow showing specific trailers
Junio C Hamano writes:
> I was confused by the "only" stuff.
>
> When you give a key (or two), they cannot possibly name non-trailer
> lines, so while it may be possible to ask "oh, by the way, I also
> want non-trailer lines in addition to signed-off-by and cc lines",
> the value of being able to make such a request is dubious.
>
> The value is dubious, but logically it makes it more consistent with
> the current %(trailers) that lack 'only', which is "oh by the way, I
> also want non-trailer lines in addition to the trailers with
> keyword", to allow a way to countermand the 'only' you flip on by
> default when keywords are given.
Would it feel less inconsistent if it did not set the 'only_trailers'
option?
Now that I look at it again setting 'only_trailers' is more of an
implementation trick/hack to make sure it doesn't take the fast-path in
format_trailer_info (and by documenting it it justifies that hack). If
instead the 'filter' option is checked in the relevant places there
would be no need to mix up 'only' with 'filter'.
That is, do you think something like this should be squashed in?
diff --git a/pretty.c b/pretty.c
index 302e67fa8..f45ccaf51 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1360,7 +1360,6 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
opts.filter = format_trailer_match_cb;
opts.filter_data = &filter_list;
- opts.only_trailers = 1;
} else
break;
}
diff --git a/trailer.c b/trailer.c
index 97c8f2762..07ca2b2c6 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1132,7 +1132,7 @@ static void format_trailer_info(struct strbuf *out,
size_t i;
/* If we want the whole block untouched, we can take the fast path. */
- if (!opts->only_trailers && !opts->unfold) {
+ if (!opts->only_trailers && !opts->unfold && !opts->filter) {
strbuf_add(out, info->trailer_start,
info->trailer_end - info->trailer_start);
return;
@@ -1156,7 +1156,7 @@ static void format_trailer_info(struct strbuf *out,
strbuf_release(&tok);
strbuf_release(&val);
- } else if (!opts->only_trailers) {
+ } else if (!opts->only_trailers && !opts->filter) {
strbuf_addstr(out, trailer);
}
}
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 7548e1d38..ea3cd5b28 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -228,9 +228,9 @@ endif::git-rev-list[]
** 'key=': only show trailers with specified key. Matching is done
case-insensitively and trailing colon is optional. If option is
given multiple times trailer lines matching any of the keys are
- shown. Non-trailer lines in the trailer block are also hidden
- (i.e. 'key' implies 'only'). E.g., `%(trailers:key=Reviewed-by)`
- shows trailer lines with key `Reviewed-by`.
+ shown. Non-trailer lines in the trailer block are also hidden.
+ E.g., `%(trailers:key=Reviewed-by)` shows trailer lines with key
+ `Reviewed-by`.
** 'only': omit non-trailer lines from the trailer block.
** 'unfold': make it behave as if interpret-trailer's `--unfold`
option was given. E.g., `%(trailers:only,unfold)` unfolds and
Re: [PATCH v3 2/5] pretty: allow showing specific trailers
Junio C Hamano writes: > Also, use of 'key=' automatically turns on 'only' as described, and > I tend to agree that it would a convenient default mode (i.e. when > picking certain trailers only with this mechanism, it is likely that > the user is willing to use %(subject) etc. to fill in what was lost > by the implicit use of 'only'), but at the same time, it makes me > wonder if we need to have a way to countermand an 'only' (or > 'unfold' for that matter) that was given earlier, e.g. > > %(trailers:key=signed-off-by,only=no) > > Thanks. I'm not sure what that would mean. The non-trailer lines in the trailer block doesn't match the key. Take this commit as an example: $ git show -s --pretty=format:'%(trailers)' b4d065df03049bacfbc40467b60b13e804b7d289 Helped-by: Jeff King [jc: took idea and log message from peff] Signed-off-by: Junio C Hamano With 'only' it shows: $ git show -s --pretty=format:'%(trailers:only)' b4d065df03049bacfbc40467b60b13e804b7d289 Helped-by: Jeff King Signed-off-by: Junio C Hamano Now with a "key=signed-off-by" option I would imagine that as adding a "| grep -i '^signed-off-by:'" to the end. In both cases (with and without 'only') that would give the same result: "Signed-off-by: Junio C Hamano " anders
[PATCH v3 0/5] %(trailers) improvements in pretty format
Updated since v2: * Allow trailing colon in 'key=' argument * Clarify documentation on how matching is done * Rename option to "valueonly" * Make trailing matching a callback function * Avoid copying match string * Simplify generation of "expected" in tests * Rename function to strbuf_expand_literal_cb * cocci suggested fixes Anders Waldenborg (5): pretty: single return path in %(trailers) handling pretty: allow showing specific trailers pretty: add support for "valueonly" option in %(trailers) strbuf: separate callback for strbuf_expand:ing literals pretty: add support for separator option in %(trailers) Documentation/pretty-formats.txt | 26 --- pretty.c | 68 ++-- strbuf.c | 21 + strbuf.h | 8 t/t4205-log-pretty-formats.sh| 78 trailer.c| 25 -- trailer.h| 4 ++ 7 files changed, 206 insertions(+), 24 deletions(-) -- 2.17.1
[PATCH v3 1/5] pretty: single return path in %(trailers) handling
No functional change intended.
This change may not seem useful on its own, but upcoming commits will do
memory allocation in there, and a single return path makes deallocation
easier.
Signed-off-by: Anders Waldenborg
---
pretty.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/pretty.c b/pretty.c
index b83a3ecd2..aa03d5b23 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1312,6 +1312,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts =
PROCESS_TRAILER_OPTIONS_INIT;
+ size_t ret = 0;
opts.no_divider = 1;
@@ -1328,8 +1329,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
}
if (*arg == ')') {
format_trailers_from_commit(sb, msg + c->subject_off,
&opts);
- return arg - placeholder + 1;
+ ret = arg - placeholder + 1;
}
+ return ret;
}
return 0; /* unknown placeholder */
--
2.17.1
[PATCH v3 4/5] strbuf: separate callback for strbuf_expand:ing literals
Expanding '%n' and '%xNN' is generic functionality, so extract that from
the pretty.c formatter into a callback that can be reused.
No functional change intended
Signed-off-by: Anders Waldenborg
---
pretty.c | 16 +---
strbuf.c | 21 +
strbuf.h | 8
3 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/pretty.c b/pretty.c
index 2e99f2418..819c5c50a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1094,9 +1094,13 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
const char *msg = c->message;
struct commit_list *p;
const char *arg;
- int ch;
+ size_t res;
/* these are independent of the commit */
+ res = strbuf_expand_literal_cb(sb, placeholder, NULL);
+ if (res)
+ return res;
+
switch (placeholder[0]) {
case 'C':
if (starts_with(placeholder + 1, "(auto)")) {
@@ -1115,16 +1119,6 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
*/
return ret;
}
- case 'n': /* newline */
- strbuf_addch(sb, '\n');
- return 1;
- case 'x':
- /* %x00 == NUL, %x0a == LF, etc. */
- ch = hex2chr(placeholder + 1);
- if (ch < 0)
- return 0;
- strbuf_addch(sb, ch);
- return 3;
case 'w':
if (placeholder[1] == '(') {
unsigned long width = 0, indent1 = 0, indent2 = 0;
diff --git a/strbuf.c b/strbuf.c
index f6a6cf78b..78eecd29f 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -380,6 +380,27 @@ void strbuf_expand(struct strbuf *sb, const char *format,
expand_fn_t fn,
}
}
+size_t strbuf_expand_literal_cb(struct strbuf *sb,
+ const char *placeholder,
+ void *context)
+{
+ int ch;
+
+ switch (placeholder[0]) {
+ case 'n': /* newline */
+ strbuf_addch(sb, '\n');
+ return 1;
+ case 'x':
+ /* %x00 == NUL, %x0a == LF, etc. */
+ ch = hex2chr(placeholder + 1);
+ if (ch < 0)
+ return 0;
+ strbuf_addch(sb, ch);
+ return 3;
+ }
+ return 0;
+}
+
size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
void *context)
{
diff --git a/strbuf.h b/strbuf.h
index fc40873b6..52e44c9ab 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -320,6 +320,14 @@ void strbuf_expand(struct strbuf *sb,
expand_fn_t fn,
void *context);
+/**
+ * Used as callback for `strbuf_expand` to only expand literals
+ * (i.e. %n and %xNN). The context argument is ignored.
+ */
+size_t strbuf_expand_literal_cb(struct strbuf *sb,
+ const char *placeholder,
+ void *context);
+
/**
* Used as callback for `strbuf_expand()`, expects an array of
* struct strbuf_expand_dict_entry as context, i.e. pairs of
--
2.17.1
[PATCH v3 5/5] pretty: add support for separator option in %(trailers)
By default trailer lines are terminated by linebreaks ('\n'). By
specifying the new 'separator' option they will instead be separated by
user provided string and have separator semantics rather than terminator
semantics. The separator string can contain the literal formatting codes
%n and %xNN allowing it to be things that are otherwise hard to type as
%x00, or comma and end-parenthesis which would break parsing.
E.g:
$ git log --pretty='%(trailers:key=Reviewed-by,valueonly,separator=%x00)'
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 13 +---
pretty.c | 15 +
t/t4205-log-pretty-formats.sh| 36
trailer.c| 15 +++--
trailer.h| 1 +
5 files changed, 75 insertions(+), 5 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 8cc8c3f9f..30e238338 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -218,9 +218,16 @@ endif::git-rev-list[]
is given multiple times only last one is used.
** 'valueonly': skip over the key part of the trailer and only show
the its value part.
- ** Examples: `%(trailers:only,unfold)` unfolds and shows all trailer
- lines, `%(trailers:key=Reviewed-by,unfold)` unfolds and shows
- trailer lines with key `Reviewed-by`.
+ ** 'separator=': specifying an alternative separator than the
+ default line feed character. SEP may can contain the literal
+ formatting codes %n and %xNN allowing it to contain characters
+ that are hard to type such as %x00, or comma and end-parenthesis
+ which would break parsing. If option is given multiple times only
+ the last one is used.
+ ** Examples: `%(trailers:only,unfold,separator=%x00)` unfolds and
+ shows all trailer lines separated by NUL character,
+ `%(trailers:key=Reviewed-by,unfold)` unfolds and shows trailer
+ lines with key `Reviewed-by`.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index 819c5c50a..5b22a7237 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1318,6 +1318,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts =
PROCESS_TRAILER_OPTIONS_INIT;
struct format_trailer_match_data filter_data;
+ struct strbuf sepbuf = STRBUF_INIT;
size_t ret = 0;
opts.no_divider = 1;
@@ -1348,6 +1349,19 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
arg = end;
if (*arg == ',')
arg++;
+ } else if (skip_prefix(arg, "separator=",
&arg)) {
+ size_t seplen = strcspn(arg, ",)");
+ char *fmt;
+
+ strbuf_reset(&sepbuf);
+ fmt = xstrndup(arg, seplen);
+ strbuf_expand(&sepbuf, fmt,
strbuf_expand_literal_cb, NULL);
+ free(fmt);
+ opts.separator = &sepbuf;
+
+ arg += seplen;
+ if (*arg == ',')
+ arg++;
} else
break;
}
@@ -1356,6 +1370,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
format_trailers_from_commit(sb, msg + c->subject_off,
&opts);
ret = arg - placeholder + 1;
}
+ strbuf_release(&sepbuf);
return ret;
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 095208d6b..562b56dda 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -640,6 +640,42 @@ test_expect_success '%(trailers:key=foo,valueonly) shows
only value' '
test_cmp expect actual
'
+test_expect_success 'pretty format %(trailers:separator) changes separator' '
+ git log --no-walk --pretty=format:"X%(trailers:separator=%x00,unfold)X"
>actual &&
+ printf "XSigned-off-by: A U Thor \0Acked-by: A U
Thor \0[ v2 updated patch description ]\0Signed-off-by: A U
Thor X" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_su
[PATCH v3 3/5] pretty: add support for "valueonly" option in %(trailers)
With the new "key=" option to %(trailers) it often makes little sense to
show the key, as it by definition already is know which trailer is
printed there. This new "valueonly" option makes it omit the key when
printing trailers.
E.g.:
$ git show -s --pretty='%s%n%(trailers:key=Signed-off-by,valueonly)' 88182
will show:
> upload-pack: fix broken if/else chain in config callback
> Jeff King
> Junio C Hamano
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 2 ++
pretty.c | 2 ++
t/t4205-log-pretty-formats.sh| 6 ++
trailer.c| 6 --
trailer.h| 1 +
5 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 75c2e838d..8cc8c3f9f 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -216,6 +216,8 @@ endif::git-rev-list[]
** 'key=': only show trailers with specified key. Matching is
done case-insensitively and trailing colon is optional. If option
is given multiple times only last one is used.
+ ** 'valueonly': skip over the key part of the trailer and only show
+ the its value part.
** Examples: `%(trailers:only,unfold)` unfolds and shows all trailer
lines, `%(trailers:key=Reviewed-by,unfold)` unfolds and shows
trailer lines with key `Reviewed-by`.
diff --git a/pretty.c b/pretty.c
index aaedc8447..2e99f2418 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1335,6 +1335,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
opts.only_trailers = 1;
else if (match_placeholder_arg(arg, "unfold",
&arg))
opts.unfold = 1;
+ else if (match_placeholder_arg(arg,
"valueonly", &arg))
+ opts.value_only = 1;
else if (skip_prefix(arg, "key=", &arg)) {
const char *end = arg + strcspn(arg,
",)");
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index aba7ba206..095208d6b 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -634,6 +634,12 @@ test_expect_success '%(trailers:key=foo,unfold) properly
unfolds' '
test_cmp expect actual
'
+test_expect_success '%(trailers:key=foo,valueonly) shows only value' '
+ git log --no-walk --pretty="format:%(trailers:key=Acked-by,valueonly)"
>actual &&
+ echo "A U Thor " >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'trailer parsing not fooled by --- line' '
git commit --allow-empty -F - <<-\EOF &&
this is the subject
diff --git a/trailer.c b/trailer.c
index 97c8f2762..662c7ff03 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1150,8 +1150,10 @@ static void format_trailer_info(struct strbuf *out,
if (!opts->filter || opts->filter(&tok,
opts->filter_data)) {
if (opts->unfold)
unfold_value(&val);
-
- strbuf_addf(out, "%s: %s\n", tok.buf, val.buf);
+ if (!opts->value_only)
+ strbuf_addf(out, "%s: ", tok.buf);
+ strbuf_addbuf(out, &val);
+ strbuf_addch(out, '\n');
}
strbuf_release(&tok);
strbuf_release(&val);
diff --git a/trailer.h b/trailer.h
index 5255b676d..06d417fe9 100644
--- a/trailer.h
+++ b/trailer.h
@@ -72,6 +72,7 @@ struct process_trailer_options {
int only_input;
int unfold;
int no_divider;
+ int value_only;
int (*filter)(const struct strbuf *, void *);
void *filter_data;
};
--
2.17.1
[PATCH v3 2/5] pretty: allow showing specific trailers
Adds a new "key=X" option to "%(trailers)" which will cause it to only
print trailers lines which match the specified key.
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 17 +--
pretty.c | 31 ++-
t/t4205-log-pretty-formats.sh| 36
trailer.c| 8 ---
trailer.h| 2 ++
5 files changed, 84 insertions(+), 10 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 417b638cd..75c2e838d 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -207,13 +207,18 @@ endif::git-rev-list[]
than given and there are spaces on its left, use those spaces
- '%><()', '%><|()': similar to '%<()', '%<|()'
respectively, but padding both sides (i.e. the text is centered)
-- %(trailers[:options]): display the trailers of the body as interpreted
+- '%(trailers[:options])': display the trailers of the body as interpreted
by linkgit:git-interpret-trailers[1]. The `trailers` string may be
- followed by a colon and zero or more comma-separated options. If the
- `only` option is given, omit non-trailer lines from the trailer block.
- If the `unfold` option is given, behave as if interpret-trailer's
- `--unfold` option was given. E.g., `%(trailers:only,unfold)` to do
- both.
+ followed by a colon and zero or more comma-separated options:
+ ** 'only': omit non-trailer lines from the trailer block.
+ ** 'unfold': make it behave as if interpret-trailer's `--unfold`
+ option was given.
+ ** 'key=': only show trailers with specified key. Matching is
+ done case-insensitively and trailing colon is optional. If option
+ is given multiple times only last one is used.
+ ** Examples: `%(trailers:only,unfold)` unfolds and shows all trailer
+ lines, `%(trailers:key=Reviewed-by,unfold)` unfolds and shows
+ trailer lines with key `Reviewed-by`.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index aa03d5b23..aaedc8447 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1074,6 +1074,17 @@ static int match_placeholder_arg(const char *to_parse,
const char *candidate,
return 0;
}
+struct format_trailer_match_data {
+ const char *trailer;
+ size_t trailer_len;
+};
+
+static int format_trailer_match_cb(const struct strbuf *sb, void *ud)
+{
+ struct format_trailer_match_data *data = ud;
+ return data->trailer_len == sb->len && !strncasecmp (data->trailer,
sb->buf, sb->len);
+}
+
static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -1312,6 +1323,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts =
PROCESS_TRAILER_OPTIONS_INIT;
+ struct format_trailer_match_data filter_data;
size_t ret = 0;
opts.no_divider = 1;
@@ -1323,7 +1335,24 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
opts.only_trailers = 1;
else if (match_placeholder_arg(arg, "unfold",
&arg))
opts.unfold = 1;
- else
+ else if (skip_prefix(arg, "key=", &arg)) {
+ const char *end = arg + strcspn(arg,
",)");
+
+ filter_data.trailer = arg;
+ filter_data.trailer_len = end - arg;
+
+ if (filter_data.trailer_len &&
+
filter_data.trailer[filter_data.trailer_len - 1] == ':')
+ filter_data.trailer_len--;
+
+ opts.filter = format_trailer_match_cb;
+ opts.filter_data = &filter_data;
+ opts.only_trailers = 1;
+
+ arg = end;
+ if (*arg == ',')
+ arg++;
+ } else
break;
}
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 978a8a66f..aba7ba206 100755
Re: [PATCH v2 5/5] pretty: add support for separator option in %(trailers)
Junio C Hamano writes:
> Anders Waldenborg writes:
>
>> @@ -1352,6 +1353,17 @@ static size_t format_commit_one(struct strbuf *sb, /*
>> in UTF-8 */
>> arg++;
>>
>> opts.only_trailers = 1;
>> +} else if (skip_prefix(arg, "separator=",
>> &arg)) {
>> +size_t seplen = strcspn(arg, ",)");
>> +strbuf_reset(&sepbuf);
>> +char *fmt = xstrndup(arg, seplen);
>> +strbuf_expand(&sepbuf, fmt,
>> format_fundamental, NULL);
>
> This somehow feels akin to using end-user supplied param to printf(3)
> as its format argument e.g.
>
> int main(int ac, char *av) {
> printf(av[1]);
> return 0;
> }
>
> which is not a good idea. Is there a mechanism with which we can
> ensure that the separator= specification will never come from
> potentially malicious sources (e.g. not used to show things on webpage
> allowing random folks who access he site to supply custom format)?
I can't see a case where this could add anything that isn't already
possible.
AFAICU strbuf_expand doesn't suffer from the worst things that printf(3)
suffers from wrt untrusted format string (i.e no printf style %n which
can write to memory, and no vaargs on stack which allows leaking random
stuff).
The separator option is part of the full format string. If a malicious
user can specify that, they can't really do anything new, as the
separator only can expand %n and %xNN, which they already can do in the
full string.
But maybe I'm missing something?
Re: [PATCH v2 4/5] pretty: extract fundamental placeholders to separate function
Junio C Hamano writes: > I do not think "fundamental" is the best name for this, but I agree > that it would be useful to split the helpers into one that is > "constant across commits" and the other one that is "per commit". Any suggestions for a better name? standalone? simple? invariant? free?
Re: [PATCH v2 2/5] pretty: allow showing specific trailers
Eric Sunshine writes:
> Should the code tolerate a trailing colon? (Genuine question; it's
> easy to do and would be more user-friendly.)
I would make sense to allow the trailing colon, it is easy enough to
just strip that away when reading the argument.
However I'm not sure how that would fit together with the possibility to
later lifting it to a regexp, hard to strip a trailing colon from a
regexp in a generic way.
> What happens if 'key=...' is specified multiple times?
My first thought was to simply disallow that. But that seemed hard to
fit into current model where errors just means don't expand.
I would guess that most useful and intuitive to user would be to handle
multiple key arguments by showing any of those keys.
> Thinking further on the last two points, should be a regular expression?
It probably would make sense. I can see how the regexp '^.*-by$' would
be useful (but glob style matching would suffice in that case).
Also handling multi-matching with an alternation group would be elegant
%(trailers:key="(A|B)"). Except for the fact that the parser would need to
understand some kind of quoting, which seems like an major undertaking.
I guess having it as a regular exception would also mean that it needs
to get some way to cache the re so it is compiled once, and not for each
expansion.
>
>> + free(opts.filter_key);
>
> If I understand correctly, this is making a copy of so that it
> will be NUL-terminated since the code added to trailer.c uses a simple
> strcasecmp() to match it. Would it make sense to avoid the copy by
> adding fields 'opts.filter_key' and 'opts.filter_key_len' and using
> strncasecmp() instead? (Genuine question; not necessarily a request
> for change.)
I'm also not very happy about that copy.
Just using strncasecmp would cause it to be prefix match, no?
But if changing matching semantics to handle multiple key options to
something else I'm thinking opts.filter_key would be replaced with a
opts.filter callback function, and that part would need to be rewritten
anyway.
>
>> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
>> @@ -598,6 +598,51 @@ test_expect_success ':only and :unfold work together' '
>> +test_expect_success 'pretty format %(trailers:key=foo) shows that trailer' '
>> + git log --no-walk --pretty="%(trailers:key=Acked-by)" >actual &&
>> + {
>> + echo "Acked-by: A U Thor " &&
>> + echo
>> + } >expect &&
>> + test_cmp expect actual
>> +'
>
> I guess these new tests are modeled after one or two existing tests
> which use a series of 'echo' statements
I guess I could change it to "--pretty=format:%(trailers:key=Acked-by)"
to get separator semantics and avoid that extra blank line, making it
simpler.
Re: [PATCH v2 0/5] %(trailers) improvements in pretty format
Eric Sunshine writes: > If "key" is for including particular trailers, intuition might lead > people to think that "nokey" is for excluding certain trailers. > Perhaps a different name for "nokey", such as "valueonly" or > "stripkey", would be better. Good point. I guess "valueonly" would be preferred as it says what it shows, not what it hides.
[PATCH v2 3/5] pretty: add support for "nokey" option in %(trailers)
With the new "key=" option to %trailers it often makes little sense to
show the key, as it by definition already is know which trailer is
printed there. This new "nokey" option makes it omit key trailer key
when printing trailers.
E.g.:
$ git show -s --pretty='%s%n%(trailers:key=Signed-off-by,nokey)' 88182
will show:
> upload-pack: fix broken if/else chain in config callback
> Jeff King
> Junio C Hamano
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 11 ++-
pretty.c | 2 ++
t/t4205-log-pretty-formats.sh| 9 +
trailer.c| 6 --
trailer.h| 1 +
5 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 8326fc45e..e115e355d 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -212,11 +212,12 @@ endif::git-rev-list[]
followed by a colon and zero or more comma-separated options. The
allowed options are `only` which omits non-trailer lines from the
trailer block, `unfold` to make it behave as if interpret-trailer's
- `--unfold` option was given, and `key=T` to only show trailers with
- specified key (matching is done
- case-insensitively). E.g. `%(trailers:only,unfold)` unfolds and
- shows all trailer lines, `%(trailers:key=Reviewed-by,unfold)`
- unfolds and shows trailer lines with key `Reviewed-by`.
+ `--unfold` option was given, `key=T` to only show trailers with
+ specified key (matching is done case-insensitively), and `nokey`
+ which makes it skip over the key part of the trailer and only show
+ value. E.g. `%(trailers:only,unfold)` unfolds and shows all trailer
+ lines, `%(trailers:key=Reviewed-by,unfold)` unfolds and shows
+ trailer lines with key `Reviewed-by`.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index cdca9dce2..f87ba4f18 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1323,6 +1323,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
opts.only_trailers = 1;
else if (match_placeholder_arg(arg, "unfold",
&arg))
opts.unfold = 1;
+ else if (match_placeholder_arg(arg, "nokey",
&arg))
+ opts.no_key = 1;
else if (skip_prefix(arg, "key=", &arg)) {
const char *end = arg + strcspn(arg,
",)");
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 0f5207242..e7de3b18a 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -643,6 +643,15 @@ test_expect_success '%(trailers:key=foo,unfold) properly
unfolds' '
test_cmp expect actual
'
+test_expect_success '%(trailers:key=foo,nokey) shows only value' '
+ git log --no-walk --pretty="%(trailers:key=Acked-by,nokey)" >actual &&
+ {
+ echo "A U Thor " &&
+ echo
+ } >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'trailer parsing not fooled by --- line' '
git commit --allow-empty -F - <<-\EOF &&
this is the subject
diff --git a/trailer.c b/trailer.c
index cbbb553e4..4f19c34cb 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1150,8 +1150,10 @@ static void format_trailer_info(struct strbuf *out,
if (!opts->filter_key || !strcasecmp (tok.buf,
opts->filter_key)) {
if (opts->unfold)
unfold_value(&val);
-
- strbuf_addf(out, "%s: %s\n", tok.buf, val.buf);
+ if (opts->no_key)
+ strbuf_addf(out, "%s\n", val.buf);
+ else
+ strbuf_addf(out, "%s: %s\n", tok.buf,
val.buf);
}
strbuf_release(&tok);
strbuf_release(&val);
diff --git a/trailer.h b/trailer.h
index d052d02ae..83de87ee9 100644
--- a/trailer.h
+++ b/trailer.h
@@ -72,6 +72,7 @@ struct process_trailer_options {
int only_input;
int unfold;
int no_divider;
+ int no_key;
char *filter_key;
};
--
2.17.1
[PATCH v2 2/5] pretty: allow showing specific trailers
Adds a new "key=X" option to "%(trailers)" which will cause it to only
print trailers lines which matches the specified key.
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 13 +
pretty.c | 15 ++-
t/t4205-log-pretty-formats.sh| 45
trailer.c| 8 +++---
trailer.h| 1 +
5 files changed, 73 insertions(+), 9 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 417b638cd..8326fc45e 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -209,11 +209,14 @@ endif::git-rev-list[]
respectively, but padding both sides (i.e. the text is centered)
- %(trailers[:options]): display the trailers of the body as interpreted
by linkgit:git-interpret-trailers[1]. The `trailers` string may be
- followed by a colon and zero or more comma-separated options. If the
- `only` option is given, omit non-trailer lines from the trailer block.
- If the `unfold` option is given, behave as if interpret-trailer's
- `--unfold` option was given. E.g., `%(trailers:only,unfold)` to do
- both.
+ followed by a colon and zero or more comma-separated options. The
+ allowed options are `only` which omits non-trailer lines from the
+ trailer block, `unfold` to make it behave as if interpret-trailer's
+ `--unfold` option was given, and `key=T` to only show trailers with
+ specified key (matching is done
+ case-insensitively). E.g. `%(trailers:only,unfold)` unfolds and
+ shows all trailer lines, `%(trailers:key=Reviewed-by,unfold)`
+ unfolds and shows trailer lines with key `Reviewed-by`.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index aa03d5b23..cdca9dce2 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1323,7 +1323,19 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
opts.only_trailers = 1;
else if (match_placeholder_arg(arg, "unfold",
&arg))
opts.unfold = 1;
- else
+ else if (skip_prefix(arg, "key=", &arg)) {
+ const char *end = arg + strcspn(arg,
",)");
+
+ if (opts.filter_key)
+ free(opts.filter_key);
+
+ opts.filter_key = xstrndup(arg, end -
arg);
+ arg = end;
+ if (*arg == ',')
+ arg++;
+
+ opts.only_trailers = 1;
+ } else
break;
}
}
@@ -1331,6 +1343,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
format_trailers_from_commit(sb, msg + c->subject_off,
&opts);
ret = arg - placeholder + 1;
}
+ free(opts.filter_key);
return ret;
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 978a8a66f..0f5207242 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -598,6 +598,51 @@ test_expect_success ':only and :unfold work together' '
test_cmp expect actual
'
+test_expect_success 'pretty format %(trailers:key=foo) shows that trailer' '
+ git log --no-walk --pretty="%(trailers:key=Acked-by)" >actual &&
+ {
+ echo "Acked-by: A U Thor " &&
+ echo
+ } >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'pretty format %(trailers:key=foo) is case insensitive' '
+ git log --no-walk --pretty="%(trailers:key=AcKed-bY)" >actual &&
+ {
+ echo "Acked-by: A U Thor " &&
+ echo
+ } >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '%(trailers:key=nonexistant) becomes empty' '
+ git log --no-walk --pretty="x%(trailers:key=Nacked-by)x" >actual &&
+ {
+ echo "xx"
+ } >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '%(trailers:key=foo) handles multiple lines even if
folded' '
+ git log --no-walk --pretty="%(trailers:key=Signed-Off-by)" >actual &&
+ {
+
[PATCH v2 4/5] pretty: extract fundamental placeholders to separate function
No functional change intended
Signed-off-by: Anders Waldenborg
---
pretty.c | 37 ++---
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/pretty.c b/pretty.c
index f87ba4f18..9fdddce9d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1074,6 +1074,27 @@ static int match_placeholder_arg(const char *to_parse,
const char *candidate,
return 0;
}
+static size_t format_fundamental(struct strbuf *sb, /* in UTF-8 */
+const char *placeholder,
+void *context)
+{
+ int ch;
+
+ switch (placeholder[0]) {
+ case 'n': /* newline */
+ strbuf_addch(sb, '\n');
+ return 1;
+ case 'x':
+ /* %x00 == NUL, %x0a == LF, etc. */
+ ch = hex2chr(placeholder + 1);
+ if (ch < 0)
+ return 0;
+ strbuf_addch(sb, ch);
+ return 3;
+ }
+ return 0;
+}
+
static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -1083,9 +1104,13 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
const char *msg = c->message;
struct commit_list *p;
const char *arg;
- int ch;
+ size_t res;
/* these are independent of the commit */
+ res = format_fundamental(sb, placeholder, NULL);
+ if (res)
+ return res;
+
switch (placeholder[0]) {
case 'C':
if (starts_with(placeholder + 1, "(auto)")) {
@@ -1104,16 +1129,6 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
*/
return ret;
}
- case 'n': /* newline */
- strbuf_addch(sb, '\n');
- return 1;
- case 'x':
- /* %x00 == NUL, %x0a == LF, etc. */
- ch = hex2chr(placeholder + 1);
- if (ch < 0)
- return 0;
- strbuf_addch(sb, ch);
- return 3;
case 'w':
if (placeholder[1] == '(') {
unsigned long width = 0, indent1 = 0, indent2 = 0;
--
2.17.1
[PATCH v2 5/5] pretty: add support for separator option in %(trailers)
By default trailer lines are terminated by linebreaks ('\n'). By
specifying the new 'separator' option they will instead be separated by
user provided string and have separator semantics rather than terminator
semantics. The separator string can contain the fundamental formatting
codes %n and %xNN allowing it to be things that are otherwise hard to
type as %x00, or command and end-parenthesis which would break parsing.
E.g:
$ git log --pretty='%(trailers:key=Reviewed-by,nokey,separator=%x00)'
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 13 -
pretty.c | 13 +
t/t4205-log-pretty-formats.sh| 6 ++
trailer.c| 20 +---
trailer.h| 1 +
5 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index e115e355d..3312850e6 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -213,11 +213,14 @@ endif::git-rev-list[]
allowed options are `only` which omits non-trailer lines from the
trailer block, `unfold` to make it behave as if interpret-trailer's
`--unfold` option was given, `key=T` to only show trailers with
- specified key (matching is done case-insensitively), and `nokey`
- which makes it skip over the key part of the trailer and only show
- value. E.g. `%(trailers:only,unfold)` unfolds and shows all trailer
- lines, `%(trailers:key=Reviewed-by,unfold)` unfolds and shows
- trailer lines with key `Reviewed-by`.
+ specified key (matching is done case-insensitively), `nokey` which
+ makes it skip over the key part of the trailer and only show value
+ and `separator` which allows specifying an alternative separator
+ than the default line
+ break. E.g. `%(trailers:only,unfold,separator=%x00)` unfolds and
+ shows all trailer lines separated by NUL character,
+ `%(trailers:key=Reviewed-by,unfold)` unfolds and shows trailer lines
+ with key `Reviewed-by`.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index 9fdddce9d..f73a2b0dc 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1327,6 +1327,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts =
PROCESS_TRAILER_OPTIONS_INIT;
+ struct strbuf sepbuf = STRBUF_INIT;
size_t ret = 0;
opts.no_divider = 1;
@@ -1352,6 +1353,17 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
arg++;
opts.only_trailers = 1;
+ } else if (skip_prefix(arg, "separator=",
&arg)) {
+ size_t seplen = strcspn(arg, ",)");
+ strbuf_reset(&sepbuf);
+ char *fmt = xstrndup(arg, seplen);
+ strbuf_expand(&sepbuf, fmt,
format_fundamental, NULL);
+ free(fmt);
+ opts.separator = &sepbuf;
+
+ arg += seplen;
+ if (*arg == ',')
+ arg++;
} else
break;
}
@@ -1360,6 +1372,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
format_trailers_from_commit(sb, msg + c->subject_off,
&opts);
ret = arg - placeholder + 1;
}
+ strbuf_release (&sepbuf);
free(opts.filter_key);
return ret;
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index e7de3b18a..71218d22e 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -652,6 +652,12 @@ test_expect_success '%(trailers:key=foo,nokey) shows only
value' '
test_cmp expect actual
'
+test_expect_success 'pretty format %(trailers:separator) changes separator' '
+ git log --no-walk --pretty=format:"X%(trailers:separator=%x00,unfold)X"
>actual &&
+ printf "XSigned-off-by: A U Thor \0Acked-by: A U
Thor \0[ v2 updated patch description ]\0Signed-off-by: A U
Thor X" >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'trailer parsing not fooled by --- line' '
git commit --allow-empty -F - <<-\E
[PATCH v2 1/5] pretty: single return path in %(trailers) handling
No functional change intended.
This change may not seem useful on its own, but upcoming commits will do
memory allocation in there, and a single return path makes deallocation
easier.
Signed-off-by: Anders Waldenborg
---
pretty.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/pretty.c b/pretty.c
index b83a3ecd2..aa03d5b23 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1312,6 +1312,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts =
PROCESS_TRAILER_OPTIONS_INIT;
+ size_t ret = 0;
opts.no_divider = 1;
@@ -1328,8 +1329,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
}
if (*arg == ')') {
format_trailers_from_commit(sb, msg + c->subject_off,
&opts);
- return arg - placeholder + 1;
+ ret = arg - placeholder + 1;
}
+ return ret;
}
return 0; /* unknown placeholder */
--
2.17.1
[PATCH v2 0/5] %(trailers) improvements in pretty format
This adds support for three new options to %(trailers): * key -- show only trailers with specified key * nokey -- don't show key part of trailers * separator -- allow specifying custom separator between trailers Anders Waldenborg (5): pretty: single return path in %(trailers) handling pretty: allow showing specific trailers pretty: add support for "nokey" option in %(trailers) pretty: extract fundamental placeholders to separate function pretty: add support for separator option in %(trailers) Documentation/pretty-formats.txt | 17 +--- pretty.c | 71 ++-- t/t4205-log-pretty-formats.sh| 60 +++ trailer.c| 28 ++--- trailer.h| 3 ++ 5 files changed, 156 insertions(+), 23 deletions(-) -- 2.17.1
Re: [PATCH] pretty: Add %(trailer:X) to display single trailer
Jeff King writes:
> On the other hand, if the rule were not "this affects the next
> placeholder" but had a true ending mark, then we could make a real
> parse-tree out of it, and format chunks of placeholders. E.g.:
>
> %(format:lpad=30,filename)%(subject) %(authordate)%(end)
>
> would pad and format the whole string with two placeholders. I know that
> going down this road eventually involves reinventing XML, but I think
> having an actual tree structure may not be an unreasonable thing to
> shoot for.
Yes. I'm thinking that with [] for formatting specifiers and () for
placeholders, {} would be available for nesting. E.g:
%[lpad=30,mangle]{%(subject) %ad%}
> My main concern for now is to avoid introducing new
> syntax that we'll be stuck with forever, even though it may later become
> redundant (or worse, create parsing ambiguities).
Agreed.
I'm planning to work on the initial "trailer:key=" part later this
week. Maybe I can play around with different formatting options and see
how it affects the parser.
Re: [PATCH] pretty: Add %(trailer:X) to display single trailer
On Mon, Oct 29, 2018 at 3:14 PM Jeff King wrote:
> Junio's review already covered my biggest question, which is why not
> something like "%(trailers:key=ticket)". And likewise making things like
> comma-separation options.
Jeff, Junio,
thanks!
Your questions pretty much matches what I (and a colleague I discussed
this with before posting) was concerned about.
My first try actually had it as an option to "trailers". But it got a
bit messy with the argument parsing, and the fact that there was a
fast path making it work when only specified. I did not want to spend
lot of time reworking fixing that before I had some feedback, so I
went for a smallest possible patch to float the idea with (a patch is
worth a 1000 words).
I'll start by reworking my patch to handle %(trailers:key=X) (I'll
assume keys never contain ')' or ','), and ignore any formatting until
the way forward there is decided (see below).
> But my second question is whether we want to provide something more
> flexible than the always-parentheses that "%d" provides. That has been a
> problem in the past when people want to format the decoration in some
> other way.
Maybe just like +/-/space can be used directly after %, a () pair can
be allowed.. E.g "%d" would just be an alias for "%()D", and for
trailers it would be something like "%()(trailers:key=foo)"
There is another special cased placeholder %f (sanitized subject line,
suitable for a filename). Which also could be changed to be a format
specifiier, allowing sanitize any thing, e.g "%!an" for sanitized
author name.
Is even the linebreak to commaseparation a generic thing?
"% ,()(trailers:key=Ticket)" it starts go look a bit silly.
Then there are the padding modifiers. %<() %<|(). They operate on next
placeholder. "%<(10)%s" Is that a better syntax?
"%()%(trailers:key=Ticket,comma)"
I can also imagine moving all these modifiers into a generic modifier
syntax in brackets (and keeping old for backwards compat)
%[lpad=10,ltrunc=10]s == %<(10,trunc)%s
%[nonempty-prefix="%n"]GS == %+GS
%[nonempty-prefix=" (",nonempty-suffix=")"]D == %d
Which would mean something like this for tickets thing:
%[nonempty-prefix="
(Tickets:",nonempty-suffix=")",commaseparatelines](trailers:key=Ticket,nokey)
which is kinda verbose.
> We have formatting magic for "if this thing is non-empty, then show this
> prefix" in the for-each-ref formatter, but I'm not sure that we do in
> the commit pretty-printer beyond "% ". I wonder if we could/should add a
> a placeholder for "if this thing is non-empty, put in a space and
> enclose it in parentheses".
Would there be any interest in consolidating those formatters? Even
though they are totally separate beasts today. I think having all
attributes available on long form (e.g "%(authorname)") in addition to
existing short forms in pretty-formatter would make sense.
anders
[PATCH] pretty: Add %(trailer:X) to display single trailer
This new format placeholder allows displaying only a single
trailer. The formatting done is similar to what is done for
--decorate/%d using parentheses and comma separation.
It's intended use is for things like ticket references in trailers.
So with a commit with a message like:
> Some good commit
>
> Ticket: XYZ-123
running:
$ git log --pretty="%H %s% (trailer:Ticket)"
will give:
> 123456789a Some good commit (Ticket: XYZ-123)
Signed-off-by: Anders Waldenborg
---
Documentation/pretty-formats.txt | 4
pretty.c | 16 +
t/t4205-log-pretty-formats.sh| 40
trailer.c| 18 --
trailer.h| 1 +
5 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 6109ef09aa..a46d0c0717 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -211,6 +211,10 @@ endif::git-rev-list[]
If the `unfold` option is given, behave as if interpret-trailer's
`--unfold` option was given. E.g., `%(trailers:only,unfold)` to do
both.
+- %(trailer:): display the specified trailer in parentheses (like
+ %d does for refnames). If there are multiple entries of that trailer
+ they are shown comma separated. If there are no matching trailers
+ nothing is displayed.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index 8ca29e9281..61ae34ced4 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1324,6 +1324,22 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
}
}
+ if (skip_prefix(placeholder, "(trailer:", &arg)) {
+ struct process_trailer_options opts =
PROCESS_TRAILER_OPTIONS_INIT;
+ opts.no_divider = 1;
+ opts.only_trailers = 1;
+ opts.unfold = 1;
+
+ const char *end = strchr(arg, ')');
+ if (!end)
+ return 0;
+
+ opts.filter_trailer = xstrndup(arg, end - arg);
+ format_trailers_from_commit(sb, msg + c->subject_off, &opts);
+ free(opts.filter_trailer);
+ return end - placeholder + 1;
+ }
+
return 0; /* unknown placeholder */
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 978a8a66ff..e929f820e7 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -598,6 +598,46 @@ test_expect_success ':only and :unfold work together' '
test_cmp expect actual
'
+test_expect_success 'pretty format %(trailer:foo) shows that trailer' '
+ git log --no-walk --pretty="%(trailer:Acked-By)" >actual &&
+ {
+ echo "(Acked-By: A U Thor )"
+ } >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '%(trailer:nonexistant) becomes empty' '
+ git log --no-walk --pretty="x%(trailer:Nacked-By)x" >actual &&
+ {
+ echo "xx"
+ } >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '% (trailer:nonexistant) with space becomes empty' '
+ git log --no-walk --pretty="x% (trailer:Nacked-By)x" >actual &&
+ {
+ echo "xx"
+ } >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '% (trailer:foo) with space adds space before' '
+ git log --no-walk --pretty="x% (trailer:Acked-By)x" >actual &&
+ {
+ echo "x (Acked-By: A U Thor )x"
+ } >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '%(trailer:foo) with multiple lines becomes comma
separated and unwrapped' '
+ git log --no-walk --pretty="%(trailer:Signed-Off-By)" >actual &&
+ {
+ echo "(Signed-Off-By: A U Thor , A U Thor
)"
+ } >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'trailer parsing not fooled by --- line' '
git commit --allow-empty -F - <<-\EOF &&
this is the subject
diff --git a/trailer.c b/trailer.c
index 0796f326b3..d337bca8dd 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1138,6 +1138,7 @@ static void format_trailer_info(struct strbuf *out,
return;
}
+ int printed_first = 0;
for (i = 0; i < info->trailer_nr; i++) {
char *trailer = info->trailers[i];
ssiz
Re: [PATCH] diff: Add diff.orderfile configuration variable
(Jonathan, sorry if you got this multiple times, it seems I forgot to Cc list) On Mon, Oct 21, 2013 at 8:40 PM, Jonathan Nieder wrote: > Should the git-diff(1) manpage get a note about this setting as > well (perhaps in a new CONFIGURATION section)? I'll add a reference to the documentation for the -O option at least. That is how --check, --color, --dirstat and others do it, I guess that could be moved to a CONFIGURATION section later? > Should Documentation/technical/api-diff.txt be tweaked to mention that > the options set by diff_setup() depend on configuration now? It already did, didn't it? At least diff.context, diff.renames and diff.color seems to affect diff_setup(), no? > If a caller wants to parse diff config and also wants to make a diff > without using the config (the example I'm imagining is an alternative > implemention fo "git log -p --cherry-pick"), can they do that? It's > tempting to move handling of configuration into a separate function. > (Perhaps it's not worth worrying about that until someone needs the > flexibility, though.) Right, patch-ids are not stable wrt ordering. That might be a problem if some tool stores patch-ids. But maybe that even is a separate bug? Should patch-id always reorder the files internally? Is it expected that "git diff -Oorder1 | git patch-id" and "git diff -Oorder2 | git patch-id" gives same patch id? It gets very interesting in an imaginative "git log -p --cherry-pick" which caches patch-ids on disk, one would want one stable ordering for calculating the patchid, while the displayed patch should respect the user requested order. I guess that in most cases one would want to respect user configured ordering. Should diff_setup grow an argument "ignore_config"? Or should we maybe add an --no-order-file option that easily be set as a flag in diff_options in those cases? > Hope that helps, It does. Thanks! I have updated patch as per your other comments. anders -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] diff: Add diff.orderfile configuration variable
diff.orderfile acts as a default for the -O command line option.
Signed-off-by: Anders Waldenborg
---
Documentation/diff-config.txt | 4 +++
diff.c| 5 +++
t/t4056-diff-order.sh | 74 +++
3 files changed, 83 insertions(+)
create mode 100755 t/t4056-diff-order.sh
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 223b931..51f9190 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -98,6 +98,10 @@ diff.mnemonicprefix::
diff.noprefix::
If set, 'git diff' does not show any source or destination prefix.
+diff.orderfile::
+ Path to file to use for ordering the files in the diff, each line
+ is a shell glob pattern; equivalent to the 'git diff' option '-O'.
+
diff.renameLimit::
The number of files to consider when performing the copy/rename
detection; equivalent to the 'git diff' option '-l'.
diff --git a/diff.c b/diff.c
index a04a34d..e66f031 100644
--- a/diff.c
+++ b/diff.c
@@ -30,6 +30,7 @@ static int diff_use_color_default = -1;
static int diff_context_default = 3;
static const char *diff_word_regex_cfg;
static const char *external_diff_cmd_cfg;
+static const char *diff_order_file_cfg;
int diff_auto_refresh_index = 1;
static int diff_mnemonic_prefix;
static int diff_no_prefix;
@@ -201,6 +202,8 @@ int git_diff_ui_config(const char *var, const char
*value, void *cb)
return git_config_string(&external_diff_cmd_cfg, var, value);
if (!strcmp(var, "diff.wordregex"))
return git_config_string(&diff_word_regex_cfg, var, value);
+ if (!strcmp(var, "diff.orderfile"))
+ return git_config_string(&diff_order_file_cfg, var, value);
if (!strcmp(var, "diff.ignoresubmodules"))
handle_ignore_submodules_arg(&default_diff_options, value);
@@ -3207,6 +3210,8 @@ void diff_setup(struct diff_options *options)
options->detect_rename = diff_detect_rename_default;
options->xdl_opts |= diff_algorithm;
+ options->orderfile = diff_order_file_cfg;
+
if (diff_no_prefix) {
options->a_prefix = options->b_prefix = "";
} else if (!diff_mnemonic_prefix) {
diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
new file mode 100755
index 000..fd005d6
--- /dev/null
+++ b/t/t4056-diff-order.sh
@@ -0,0 +1,74 @@
+#!/bin/sh
+
+test_description='diff order'
+
+. ./test-lib.sh
+
+_test_create_files () {
+ mkdir c
+ echo "$1" >a.h
+ echo "$1" >b.c
+ echo "$1" >c/Makefile
+ echo "$1" >d.txt
+ git add a.h b.c c/Makefile d.txt && \
+ git commit -m"$1"
+}
+
+cat >order_file_1 <order_file_2 <expect_diff_headers_none <expect_diff_headers_1 <expect_diff_headers_2 <actual_diff_headers &&
+ test_debug actual_diff_headers
+ test_cmp expect_diff_headers_none actual_diff_headers'
+
+test_expect_success "orderfile using option" '
+ git diff -Oorder_file_1 HEAD^..HEAD | grep ^diff >actual_diff_headers &&
+ test_debug actual_diff_headers
+ test_cmp expect_diff_headers_1 actual_diff_headers &&
+ git diff -Oorder_file_2 HEAD^..HEAD | grep ^diff >actual_diff_headers &&
+ test_debug actual_diff_headers
+ test_cmp expect_diff_headers_2 actual_diff_headers'
+
+test_expect_success "orderfile using config" '
+ git -c diff.orderfile=order_file_1 diff HEAD^..HEAD | grep ^diff
>actual_diff_headers &&
+ test_debug actual_diff_headers
+ test_cmp expect_diff_headers_1 actual_diff_headers &&
+ git -c diff.orderfile=order_file_2 diff HEAD^..HEAD | grep ^diff
>actual_diff_headers &&
+ test_debug actual_diff_headers
+ test_cmp expect_diff_headers_2 actual_diff_headers'
+
+test_done
--
1.8.4.1.559.gdb9bdfb.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
