[PATCH v2 1/3] strbuf_expand: convert "%%" to "%"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The only way to safely quote arbitrary text in a
pretty-print user format is to replace instances of "%" with
"%x25". This is slightly unreadable, and many users would
expect "%%" to produce a single "%", as that is what printf
format specifiers do.

This patch converts "%%" to "%" for all users of
strbuf_expand:

 1. git-daemon interpolated paths

 2. pretty-print user formats

 3. merge driver command lines

Case (1) was already doing the conversion itself outside of
strbuf_expand. Case (2) is the intended beneficiary of this
patch. Case (3) users probably won't notice, but as this is
user-facing behavior, consistently providing the quoting
mechanism makes sense.

Signed-off-by: Jeff King <peff@xxxxxxxxxxxxxxxxxxxxxxx>
---
Changes from v1:
  - note change in strbuf api docs

 Documentation/pretty-formats.txt       |    1 +
 Documentation/technical/api-strbuf.txt |    4 ++++
 daemon.c                               |    1 -
 strbuf.c                               |    6 ++++++
 t/t6006-rev-list-format.sh             |    7 +++++++
 5 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 53a9168..1686a54 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -134,6 +134,7 @@ The placeholders are:
 - '%C(...)': color specification, as described in color.branch.* config option
 - '%m': left, right or boundary mark
 - '%n': newline
+- '%%': a raw '%'
 - '%x00': print a byte from a hex code
 - '%w([<w>[,<i1>[,<i2>]]])': switch line wrapping, like the -w option of
   linkgit:git-shortlog[1].
diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt
index a0e0f85..3b1da10 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -199,6 +199,10 @@ character if the letter `n` appears after a `%`.  The function returns
 the length of the placeholder recognized and `strbuf_expand()` skips
 over it.
 +
+The format `%%` is automatically expanded to a single `%` as a quoting
+mechanism; callers do not need to handle the `%` placeholder themselves,
+and the callback function will not be invoked for this placeholder.
++
 All other characters (non-percent and not skipped ones) are copied
 verbatim to the strbuf.  If the callback returned zero, meaning that the
 placeholder is unknown, then the percent sign is copied, too.
diff --git a/daemon.c b/daemon.c
index 918e560..360635e 100644
--- a/daemon.c
+++ b/daemon.c
@@ -147,7 +147,6 @@ static char *path_ok(char *directory)
 			{ "IP", ip_address },
 			{ "P", tcp_port },
 			{ "D", directory },
-			{ "%", "%" },
 			{ NULL }
 		};
 
diff --git a/strbuf.c b/strbuf.c
index a6153dc..6cbc1fc 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -227,6 +227,12 @@ void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn,
 			break;
 		format = percent + 1;
 
+		if (*format == '%') {
+			strbuf_addch(sb, '%');
+			format++;
+			continue;
+		}
+
 		consumed = fn(sb, format, context);
 		if (consumed)
 			format += consumed;
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 5719315..b0047d3 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -19,6 +19,13 @@ test_cmp expect.$1 output.$1
 "
 }
 
+test_format percent %%h <<'EOF'
+commit 131a310eb913d107dd3c09a65d1651175898735d
+%h
+commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+%h
+EOF
+
 test_format hash %H%n%h <<'EOF'
 commit 131a310eb913d107dd3c09a65d1651175898735d
 131a310eb913d107dd3c09a65d1651175898735d
-- 
1.6.6.140.g92e4d.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]