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

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

[Junio C Hamano]

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(-)

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);
 

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

[Johannes Schindelin]

Hi,

On Thu, 8 Mar 2007, Junio C Hamano 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
> 
> 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>

>    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

Re: GSoC project mentors

[Shawn O. Pearce]

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.

Re: GSoC project mentors

[Alex Riesen]

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.

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

[Linus Torvalds]

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

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

[Johannes Schindelin]

Hi,

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.

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

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

[Matthieu Moy]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> How about something totally different: "git log --shortlog". I.e.

Then, I'd say just --short.

-- 
Matthieu

Re: GSoC project mentors

[Johannes Schindelin]

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

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

[Johannes Schindelin]

Hi,

On Thu, 8 Mar 2007, Matthieu Moy wrote:

> 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

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

[Linus Torvalds]

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

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

[Johannes Schindelin]

Hi,

On Thu, 8 Mar 2007, Linus Torvalds wrote:

> 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

Re: GSoC project mentors

[Shawn O. Pearce]

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?
:-)

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.

Re: GSoC project mentors

[Shawn O. Pearce]

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.

Re: GSoC project mentors

[Alex Riesen]

On 3/9/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> 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.

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

[Jakub Narebski]

Johannes Schindelin wrote:

> 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

Re: GSoC project mentors

[Shawn O. Pearce]

Alex Riesen <raa.lkml@gmail.com> wrote:
> 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.

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

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

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