threads / patch / 7150

patchshortlog: prompt when reading from terminal by mistake

Subject: [PATCH] shortlog: prompt when reading from terminal by mistake

## tl;dr

17 messages between Mar 8, 2007 and Mar 10, 2007. Diffs are folded; open one to read it.

replies: 16people: 7as markdown or json

Junio C Hamano· Mar 8, 2007, 10:12 UTC · lore

I was trying to see who have been active recently to find GSoC mentor candidates by running:

	$ git shortlog -s -n --since=4.months | head -n 20

After waiting for about 20 seconds, I started getting worried, thinking that the recent revision traversal updates might have had an unintended side effect.

Not so. "git shortlog" acts as a filter when no revs are given, unlike "git log" which defaults to HEAD. It was reading from its standard input.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
   Arguably because --since=4.months clearly states I am
   expecting it to start digging from somewhere, it would be
   more sensible to default to HEAD in this case.  I suspect it
   might be as simple as checking argc before we call
   setup_revisions() and pass "HEAD" as the default parameter to
   it, but it is getting late here.
 builtin-shortlog.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
Show changes to builtin-shortlog.c +4 −1
diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index 2d7726e..29343ae 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -304,8 +304,11 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 	if (!access(".mailmap", R_OK))
 		read_mailmap(".mailmap");
 
-	if (rev.pending.nr == 0)
+	if (rev.pending.nr == 0) {
+		if (isatty(0))
+			fprintf(stderr, "(reading log to summarize from standard input)\n");
 		read_from_stdin(&list);
+	}
 	else
 		get_from_rev(&rev, &list);
Johannes Schindelin· Mar 8, 2007, 13:04 UTC · re: Junio C Hamano · lore

Re: [PATCH] shortlog: prompt when reading from terminal by mistake

Hi,
On Thu, 8 Mar 2007, Junio C Hamano wrote:
Show 14 quoted lines
> I was trying to see who have been active recently to find GSoC
> mentor candidates by running:
> 
> 	$ git shortlog -s -n --since=4.months | head -n 20
> 
> After waiting for about 20 seconds, I started getting worried,
> thinking that the recent revision traversal updates might have
> had an unintended side effect.
> 
> Not so.  "git shortlog" acts as a filter when no revs are given,
> unlike "git log" which defaults to HEAD.  It was reading from
> its standard input.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Show 6 quoted lines
>    Arguably because --since=4.months clearly states I am
>    expecting it to start digging from somewhere, it would be
>    more sensible to default to HEAD in this case.  I suspect it
>    might be as simple as checking argc before we call
>    setup_revisions() and pass "HEAD" as the default parameter to
>    it, but it is getting late here.

Hm. Better check argc _afterwards_, and if it changed, but no pending object was there, add "HEAD".

Ciao, Dscho

Shawn O. Pearce· Mar 8, 2007, 15:07 UTC · re: Junio C Hamano · lore

Re: GSoC project mentors

Junio C Hamano <junkio@cox.net> wrote:
> I was trying to see who have been active recently to find GSoC
> mentor candidates by running:
> 
> 	$ git shortlog -s -n --since=4.months | head -n 20

You aren't alone; I did the same stupid thing last night. Only without the --since=4.months argument. Which is the old calling convention. At least your little warning would have saved me. ;-)

<plug> We need GSoC project mentors!

Reading http://git.or.cz/gitwiki/SoC2007Ideas it appears as though (with the exception of 2 projects that Martin Langhoff is interested in, thanks Martin!) I am *the* GSoC mentor. I really hoped that we could get more community involvement than that. Junio's "top 20" is an interesting set of people, but there are still some very interesting and qualified names in the top 65...

Please, look over the SoC2007Ideas list on the wiki and stick your name next to a project or two that you have watched on the mailing list, are interested in seeing get added to Git, and would like to help out with this summer.

Being a GSoC mentor does have a time commitment. You will need to work with the student via email to help them find their way around the Git source code, and to help them refine their changes such that they are suitable for acceptance to the respective Git tree.

Remember, our goal here is to have a working enhancement that Junio would be proud to merge! </plug>

-- 
Shawn.
Alex Riesen· Mar 8, 2007, 16:04 UTC · re: Shawn O. Pearce · lore

Re: GSoC project mentors

On 3/8/07, Shawn O. Pearce <spearce@spearce.org> wrote:
>
> Being a GSoC mentor does have a time commitment.  You will need to

