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

From: Jonas Fonseca <fonseca@diku.dk>
Date: Jun 9, 2005, 14:22 UTC
Message-ID: <20050609142243.GA26524@diku.dk>
In-Reply-To: <42A82211.9060305@gmail.com>

Dan Holmsand <holmsand@gmail.com> wrote Thu, Jun 09, 2005:

> This series adds optget-style option parsing, support for almost all
> git-diff-* features, git-apply --stat support, common colorization code, 
> better performance for cg-log and some other stuff.

I tried out your patchset and have a few comments ...

cg-diff:

 - The pager is only used when passing -c. Is that intentional?

 - Nice with the diffstat option.

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.

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

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.

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.

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

A minor note about the option parsing. cg-log -sh will give the error

	cg-log: unrecoginized option `-h'

-- 
Jonas Fonseca