4 messages, 3 authors, 2016-06-15 · open the first message on its own page
From: Dan Holmsand <hidden>
Date: 2016-06-15 22:41:59
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. /dan
From: Jonas Fonseca <hidden>
Date: 2016-06-15 22:41:59
Dan Holmsand [off-list ref] 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
From: Dan Holmsand <hidden>
Date: 2016-06-15 22:41:59
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
From: Dan Holmsand <hidden>
Date: 2016-06-15 22:41:59
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