From 0a0416a34a7ef5c64f4e0226371e4cab8c1ba982 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 13 Jan 2010 12:35:31 -0500 Subject: 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 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([[,[,]]])': 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 5783e24..51d9d6b 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 -- cgit v0.10.2-6-g49f6 From 361df5df77255321b2ca409d892b4c24b7b0441d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 13 Jan 2010 12:36:42 -0500 Subject: 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 diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt index 3b1da10..afe2759 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 6cbc1fc..af9130e 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 fa07ecf..84ac942 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, ...); -- cgit v0.10.2-6-g49f6 From 49ff9a7a02266a1b96e2236bc8f8d95e4b9507dd Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 13 Jan 2010 12:39:51 -0500 Subject: 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 diff --git a/builtin-commit.c b/builtin-commit.c index f54772f..b923038 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 a603f6d..0166d35 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" \ -- cgit v0.10.2-6-g49f6 From b706fcfe93262e485976ed2bc648b779cc47981f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 13 Jan 2010 15:17:08 -0500 Subject: 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 diff --git a/Documentation/config.txt b/Documentation/config.txt index a1e36d7..6f70cc9 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 cb666ac..8f7de0e 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 3de5000..728ab90 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 b923038..a73a532 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); -- cgit v0.10.2-6-g49f6 From fc6f19fe2b49928dcb4d2dfac88ca38a47d64cde Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 17 Jan 2010 00:57:51 -0800 Subject: 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 diff --git a/builtin-commit.c b/builtin-commit.c index a73a532..7f61e87 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) -- cgit v0.10.2-6-g49f6 From 1a893064d7b403625896a2c8bdab39f0f7db61d5 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 17 Jan 2010 13:59:36 -0800 Subject: user_ident_sufficiently_given(): refactor the logic to be usable from elsewhere Signed-off-by: Junio C Hamano diff --git a/builtin-commit.c b/builtin-commit.c index 7f61e87..29dc3df 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 bf468e5..63e0701 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 26409b2..248f769 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; +} -- cgit v0.10.2-6-g49f6