[PATCH] log: respect log.shownotes variable

[PATCH] log: respect log.shownotes variable

[Jeff King]

Log shows notes by default if no --pretty format is given.
However, depending on what's in your default notes ref, that
may actually be quite distracting when reading a normal log.
Let's let the user decide whether they want to see notes by
default.

Signed-off-by: Jeff King <peff@peff.net>
---
My git repository has your mailing list reference notes in
refs/notes/commits. They're quite bulky, and being headers, they look
kind of like a new commit stanza, which always confuses me when reading
log output. I got tired of typing --no-notes.

I guess an alternate solution would be for me to store them in some
other ref, and then use --show-notes=email when I do want to see them
(instead of just --show-notes, as I would do with this patch). But I
still wonder if log.shownotes is a sensible addition, just for the sake
of completeness.

 builtin/log.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 046f3e4..3d93607 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -24,6 +24,7 @@
 static const char *default_date_mode = NULL;
 
 static int default_show_root = 1;
+static int default_show_notes = 1;
 static int decoration_style;
 static const char *fmt_patch_subject_prefix = "PATCH";
 static const char *fmt_pretty;
@@ -77,7 +78,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	argc = setup_revisions(argc, argv, rev, opt);
 
 	if (!rev->show_notes_given && !rev->pretty_given)
-		rev->show_notes = 1;
+		rev->show_notes = default_show_notes;
 	if (rev->show_notes)
 		init_display_notes(&rev->notes_opt);
 
@@ -291,6 +292,10 @@ static int git_log_config(const char *var, const char *value, void *cb)
 		default_show_root = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "log.shownotes")) {
+		default_show_notes = git_config_bool(var, value);
+		return 0;
+	}
 	return git_diff_ui_config(var, value, cb);
 }
 
-- 
1.7.1.rc0.220.g92be4.dirty

Re: [PATCH] log: respect log.shownotes variable

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I guess an alternate solution would be for me to store them in some
> other ref, and then use --show-notes=email when I do want to see them
> (instead of just --show-notes, as I would do with this patch). But I
> still wonder if log.shownotes is a sensible addition, just for the sake
> of completeness.

My gut feeling is that people who do have notes in the default namespace
(or the ones in notes.displayRef) do want to see them _all the time_.
Otherwise they'd put such notes in namespaces that are not shown by
default, and view them on demand, no?

But then there is a bigger question: what kind of notes would you want to
always see as part of "git log" output?  My answer to that question for my
personal workflow (both dayjob and here) so far has been "none", and that
is exactly the reason why my "applied from this message" notes live in
notes/am namespace and I have nothing in the default notes namespace.

If the answer is "none" for everybody, we should disable showing notes in
any command unless the user explicitly asks with either the --show-notes
command line option or the notes.displayRef and perhaps log.shownotes
configuration variables.

Re: [PATCH] log: respect log.shownotes variable

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 08.04.2010 05:53:
> Jeff King <peff@peff.net> writes:
> 
>> I guess an alternate solution would be for me to store them in some
>> other ref, and then use --show-notes=email when I do want to see them
>> (instead of just --show-notes, as I would do with this patch). But I
>> still wonder if log.shownotes is a sensible addition, just for the sake
>> of completeness.
> 
> My gut feeling is that people who do have notes in the default namespace
> (or the ones in notes.displayRef) do want to see them _all the time_.
> Otherwise they'd put such notes in namespaces that are not shown by
> default, and view them on demand, no?
> 
> But then there is a bigger question: what kind of notes would you want to
> always see as part of "git log" output?  My answer to that question for my
> personal workflow (both dayjob and here) so far has been "none", and that
> is exactly the reason why my "applied from this message" notes live in
> notes/am namespace and I have nothing in the default notes namespace.
> 
> If the answer is "none" for everybody, we should disable showing notes in
> any command unless the user explicitly asks with either the --show-notes
> command line option or the notes.displayRef and perhaps log.shownotes
> configuration variables.

