From 0a0416a34a7ef5c64f4e0226371e4cab8c1ba982 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 13 Jan 2010 12:35:31 -0500 Subject: [PATCH 1/6] strbuf_expand: convert "%%" to "%" 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 Signed-off-by: Junio C Hamano --- 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 deletion(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index 53a9168ba7..1686a54d22 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([[,[,]]])': 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 a0e0f850f8..3b1da10f26 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 5783e24011..51d9d6b8ac 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 a6153dca27..6cbc1fcfd8 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 571931588e..b0047d3c6b 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 From 361df5df77255321b2ca409d892b4c24b7b0441d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 13 Jan 2010 12:36:42 -0500 Subject: [PATCH 2/6] strbuf: add strbuf_addbuf_percentquote This is handy for creating strings which will be fed to printf() or strbuf_expand(). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/technical/api-strbuf.txt | 7 +++++++ strbuf.c | 11 +++++++++++ strbuf.h | 1 + 3 files changed, 19 insertions(+) diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt index 3b1da10f26..afe2759951 100644 --- a/Documentation/technical/api-strbuf.txt +++ b/Documentation/technical/api-strbuf.txt @@ -218,6 +218,13 @@ which can be used by the programmer of the callback as she sees fit. placeholder and replacement string. The array needs to be terminated by an entry with placeholder set to NULL. +`strbuf_addbuf_percentquote`:: + + Append the contents of one strbuf to another, quoting any + percent signs ("%") into double-percents ("%%") in the + destination. This is useful for literal data to be fed to either + strbuf_expand or to the *printf family of functions. + `strbuf_addf`:: Add a formatted string to the buffer. diff --git a/strbuf.c b/strbuf.c index 6cbc1fcfd8..af9130e52d 100644 --- a/strbuf.c +++ b/strbuf.c @@ -257,6 +257,17 @@ size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder, return 0; } +void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src) +{ + int i, len = src->len; + + for (i = 0; i < len; i++) { + if (src->buf[i] == '%') + strbuf_addch(dst, '%'); + strbuf_addch(dst, src->buf[i]); + } +} + size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f) { size_t res; diff --git a/strbuf.h b/strbuf.h index fa07ecf094..84ac9424b5 100644 --- a/strbuf.h +++ b/strbuf.h @@ -116,6 +116,7 @@ struct strbuf_expand_dict_entry { const char *value; }; extern size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder, void *context); +extern void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src); __attribute__((format (printf,2,3))) extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...); From 49ff9a7a02266a1b96e2236bc8f8d95e4b9507dd Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 13 Jan 2010 12:39:51 -0500 Subject: [PATCH 3/6] commit: show interesting ident information in summary There are a few cases of user identity information that we consider interesting: (1) When the author and committer identities do not match. (2) When the committer identity was picked automatically from the username, hostname and GECOS information. In these cases, we already show the information in the commit message template. However, users do not always see that template because they might use "-m" or "-F". With this patch, we show these interesting cases after the commit, along with the subject and change summary. The new output looks like: $ git commit \ -m "federalist papers" \ --author='Publius ' [master 3d226a7] federalist papers Author: Publius 1 files changed, 1 insertions(+), 0 deletions(-) for case (1), and: $ git config --global --unset user.name $ git config --global --unset user.email $ git commit -m foo [master 7c2a927] foo Committer: Jeff King Your name and email address were configured automatically based on your username and hostname. Please check that they are accurate. You can suppress this message by setting them explicitly: git config --global user.name Your Name git config --global user.email you@example.com If the identity used for this commit is wrong, you can fix it with: git commit --amend --author='Your Name ' 1 files changed, 1 insertions(+), 0 deletions(-) for case (2). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin-commit.c | 40 +++++++++++++++++++++++++++++++++++++--- t/t7501-commit.sh | 6 +++++- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/builtin-commit.c b/builtin-commit.c index f54772f74a..b923038b0a 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -35,7 +35,20 @@ static const char * const builtin_status_usage[] = { NULL }; +static const char implicit_ident_advice[] = +"Your name and email address were configured automatically based\n" +"on your username and hostname. Please check that they are accurate.\n" +"You can suppress this message by setting them explicitly:\n" +"\n" +" git config --global user.name Your Name\n" +" git config --global user.email you@example.com\n" +"\n" +"If the identity used for this commit is wrong, you can fix it with:\n" +"\n" +" git commit --amend --author='Your Name '\n"; + static unsigned char head_sha1[20], merge_head_sha1[20]; + static char *use_message_buffer; static const char commit_editmsg[] = "COMMIT_EDITMSG"; static struct lock_file index_lock; /* real index */ @@ -957,9 +970,12 @@ static void print_summary(const char *prefix, const unsigned char *sha1) { struct rev_info rev; struct commit *commit; - static const char *format = "format:%h] %s"; + struct strbuf format = STRBUF_INIT; unsigned char junk_sha1[20]; const char *head = resolve_ref("HEAD", junk_sha1, 0, NULL); + struct pretty_print_context pctx = {0}; + struct strbuf author_ident = STRBUF_INIT; + struct strbuf committer_ident = STRBUF_INIT; commit = lookup_commit(sha1); if (!commit) @@ -967,6 +983,23 @@ static void print_summary(const char *prefix, const unsigned char *sha1) if (!commit || parse_commit(commit)) die("could not parse newly created commit"); + strbuf_addstr(&format, "format:%h] %s"); + + format_commit_message(commit, "%an <%ae>", &author_ident, &pctx); + format_commit_message(commit, "%cn <%ce>", &committer_ident, &pctx); + if (strbuf_cmp(&author_ident, &committer_ident)) { + strbuf_addstr(&format, "\n Author: "); + strbuf_addbuf_percentquote(&format, &author_ident); + } + if (!user_ident_explicitly_given) { + strbuf_addstr(&format, "\n Committer: "); + strbuf_addbuf_percentquote(&format, &committer_ident); + strbuf_addch(&format, '\n'); + strbuf_addstr(&format, implicit_ident_advice); + } + strbuf_release(&author_ident); + strbuf_release(&committer_ident); + init_revisions(&rev, prefix); setup_revisions(0, NULL, &rev, NULL); @@ -977,7 +1010,8 @@ static void print_summary(const char *prefix, const unsigned char *sha1) rev.verbose_header = 1; rev.show_root_diff = 1; - get_commit_format(format, &rev); + get_commit_format(format.buf, &rev); + strbuf_release(&format); rev.always_show_header = 0; rev.diffopt.detect_rename = 1; rev.diffopt.rename_limit = 100; @@ -996,7 +1030,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1) struct pretty_print_context ctx = {0}; struct strbuf buf = STRBUF_INIT; ctx.date_mode = DATE_NORMAL; - format_commit_message(commit, format + 7, &buf, &ctx); + format_commit_message(commit, format.buf + 7, &buf, &ctx); printf("%s\n", buf.buf); strbuf_release(&buf); } diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index a603f6d21a..0166d35e51 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -117,7 +117,11 @@ test_expect_success \ test_expect_success \ "overriding author from command line" \ "echo 'gak' >file && \ - git commit -m 'author' --author 'Rubber Duck ' -a" + git commit -m 'author' --author 'Rubber Duck ' -a >output 2>&1" + +test_expect_success \ + "commit --author output mentions author" \ + "grep Rubber.Duck output" test_expect_success PERL \ "interactive add" \ From b706fcfe93262e485976ed2bc648b779cc47981f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 13 Jan 2010 15:17:08 -0500 Subject: [PATCH 4/6] commit: allow suppression of implicit identity advice We now nag the user with a giant warning when their identity was pulled from the username, hostname, and gecos information, in case it is not correct. Most users will suppress this by simply setting up their information correctly. However, there may be some users who consciously want to use that information, because having the value change from host to host contains useful information. These users can now set advice.implicitidentity to false to suppress the message. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/config.txt | 4 ++++ advice.c | 2 ++ advice.h | 1 + builtin-commit.c | 6 ++++-- 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index a1e36d7e42..6f70cc953e 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -130,6 +130,10 @@ advice.*:: Advice shown when linkgit:git-merge[1] refuses to merge to avoid overwritting local changes. Default: true. + implicitIdentity:: + Advice on how to set your identity configuration when + your information is guessed from the system username and + domain name. Default: true. -- core.fileMode:: diff --git a/advice.c b/advice.c index cb666acc3c..8f7de0e9ed 100644 --- a/advice.c +++ b/advice.c @@ -3,6 +3,7 @@ int advice_push_nonfastforward = 1; int advice_status_hints = 1; int advice_commit_before_merge = 1; +int advice_implicit_identity = 1; static struct { const char *name; @@ -11,6 +12,7 @@ static struct { { "pushnonfastforward", &advice_push_nonfastforward }, { "statushints", &advice_status_hints }, { "commitbeforemerge", &advice_commit_before_merge }, + { "implicitidentity", &advice_implicit_identity }, }; int git_default_advice_config(const char *var, const char *value) diff --git a/advice.h b/advice.h index 3de5000d8c..728ab90ef1 100644 --- a/advice.h +++ b/advice.h @@ -4,6 +4,7 @@ extern int advice_push_nonfastforward; extern int advice_status_hints; extern int advice_commit_before_merge; +extern int advice_implicit_identity; int git_default_advice_config(const char *var, const char *value); diff --git a/builtin-commit.c b/builtin-commit.c index b923038b0a..a73a532f2f 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -994,8 +994,10 @@ static void print_summary(const char *prefix, const unsigned char *sha1) if (!user_ident_explicitly_given) { strbuf_addstr(&format, "\n Committer: "); strbuf_addbuf_percentquote(&format, &committer_ident); - strbuf_addch(&format, '\n'); - strbuf_addstr(&format, implicit_ident_advice); + if (advice_implicit_identity) { + strbuf_addch(&format, '\n'); + strbuf_addstr(&format, implicit_ident_advice); + } } strbuf_release(&author_ident); strbuf_release(&committer_ident); From fc6f19fe2b49928dcb4d2dfac88ca38a47d64cde Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 17 Jan 2010 00:57:51 -0800 Subject: [PATCH 5/6] commit.c::print_summary: do not release the format string too early When we are showing a clean merge, log_tree_commit() won't show the header and we would need the format string to format the commit summary ourselves. Signed-off-by: Junio C Hamano --- builtin-commit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin-commit.c b/builtin-commit.c index a73a532f2f..7f61e87ebd 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -1013,7 +1013,6 @@ static void print_summary(const char *prefix, const unsigned char *sha1) rev.verbose_header = 1; rev.show_root_diff = 1; get_commit_format(format.buf, &rev); - strbuf_release(&format); rev.always_show_header = 0; rev.diffopt.detect_rename = 1; rev.diffopt.rename_limit = 100; @@ -1036,6 +1035,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1) printf("%s\n", buf.buf); strbuf_release(&buf); } + strbuf_release(&format); } static int git_commit_config(const char *k, const char *v, void *cb) From 1a893064d7b403625896a2c8bdab39f0f7db61d5 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 17 Jan 2010 13:59:36 -0800 Subject: [PATCH 6/6] user_ident_sufficiently_given(): refactor the logic to be usable from elsewhere Signed-off-by: Junio C Hamano --- builtin-commit.c | 4 ++-- cache.h | 1 + ident.c | 5 +++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/builtin-commit.c b/builtin-commit.c index 7f61e87ebd..29dc3df786 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -602,7 +602,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, author_ident); free(author_ident); - if (!user_ident_explicitly_given) + if (!user_ident_sufficiently_given()) fprintf(fp, "%s" "# Committer: %s\n", @@ -991,7 +991,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1) strbuf_addstr(&format, "\n Author: "); strbuf_addbuf_percentquote(&format, &author_ident); } - if (!user_ident_explicitly_given) { + if (!user_ident_sufficiently_given()) { strbuf_addstr(&format, "\n Committer: "); strbuf_addbuf_percentquote(&format, &committer_ident); if (advice_implicit_identity) { diff --git a/cache.h b/cache.h index bf468e5235..63e0701dee 100644 --- a/cache.h +++ b/cache.h @@ -926,6 +926,7 @@ extern const char *config_exclusive_filename; extern char git_default_email[MAX_GITNAME]; extern char git_default_name[MAX_GITNAME]; extern int user_ident_explicitly_given; +extern int user_ident_sufficiently_given(void); extern const char *git_commit_encoding; extern const char *git_log_output_encoding; diff --git a/ident.c b/ident.c index 26409b2a1b..248f769fd3 100644 --- a/ident.c +++ b/ident.c @@ -259,3 +259,8 @@ const char *git_committer_info(int flag) getenv("GIT_COMMITTER_DATE"), flag); } + +int user_ident_sufficiently_given(void) +{ + return user_ident_explicitly_given; +}