Re: [RFC PATCH] git log: support "auto" decorations

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: May 30, 2014, 04:54 UTC
Message-ID: <CA+55aFzwy09-i=hpBy-5bYS6eowGzkdcF65cFJpL2qnJvYq85w@mail.gmail.com>
In-Reply-To: <20140530015855.GG28683@sigill.intra.peff.net>

On Thu, May 29, 2014 at 6:58 PM, Jeff King <peff@peff.net> wrote:

>
> I will spare you the usual lecture on having these lines in the message
> body. ;)

We do it for the kernel because they often get lost otherwise. Particularly the date/author.

git doesn't tend to have the same kind of deep email forwarding models, and nobody uses quilt to develop git (I hope!) but it's a habit I like to encourage for the kernel, so I use it in general..

> Yeah, I think this makes a lot of sense. I do use log.decorate=true, and
> it is usually not a big deal. However, I think I have run into
> annoyances once or twice when piping it. I'd probably use
> log.decorate=auto if we had it.

I have various scripts to generate releases etc, using "git log" and piping it to random other stuff. I don't _think_ they care, but quite frankly, I'd rather not even have to think about it.

Also, I actually find the un-colorized version of the decorations to be distracting. With color (not that I really like the default color scheme, but I'm too lazy to set it up to anything else), the colorization ends up making the decorations much easier to visually separate, so I like them there.

Show 6 quoted lines

>> It's marked with RFC because
>>
>>  (a) that "isatty(1) || pager_in_use()" test is kind of hacky, maybe we
>>      would be better off sharing something with the auto-coloration?
>
> The magic for this is in color.c, want_color() and check_auto_color().

Oh, I know. That's where I stole it from. But the colorization actually has very different rules, and looks at TERM etc. So I only stole the actual non-color-specific parts of it, but I was wondering whether it might make sense to unify them.

Show 10 quoted lines

>>  (b) I also think it would be nice to have the equivalent for
>>      "--show-signature", but there we don't have any preexisting config
>>      file option.
>
> Potentially yes, though there is a real performance impact for "log
> --show-signature" if you actually have a lot of signatures. Even on
> linux.git, a full "git log" is 15s with --show-signature, and 5s
> without. Maybe that is acceptable for interactive use (and certainly it
> is not a reason to make it an _option_, if somebody wants to turn it
> on).

Yes. This is an example of where the whole "tty is fundamentally different from scripting" happens. It really is about the whole "user is looking at it" vs "scripting". There is no way in hell that "--show-signature" should be on by default for anybody sane.

That said, part of it is just that show-signature is so suboptimal performance-wise, re-parsing the commit buffer for each commit when "show_signature" is set. That's just crazy, we've already parsed the commit text, we already *could* know if it has a signature or not, and skip it if it doesn't. That would require one of the flag bits in the object, though, or something, so it's probably not worth doing.

Sure, performance might matter if you end up searching for something in the pager after you've done "git log", but personally I think I'd never even notice..

          Linus