This is a problem. How about IRC schedules? Say, if some idea has not enough mentorship, it may have at least supporters on IRC who _commit_ themselves to that schedule. In this case the students can at least know when they are likely to get help.

Shawn O. Pearce· Mar 9, 2007, 00:59 UTC · re: Alex Riesen · lore

Re: GSoC project mentors

Alex Riesen <raa.lkml@gmail.com> wrote:
Show 8 quoted lines
> On 3/8/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> >
> >Being a GSoC mentor does have a time commitment.  You will need to
> 
> This is a problem. How about IRC schedules? Say, if some idea
> has not enough mentorship, it may have at least supporters on
> IRC who _commit_ themselves to that schedule. In this case
> the students can at least know when they are likely to get help.

Committing oneself to being on IRC is a time commitment, isn't it? :-)

IRC time is difficult for me; I do hang out there more often than I used to now that the community is doing the same, but I spend a good part of my week working from a network where IRC access isn't allowed.

Anyway, people did step up (thanks Pasky, Dscho, Brian, Martin, Martin!) and now most projects have more one mentor. This is exactly what I was hoping would happen.

-- 
Shawn.
Alex Riesen· Mar 9, 2007, 09:50 UTC · re: Shawn O. Pearce · lore

Re: GSoC project mentors

On 3/9/07, Shawn O. Pearce <spearce@spearce.org> wrote:
Show 12 quoted lines
> Alex Riesen <raa.lkml@gmail.com> wrote:
> > On 3/8/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> > >
> > >Being a GSoC mentor does have a time commitment.  You will need to
> >
> > This is a problem. How about IRC schedules? Say, if some idea
> > has not enough mentorship, it may have at least supporters on
> > IRC who _commit_ themselves to that schedule. In this case
> > the students can at least know when they are likely to get help.
>
> Committing oneself to being on IRC is a time commitment, isn't it?
> :-)

yes, but it is a smaller one. And the suggestion was to commit to specific date and time.

> IRC time is difficult for me; I do hang out there more often than
> I used to now that the community is doing the same, but I spend
> a good part of my week working from a network where IRC access
> isn't allowed.

Me too. But I would gladly spent some time on IRC when I'm home, which usually is after 20:00 CET. Especially if I promised to.

Shawn O. Pearce· Mar 9, 2007, 15:58 UTC · re: Alex Riesen · lore

Re: GSoC project mentors

Alex Riesen <raa.lkml@gmail.com> wrote:
Show 14 quoted lines
> On 3/9/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> >Committing oneself to being on IRC is a time commitment, isn't it?
> >:-)
> 
> yes, but it is a smaller one. And the suggestion was to commit to specific
> date and time.
> 
> >IRC time is difficult for me; I do hang out there more often than
> >I used to now that the community is doing the same, but I spend
> >a good part of my week working from a network where IRC access
> >isn't allowed.
> 
> Me too. But I would gladly spent some time on IRC when I'm home,
> which usually is after 20:00 CET. Especially if I promised to.

Looking at the way other successful projects have worked with GSoC, it seems to really pay to have a couple of mentors per student. The students don't always reach out to the mailing list right away when they don't understand something or are seeking feedback on an approach, as they might be shy or might be afraid of getting flamed by one of the "old hands". This is where having accessible mentors comes in nicely, as the mentor can help the student frame their message and get it into the public forum. Likewise having more than one mentor gives the student somewhere to turn when one of the mentors takes a week of vacation.

Most of our projects do have more than one mentor listed now (thanks guys!). As our group's coordinator I intend on trying to make sure every project has at least two mentors assigned to it and actively participating with the student. But given the way our community works, I fully expect the whole community to be involved as patches are posted and discussed on the mailing list. Students need to be interacting with all of us on git@vger, not just their mentors.

Having a fairly consistent presence on IRC would also be good. I'm not quite sure yet how I'm personally going to do that this summer scheduling wise, but I think it would be good if we can have a few individuals on #git on a regularly scheduled basis. I'd love to see you there, along with some of our other regulars.

-- 
Shawn.
Johannes Schindelin· Mar 8, 2007, 17:27 UTC · re: Shawn O. Pearce · lore

Re: GSoC project mentors

