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

Jeff King [off-list ref] writes:

quoted

quoted

* 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.

Yeah, I considered to further narrow it down to the --pretty=raw case;
because that is not something we do for the default --show-notes, I opted
for consistency.  But a decoration and notes are quite different, and
such a consistency perhaps is not worth it.  How about this on top?

-- >8 --
Subject: log: only "--pretty=raw" defeats log.decorate from the command line

Unlike notes that are often multi-line and disrupting to be placed in many
output formats, a decoration is designed to be a small token that can be
tacked after an existing line of the output where a commit object name sits.

Disabling log.decorate for something like "log --oneline" would defeat the
purpose of the configuration.

We _might_ want to change it further in the future to force scripts that
do not want to be broken by random end user configurations to explicitly
say "log --no-decorate", but that would be an incompatible change that
needs the usual multi-release-cycle deprecation process.

Signed-off-by: Junio C Hamano <redacted>
---
 builtin-log.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 7f4186f..017fcf8 100644
--- a/builtin-log.c
+++ b/builtin-log.c

@@ -108,10 +108,11 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	}
 
 	/*
-	 * defeat log.decorate configuration interacting with --pretty
+	 * defeat log.decorate configuration interacting with --pretty=raw
 	 * from the command line.
 	 */
-	if (!decoration_given && rev->pretty_given)
+	if (!decoration_given && rev->pretty_given
+	    && rev->commit_format == CMIT_FMT_RAW)
 		decoration_style = 0;
 
 	if (decoration_style) {

On Thu, Apr 08, 2010 at 10:14:24AM -0700, Junio C Hamano wrote:

Yeah, I considered to further narrow it down to the --pretty=raw case;
because that is not something we do for the default --show-notes, I opted
for consistency.  But a decoration and notes are quite different, and
such a consistency perhaps is not worth it.  How about this on top?

Since I consider normal output, --pretty=raw, and --oneline to be the
only useful output formats for git-log, I think of the right behavior
as:

  - normal: show both by default, since that is the point of the
    features

  - raw: show neither, since we are probably parsed by a script

  - oneline: don't show notes, as they are inherently multi-line. Do
    show decorations, as they inherently fit the hash+subject model

User-formats are useful, too, but outside the scope, as they should
always show exactly what was requested, and nothing more.

For the other formats, I couldn't care less. So your patch is fine, but
I would also be fine with turning it back on _just_ for --oneline, and
leaving all other pretty formats with it disabled.

The patch itself looks fine. As you know, it needs to revert some of the
test updates from your previous tip.

We should also either apply on top or squash in:

-- >8 --
Subject: [PATCH] script with rev-list instead of log

Because log.decorate now shows decorations for --pretty=oneline,
we must explicitly turn it off when scripting. Otherwise,
users with log.decorate set will get cruft like:

  $ git stash
  Saved working directory and index state WIP on master:
    2c1f7f5 (HEAD, master) commit subject

Instead of adding --no-decorate to the log command line,
let's just use the rev-list plumbing interface instead,
which does the right thing.

git-submodule has a similar call. Since it just counts the
commit lines, nothing is broken, but let's switch it, too,
for the sake of consistency and cleanliness.

Signed-off-by: Jeff King <redacted>
---
I think these changes are Obviously Correct (tm), but please confirm
that I am not missing something subtle.

 git-stash.sh     |    2 +-
 git-submodule.sh |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 62beae0..abaeee7 100755
--- a/git-stash.sh
+++ b/git-stash.sh

@@ -57,7 +57,7 @@ create_stash () {
 	# state of the base commit
 	if b_commit=$(git rev-parse --verify HEAD)
 	then
-		head=$(git log --no-color --abbrev-commit --pretty=oneline -n 1 HEAD --)
+		head=$(git rev-list --oneline -n 1 HEAD --)
 	else
 		die "You do not have the initial commit yet"
 	fi

diff --git a/git-submodule.sh b/git-submodule.sh
index 2dd372a..187461c 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -648,7 +648,7 @@ cmd_summary() {
 				range=$sha1_dst
 			fi
 			GIT_DIR="$name/.git" \
-			git log --pretty=oneline --first-parent $range | wc -l
+			git rev-list --first-parent $range -- | wc -l
 			)
 			total_commits=" ($(($total_commits + 0)))"
 			;;

-- 
1.7.1.rc0.248.g055378.dirty