log --pretty/--oneline: ignore log.decorate

Subsystems: the rest

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

log --pretty/--oneline: ignore log.decorate

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:48:34

Junio C Hamano [off-list ref] writes:
[Stalled]

* sd/log-decorate (2010-02-17) 3 commits
  (merged to 'next' on 2010-03-08 at 58a6fba)
 + log.decorate: usability fixes
 + Add `log.decorate' configuration variable.
 + git_config_maybe_bool()

Needs squelching the configuration setting when "--pretty=raw" is given,
at least, or possibly when any "--pretty" is explicitly given.
This is necessary if we want to let users specify log.decorate and still
use gitk.  A patch should look like this (of course untested).

-- >8 --

Many scripts, most notably gitk, rely on output from the log family of
command not to be molested by random user configuration.  This is
especially true when --pretty=raw is given.

Just like we disable notes output unless the command line explicitly
asks for --show-notes, disable the decoration code unless --decorate is
given explicitly from the command line and --pretty or --oneline is
given.

Signed-off-by: Junio C Hamano <redacted>

---
 builtin-log.c  |   11 +++++++++++
 t/t4202-log.sh |   44 ++++++++++++++++++++++++++++----------------
 2 files changed, 39 insertions(+), 16 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 0afba31..7f4186f 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -53,6 +53,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		      struct rev_info *rev)
 {
 	int i;
+	int decoration_given = 0;
 
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
@@ -89,11 +90,13 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		const char *arg = argv[i];
 		if (!strcmp(arg, "--decorate")) {
 			decoration_style = DECORATE_SHORT_REFS;
+			decoration_given = 1;
 		} else if (!prefixcmp(arg, "--decorate=")) {
 			const char *v = skip_prefix(arg, "--decorate=");
 			decoration_style = parse_decoration_style(arg, v);
 			if (decoration_style < 0)
 				die("invalid --decorate option: %s", arg);
+			decoration_given = 1;
 		} else if (!strcmp(arg, "--no-decorate")) {
 			decoration_style = 0;
 		} else if (!strcmp(arg, "--source")) {
@@ -103,6 +106,14 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		} else
 			die("unrecognized argument: %s", arg);
 	}
+
+	/*
+	 * defeat log.decorate configuration interacting with --pretty
+	 * from the command line.
+	 */
+	if (!decoration_given && rev->pretty_given)
+		decoration_style = 0;
+
 	if (decoration_style) {
 		rev->show_decorations = 1;
 		load_ref_decorations(decoration_style);
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 2230e60..166de44 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -390,50 +390,62 @@ test_expect_success 'log --graph with merge' '
 test_expect_success 'log.decorate configuration' '
 	git config --unset-all log.decorate || :
 
-	git log --oneline >expect.none &&
-	git log --oneline --decorate >expect.short &&
-	git log --oneline --decorate=full >expect.full &&
+	git log >expect.none &&
+	git log --decorate >expect.short &&
+	git log --decorate=full >expect.full &&
+	git log --oneline >expect.oneline &&
 
 	echo "[log] decorate" >>.git/config &&
-	git log --oneline >actual &&
+	git log >actual &&
 	test_cmp expect.short actual &&
+	git log --oneline >actual &&
+	test_cmp expect.oneline actual &&
 
 	git config --unset-all log.decorate &&
 	git config log.decorate true &&
-	git log --oneline >actual &&
+	git log >actual &&
 	test_cmp expect.short actual &&
-	git log --oneline --decorate=full >actual &&
+	git log --decorate=full >actual &&
 	test_cmp expect.full actual &&
-	git log --oneline --decorate=no >actual &&
+	git log --decorate=no >actual &&
 	test_cmp expect.none actual &&
+	git log --oneline >actual &&
+	test_cmp expect.oneline actual &&
 
 	git config --unset-all log.decorate &&
 	git config log.decorate no &&
-	git log --oneline >actual &&
+	git log >actual &&
 	test_cmp expect.none actual &&
-	git log --oneline --decorate >actual &&
+	git log --decorate >actual &&
 	test_cmp expect.short actual &&
-	git log --oneline --decorate=full >actual &&
+	git log --decorate=full >actual &&
 	test_cmp expect.full actual &&
+	git log --oneline >actual &&
+	test_cmp expect.oneline actual &&
 
 	git config --unset-all log.decorate &&
 	git config log.decorate short &&
-	git log --oneline >actual &&
+	git log >actual &&
 	test_cmp expect.short actual &&
-	git log --oneline --no-decorate >actual &&
+	git log --no-decorate >actual &&
 	test_cmp expect.none actual &&
-	git log --oneline --decorate=full >actual &&
+	git log --decorate=full >actual &&
 	test_cmp expect.full actual &&
+	git log --oneline >actual &&
+	test_cmp expect.oneline actual &&
 
 	git config --unset-all log.decorate &&
 	git config log.decorate full &&
-	git log --oneline >actual &&
+	git log >actual &&
 	test_cmp expect.full actual &&
-	git log --oneline --no-decorate >actual &&
+	git log --no-decorate >actual &&
 	test_cmp expect.none actual &&
-	git log --oneline --decorate >actual &&
+	git log --decorate >actual &&
 	test_cmp expect.short actual
+	git log --oneline >actual &&
+	test_cmp expect.oneline actual &&
 
+	:
 '
 
 test_done

Re: log --pretty/--oneline: ignore log.decorate

From: Jeff King <hidden>
Date: 2016-06-15 22:48:35

On Tue, Apr 06, 2010 at 02:48:55PM -0700, Junio C Hamano wrote:
Junio C Hamano [off-list ref] writes:
quoted
[Stalled]

* sd/log-decorate (2010-02-17) 3 commits
  (merged to 'next' on 2010-03-08 at 58a6fba)
 + log.decorate: usability fixes
 + Add `log.decorate' configuration variable.
 + git_config_maybe_bool()

Needs squelching the configuration setting when "--pretty=raw" is given,
at least, or possibly when any "--pretty" is explicitly given.
This is necessary if we want to let users specify log.decorate and still
use gitk.  A patch should look like this (of course untested).
Hmm. You took the "any --pretty" option with this patch.

And given the markups to the test suite:
-	git log --oneline >expect.none &&
-	git log --oneline --decorate >expect.short &&
-	git log --oneline --decorate=full >expect.full &&
+	git log >expect.none &&
+	git log --decorate >expect.short &&
+	git log --decorate=full >expect.full &&
+	git log --oneline >expect.oneline &&
I suspect that will annoy users who have set log.decorate. Wouldn't they
expect to see it with "--oneline" if they have it configured (I would
guess that "git log" and "git log --oneline" are the most commonly
viewed forms by humans).

It seems reasonable to say "git log --pretty=raw should be consumable by
scripts", but I don't know that we need to do so for every pretty
format.

However:

  $ git grep oneline *.sh
  git-rebase--interactive.sh:             git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
  git-stash.sh:           head=$(git log --no-color --abbrev-commit --pretty=oneline -n 1 HEAD --)
  git-submodule.sh:                       git log --pretty=oneline --first-parent $range | wc -l

rebase--interactive properly uses rev-list. The submodule invocation
would uselessly look up the decoration, and the stash one would add it
to the stash commit message. But I am inclined to say that both of those
scripts are at fault, and should be converted to rev-list.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help