Hi,
On Thu, 8 Mar 2007, Shawn O. Pearce wrote:
> Please, look over the SoC2007Ideas list on the wiki and stick your name 
> next to a project or two that you have watched on the mailing list, are 
> interested in seeing get added to Git, and would like to help out with 
> this summer.
Done.
> Being a GSoC mentor does have a time commitment.  You will need to work 
> with the student via email to help them find their way around the Git 
> source code, and to help them refine their changes such that they are 
> suitable for acceptance to the respective Git tree.
Can't take more time than some of the discussions on the list...
> Remember, our goal here is to have a working enhancement that Junio 
> would be proud to merge!

Speaking of whom: Junio, if you don't do the backup administrator, I would be willing to.

Ciao, Dscho

Shawn O. Pearce· Mar 9, 2007, 02:26 UTC · re: Johannes Schindelin · lore

Re: GSoC project mentors

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Speaking of whom: Junio, if you don't do the backup administrator, I would 
> be willing to.

Martin Langhoff agreed first, so that is who I listed. I think he is also listed as mentor for fewer projects than you are. ;-)

-- 
Shawn.
Linus Torvalds· Mar 8, 2007, 16:37 UTC · re: Junio C Hamano · lore

Re: [PATCH] shortlog: prompt when reading from terminal by mistake

On Thu, 8 Mar 2007, Junio C Hamano wrote:
> 
> Not so.  "git shortlog" acts as a filter when no revs are given,
> unlike "git log" which defaults to HEAD.  It was reading from
> its standard input.
Could we just change that?

There aren't *that* many users of "git shortlog", I bet, and I'm not sure the "filter mode" is really worth it, especially since it ends up being confusing once you get used to using it as just another "git log" variant.

It was a filter not really because people wanted a filter, but for historical reasons, and because it wasn't really able to do things on its own, and it was just an external script...

Yeah, as a filter it *can* stil lbe useful, of course, but I suspect the usefullness is limited.