Personally, I like showing "personal" notes to patches by default. They
often end up being the comments after the "---" in patch e-mails. All
other notes (such as TR's) I keep in separate name spaces.

But I don't mind setting a(nother) config variable for that ;)

BTW: I used to keep external notes in
refs/remotes/remotename/notes/notestree but since a while ago the notes
code insists on refs/notes/. In some sense it is similar to tags, where
you need to set up special refspecs if you want to have the remote
tags/notes but don't want to mix them (and possibly have them conflict)
with your own. But I don't know a good strategy for making this more
user friendly.

Michael

Re: [PATCH] log: respect log.shownotes variable

[Jeff King]

On Thu, Apr 08, 2010 at 10:01:46AM +0800, Michael J Gruber wrote:

> BTW: I used to keep external notes in
> refs/remotes/remotename/notes/notestree but since a while ago the notes
> code insists on refs/notes/. In some sense it is similar to tags, where
> you need to set up special refspecs if you want to have the remote
> tags/notes but don't want to mix them (and possibly have them conflict)
> with your own. But I don't know a good strategy for making this more
> user friendly.

I always assumed that the "prepend refs/notes" magic would refuse to do
so if the ref started with "notes/" (in which case it would append
"refs/") or "refs/" (in which case it would append nothing). That is how
we handle some situations where a branch head or a remote could be
used.

But I never checked, so maybe it doesn't. It probably should.

-Peff

Re: [PATCH] log: respect log.shownotes variable

[Jeff King]

On Wed, Apr 07, 2010 at 02:53:42PM -0700, Junio C Hamano wrote:

> My gut feeling is that people who do have notes in the default namespace
> (or the ones in notes.displayRef) do want to see them _all the time_.
> Otherwise they'd put such notes in namespaces that are not shown by
> default, and view them on demand, no?

I dunno. I don't have any notes I've written, so I just stuck Thomas' in
refs/notes/commit. In retrospect, that was probably dumb, because it
conflicts with any notes I would write.

> But then there is a bigger question: what kind of notes would you want to
> always see as part of "git log" output?  My answer to that question for my
> personal workflow (both dayjob and here) so far has been "none", and that
> is exactly the reason why my "applied from this message" notes live in
> notes/am namespace and I have nothing in the default notes namespace.

Yeah, thinking on it more, that is much better. I now have:

  [remote "trast"]
	  url = git://repo.or.cz/git/trast.git
	  fetch = +refs/heads/notes/*:refs/notes/am/*

which makes more sense (and I was happy to see he has kept updating
them. My copy was quite stale).

So let's ignore log.shownotes for now, and encourage people to manage
their notes better. When we see more how people are using notes, we may
want it, but let's wait until then.

-Peff

Re: [PATCH] log: respect log.shownotes variable

[Thomas Rast]

Jeff King wrote:
> Log shows notes by default if no --pretty format is given.
> However, depending on what's in your default notes ref, that
> may actually be quite distracting when reading a normal log.
> Let's let the user decide whether they want to see notes by
> default.

Looks useful to me, and the patch looks obviously correct.

> My git repository has your mailing list reference notes in
> refs/notes/commits. They're quite bulky, and being headers, they look
> kind of like a new commit stanza, which always confuses me when reading
> log output. I got tired of typing --no-notes.
> 
> I guess an alternate solution would be for me to store them in some
> other ref, and then use --show-notes=email when I do want to see them
> (instead of just --show-notes, as I would do with this patch). But I
> still wonder if log.shownotes is a sensible addition, just for the sake
> of completeness.

I think it is a useful addition, *especially* in the case where the
configuration departs from the default.  Typing --show-notes=email
--show-notes=commits just to avoid showing a third set of notes in
refs/notes/bulky will get old fairly quickly...

BTW, now that I seem to have a couple of users for those notes: what
would you like to see in the "short" version (notes/terse)?  Improving
that to be the "common" case has been on my todo list for quite some
time.  For example, I'm considering changing the 'permalink' to
'thread' so that access to the full thread is easier (you'll still
have to click the subject to find the root).  Also, the message text
snippets have been broken since I revived it in February, does anyone
miss them?  They made the output even bulkier.

Of course, I also hope that in the long run we can somehow generate
this from Junio's amlog for better accuracy :-)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] log: respect log.shownotes variable

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> Looks useful to me, and the patch looks obviously correct.

It looked obviously correct and "because we can" not "because we need" to me.

> I think it is a useful addition, *especially* in the case where the
> configuration departs from the default.  Typing --show-notes=email
> --show-notes=commits just to avoid showing a third set of notes in
> refs/notes/bulky will get old fairly quickly...

I initially thought so until I realized that notes.displayRef is already
there.  But you are the one who did 894a9d3 (Support showing notes from
more than one notes tree, 2010-03-12) to add that feature, so I suspect
I forgot something while reaching my conclusion in [*1*].

[reference]

*1* Message-ID: <7vljczapzt.fsf@alter.siamese.dyndns.org>
    $gmane/144286

Re: [PATCH] log: respect log.shownotes variable

[Jeff King]

On Thu, Apr 08, 2010 at 06:12:49PM +0200, Thomas Rast wrote:

> BTW, now that I seem to have a couple of users for those notes: what
> would you like to see in the "short" version (notes/terse)?  Improving

For me, the message-id is sufficient. I keep my own local archive of the
git list (because reading old threads in mutt is much nicer than on
gmane), and it is trivial to convert message-id into whatever gmane link
I want for referencing to other people.

-Peff