Re: git notes: notes

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

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Actually I am of two minds regarding --pretty={short,medium} and the
> like.  The "how about this" patch may be the safest for people who are
> used to read "log --pretty=xxx" output with scripts, but it does look
> inconsistent and hard to explain to new people who do not even know that
> there were versions of git that does not know about notes.

Heh, it is not surprising that we have this bug, given that all of us just
missed how bogus my "how about this" patch was ;-)

The check for "if (fmt_pretty)" only kicks in when that thing was read
from the configuration; handling of command line --pretty and --pretty=
options happen long after that, when we call setup_revisions().

So if we really wanted to say "If the user explicitly tells us to run
under a particular --pretty mode, we don't show notes by default.", we
would need a patch like this on top of it.

Another thing to note is that "work differently between no --pretty on the
command line and an explicit --pretty=medium" is more work than "we always
default to show notes if showing in the default verbosity".

This version considers a user supplied "format.pretty" configuration as
"the user told us to use this specific format, and we won't show notes
unless explicitly told".  I personally don't care either way exactly
because I don't use that configuration (nor teach others to use it), but
in a sense the configuration is setting a personal "default", so I think
it could be argued that we should instead show the notes by default in
that case (i.e. remove "rev->pretty_given = 1" in the first hunk).

 builtin-log.c |    9 ++++++---
 revision.c    |    4 ++++
 revision.h    |    2 ++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 3bc3919..1e05b0f 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -39,10 +39,10 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
-	if (fmt_pretty)
+	if (fmt_pretty) {
 		get_commit_format(fmt_pretty, rev);
-	else
-		rev->show_notes = 1;
+		rev->pretty_given = 1;
+	}
 	rev->verbose_header = 1;
 	DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
 	rev->show_root_diff = default_show_root;
@@ -60,6 +60,9 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		usage(builtin_log_usage);
 	argc = setup_revisions(argc, argv, rev, "HEAD");
 
+	if (!rev->show_notes_given && !rev->pretty_given)
+		rev->show_notes = 1;
+
 	if (rev->diffopt.pickaxe || rev->diffopt.filter)
 		rev->always_show_header = 0;
 	if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
diff --git a/revision.c b/revision.c
index 7e00a6c..0de78fb 100644
--- a/revision.c
+++ b/revision.c
@@ -1161,14 +1161,18 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->verbose_header = 1;
 	} else if (!strcmp(arg, "--pretty")) {
 		revs->verbose_header = 1;
+		revs->pretty_given = 1;
 		get_commit_format(arg+8, revs);
 	} else if (!prefixcmp(arg, "--pretty=") || !prefixcmp(arg, "--format=")) {
 		revs->verbose_header = 1;
+		revs->pretty_given = 1;
 		get_commit_format(arg+9, revs);
 	} else if (!strcmp(arg, "--show-notes")) {
 		revs->show_notes = 1;
+		revs->show_notes_given = 1;
 	} else if (!strcmp(arg, "--no-notes")) {
 		revs->show_notes = 0;
+		revs->show_notes_given = 1;
 	} else if (!strcmp(arg, "--oneline")) {
 		revs->verbose_header = 1;
 		get_commit_format("oneline", revs);
diff --git a/revision.h b/revision.h
index 4167c1e..a14deef 100644
--- a/revision.h
+++ b/revision.h
@@ -81,6 +81,8 @@ struct rev_info {
 	unsigned int	shown_one:1,
 			show_merge:1,
 			show_notes:1,
+			show_notes_given:1,
+			pretty_given:1,
 			abbrev_commit:1,
 			use_terminator:1,
 			missing_newline:1,
--
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]