> +	if (rev.pending.nr == 0) {
> +		if (isatty(0))
> +			fprintf(stderr, "(reading log to summarize from standard input)\n");

Sure, this probably gets some cases (and the one you tried in particular), but if you pipe the output to a pager, I doubt it's actually all that useful.

To simulate a "wait a long time without output" case, try this:
	( echo "Important warning" >&2 ; sleep 10) | less -S

and at least for me, I don't see squat _in_case_I'm_at_the_top_ of the window, simply because "less" will clear the screen for me.

In other words, putting warnings on stderr usually isn't all that useful when you have something like a pager that will do tty stuff - it will easily overwrite the warning so quickly that you'll never see it at all.

So I think your patch is an improvement, but I suspect it would be a bigger improvement to just make "git shortlog" work as "git log". If we want to support the filtering mode, we could just make it be a separate command ("git shortlog-filter") instead?

			Linus
Johannes Schindelin· Mar 8, 2007, 17:10 UTC · re: Linus Torvalds · lore

Re: [PATCH] shortlog: prompt when reading from terminal by mistake

Hi,
On Thu, 8 Mar 2007, Linus Torvalds wrote:
Show 12 quoted lines
> On Thu, 8 Mar 2007, Junio C Hamano wrote:
> > 
> > Not so.  "git shortlog" acts as a filter when no revs are given, 
> > unlike "git log" which defaults to HEAD.  It was reading from its 
> > standard input.
> 
> Could we just change that?
>
> [...]
> 
> Yeah, as a filter it *can* stil lbe useful, of course, but I suspect the 
> usefullness is limited.
How about something totally different: "git log --shortlog". I.e.
Show changes to 2 files +5 −1

builtin-log.c, builtin-shortlog.c

diff --git a/builtin-log.c b/builtin-log.c
index 865832c..ad1dafd 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -216,7 +216,11 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
 int cmd_log(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info rev;
+	int i;
 
+	for (i = 1; i < argc; i++)
+		if (!strcmp(argv[i], "--shortlog"))
+			return cmd_shortlog(argc, argv, prefix);
 	git_config(git_log_config);
 	init_revisions(&rev, prefix);
 	rev.always_show_header = 1;
diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index 29b1636..952a475 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -289,7 +289,7 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 			summary = 1;
 		else if (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help"))
 			usage(shortlog_usage);
-		else
+		else if (strcmp(argv[1], "--shortlog"))
 			break;
 		argv++;
 		argc--;

Ciao,
Dscho
Matthieu Moy· Mar 8, 2007, 17:19 UTC · re: Johannes Schindelin · lore

Re: [PATCH] shortlog: prompt when reading from terminal by mistake

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> How about something totally different: "git log --shortlog". I.e.
Then, I'd say just --short.
-- 
Matthieu
Johannes Schindelin· Mar 8, 2007, 17:28 UTC · re: Matthieu Moy · lore

Re: [PATCH] shortlog: prompt when reading from terminal by mistake

Hi,
On Thu, 8 Mar 2007, Matthieu Moy wrote:
Show 5 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > How about something totally different: "git log --shortlog". I.e.
> 
> Then, I'd say just --short.
Too easy to confuse with "--pretty=short".

Ciao, Dscho

Linus Torvalds· Mar 8, 2007, 17:31 UTC · re: Johannes Schindelin · lore

Re: [PATCH] shortlog: prompt when reading from terminal by mistake

On Thu, 8 Mar 2007, Johannes Schindelin wrote:
> 
> How about something totally different: "git log --shortlog". I.e.
Sure, but this is even worse, because the way you wrote the patch, now
	git log --shortlog
will act as a filter too, which is even *more* confusing.
You also broke something even more fundamental. Try this:
	git log -- --shortlog
with your patch ;)

But yeah, if "cmd_shortlog()" worked like "git log" (at least if it sees the "--shortlog" flag) and with that obvious filename parsing thing fixed, that thing would be fine.

			Linus
Johannes Schindelin· Mar 8, 2007, 17:37 UTC · re: Linus Torvalds · lore

Re: [PATCH] shortlog: prompt when reading from terminal by mistake

Hi,
On Thu, 8 Mar 2007, Linus Torvalds wrote:
Show 5 quoted lines
> On Thu, 8 Mar 2007, Johannes Schindelin wrote:
> > 
> > How about something totally different: "git log --shortlog". I.e.
> 
> Sure, but this is even worse, because the way you wrote the patch

I should have marked it as "this is the idea, but even if it looks like a patch, it is not".

Unfortunately, I don't have time to do a proper patch, which is why I just wrote that simple and completely untested example of how this could be done.

I'd split off a proper builtin_shortlog(revs) from cmd_shortlog(), to avoid double parsing, and also because it looks nicer to the eye, but time is scarce.

Ciao, Dscho

Jakub Narebski· Mar 9, 2007, 14:41 UTC · re: Johannes Schindelin · lore

Re: [PATCH] shortlog: prompt when reading from terminal by mistake

Johannes Schindelin wrote:
Show 16 quoted lines
> On Thu, 8 Mar 2007, Linus Torvalds wrote:
> 
>> On Thu, 8 Mar 2007, Junio C Hamano wrote:
>>> 
>>> Not so.  "git shortlog" acts as a filter when no revs are given, 
>>> unlike "git log" which defaults to HEAD.  It was reading from its 
>>> standard input.
>> 
>> Could we just change that?
>>
>> [...]
>> 
>> Yeah, as a filter it *can* stil lbe useful, of course, but I suspect the 
>> usefullness is limited.
> 
> How about something totally different: "git log --shortlog". I.e.

Or have old filter behavior only with "git shortlog --stdin" option? Perhaps in git 1.5.1...

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Junio C Hamano· Mar 10, 2007, 00:39 UTC · re: Linus Torvalds · lore

Re: [PATCH] shortlog: prompt when reading from terminal by mistake

Linus Torvalds <torvalds@linux-foundation.org> writes:
Show 10 quoted lines
> Sure, this probably gets some cases (and the one you tried in particular), 
> but if you pipe the output to a pager, I doubt it's actually all that 
> useful.
>
> To simulate a "wait a long time without output" case, try this:
>
> 	( echo "Important warning" >&2 ; sleep 10) | less -S
>
> and at least for me, I don't see squat _in_case_I'm_at_the_top_ of the 
> window, simply because "less" will clear the screen for me.
> In other words, putting warnings on stderr usually isn't all that useful 
> when you have something like a pager that will do tty stuff - it will 
> easily overwrite the warning so quickly that you'll never see it at all.
That is correct in general, but you are being unfair.

You know git-shortlog has built-in pager _and_ you know _you_ added select(2) before spawning less, so that when that warning is emitted, less hasn't even started running --- it cannot clear the display.

← back to recent threads