Re: [PATCH 0/6] Bunch of new features for cg-log and cg-diff

Jonas Fonseca wrote:
> I tried out your patchset and have a few comments ...
> 
> cg-diff:
> 
>  - The pager is only used when passing -c. Is that intentional?

Yes. The reasoning was that people wanting more features probably will 
use color as well (and I didn't want to force the pager on people who 
use cg-diff just to check *if* something changed).

But I'm not 100% convinced I'm doing the right thing. Any suggestions?

> cg-log:
> 
>  - In the non-verbose summary you use the author date. One motivation
>    for using the commit date is that the summary output makes it easy to
>    track 'activity' and see if/when your patch made it in. Maybe I've
>    just become too used to CVS changelogs.

Yeah, maybe :-) Seriously, I think author date carries more information, 
particularly since the log is already (most of the time) ordered by 
commit date. So, when you see an old date before a newer one, you 
immediately know that some old stuff was incorporated into the repository.

Also, git-rev-list --pretty uses author date, and I wanted to be consistent.

Of course, I could always add one more option :-)

>  - Even though the more dense time format in the summary output is a
>    nice idea the new date information is unfortunately also makes the
>    summary output less useful, IMO. It can even make the by-date
>    scanning harder because you have to jump between two significantly
>    different date formats. With the new verbose distinction there should
>    be no need for making the date so dense.

This is also a matter of taste, obviously. I actually *like* having two 
different formats, as it makes the difference between "today" and 
"earlier" more obvious. And I don't really care what time of day 
something was written three weeks ago.

Perhaps this should be customisable as well, if it's felt to be 
important enough?

> I don't much like the inverted colors caused by the searching. Although
> the quick goto next entry thing is nice the colors can be very
> intrusive, and having to search for some nonsense string to remove them
> is terrible.

"export LESS=-G" will do what you want. Maybe that should be the default?

> What about a COGITO_COLORS environment variable for configuring what
> string setup_colors() will work on. It could maybe take the place of the
> COGITO_AUTO_COLOR environment variable although this is two different
> things.

That should already be there. "COGITO_COLORS='header=31' cg-log" should 
give very red headers, for example.

> With the long help output of cg-log maybe we should consider also
> displaying it in a pager.

Good idea.

> A minor note about the option parsing. cg-log -sh will give the error
> 
> 	cg-log: unrecoginized option `-h'
> 

Yeah, I know. "-h" and "--help" are the only options not handled by 
optparse in cg-log. I thought I could rely on the general help-finding 
logic in cg-Xlib for that.

On the other hand, you really shouldn't say "cg-log -sh" :-)

/dan
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html