[PATCH] remove "[PATCH]" prefix from shortlog output

[PATCH] remove "[PATCH]" prefix from shortlog output

[Nicolas Pitre]

This doesn't convey much information in the shortlog context.
And the perl version did strip it as well.

Signed-off-by: Nicolas Pitre <nico@cam.org>

---

diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index 7a2ddfe..6c4606b 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -146,6 +146,11 @@ static void insert_author_oneline(struct path_list *list,
 	while (onelinelen > 0 && isspace(oneline[onelinelen - 1]))
 		onelinelen--;
 
+	if (onelinelen > 8 && !strncasecmp(oneline, "[PATCH] ", 8)) {
+		oneline += 8;
+		onelinelen -= 8;
+	}
+
 	buffer = xmalloc(onelinelen + 1);
 	memcpy(buffer, oneline, onelinelen);

Re: [PATCH] remove "[PATCH]" prefix from shortlog output

[Johannes Schindelin]

Hi,

On Sat, 9 Dec 2006, Nicolas Pitre wrote:

> This doesn't convey much information in the shortlog context.
> And the perl version did strip it as well.

From the current builtin-shortlog.c:128ff:

        if (!strncmp(oneline, "[PATCH", 6)) {
                char *eob = strchr(oneline, ']');

                if (eob) {
                        while (isspace(eob[1]) && eob[1] != '\n')
                                eob++;
                        if (eob - oneline < onelinelen) {
                                onelinelen -= eob - oneline;
                                oneline = eob;
                        }
                }
        }

It tries not only to strip "[PATCH]", but also "[PATCH 0/n]" and basically 
every prefix beginning with "[PATCH" and ending in "]". I do not remember 
if I really tested that code, but it should work.

Ciao,
Dscho

Re: [PATCH] remove "[PATCH]" prefix from shortlog output

[Jakub Narebski]

Johannes Schindelin wrote:

> On Sat, 9 Dec 2006, Nicolas Pitre wrote:
> 
>> This doesn't convey much information in the shortlog context.
>> And the perl version did strip it as well.
> 
> From the current builtin-shortlog.c:128ff:
> 
>         if (!strncmp(oneline, "[PATCH", 6)) {
>                 char *eob = strchr(oneline, ']');
> 
>                 if (eob) {
>                         while (isspace(eob[1]) && eob[1] != '\n')
>                                 eob++;
>                         if (eob - oneline < onelinelen) {
>                                 onelinelen -= eob - oneline;
>                                 oneline = eob;
>                         }
>                 }
>         }
> 
> It tries not only to strip "[PATCH]", but also "[PATCH 0/n]" and basically 
> every prefix beginning with "[PATCH" and ending in "]". I do not remember 
> if I really tested that code, but it should work.

What happens if you have "[PATCH" without closing "]"? Does it work (and
doesn't crash)? Does it strip anything?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] remove "[PATCH]" prefix from shortlog output

[Johannes Schindelin]

Hi,

On Mon, 11 Dec 2006, Jakub Narebski wrote:

> Johannes Schindelin wrote:
> 
> > On Sat, 9 Dec 2006, Nicolas Pitre wrote:
> > 
> >> This doesn't convey much information in the shortlog context.
> >> And the perl version did strip it as well.
> > 
> > From the current builtin-shortlog.c:128ff:
> > 
> >         if (!strncmp(oneline, "[PATCH", 6)) {
> >                 char *eob = strchr(oneline, ']');
> > 
> >                 if (eob) {
> >                         while (isspace(eob[1]) && eob[1] != '\n')
> >                                 eob++;
> >                         if (eob - oneline < onelinelen) {
> >                                 onelinelen -= eob - oneline;
> >                                 oneline = eob;
> >                         }
> >                 }
> >         }
> > 
> > It tries not only to strip "[PATCH]", but also "[PATCH 0/n]" and basically 
> > every prefix beginning with "[PATCH" and ending in "]". I do not remember 
> > if I really tested that code, but it should work.
> 
> What happens if you have "[PATCH" without closing "]"? Does it work (and
> doesn't crash)? Does it strip anything?

Use the source, Luke!

If "[PATCH" is there, but "]" is not, then eob will be NULL, and nothing 
happens.

Well, actually it is not completely true: the code searches for "]" in the 
_complete_ commit message (which _is_ terminated by '\0'). But only if it 
is found, _and_ it is in the first line, the substring is trimmed away.

Hth,
Dscho

Re: [PATCH] remove "[PATCH]" prefix from shortlog output

[Junio C Hamano]

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

> It tries not only to strip "[PATCH]", but also "[PATCH 0/n]" and basically 
> every prefix beginning with "[PATCH" and ending in "]". I do not remember 
> if I really tested that code, but it should work.

The problem is that you forgot that the lines are indented when
acting as a filter.

How about this?  It also contains the extra whitespace removal
from the author name.

diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index 7a2ddfe..3fc43dd 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -188,18 +188,25 @@ static void read_from_stdin(struct path_list *list)
 				bob = buffer + strlen(buffer);
 			else {
 				offset = 8;
-				if (isspace(bob[-1]))
+				while (buffer + offset < bob &&
+				       isspace(bob[-1]))
 					bob--;
 			}
 
 			while (fgets(buffer2, sizeof(buffer2), stdin) &&
 					buffer2[0] != '\n')
 				; /* chomp input */
-			if (fgets(buffer2, sizeof(buffer2), stdin))
+			if (fgets(buffer2, sizeof(buffer2), stdin)) {
+				int l2 = strlen(buffer2);
+				int i;
+				for (i = 0; i < l2; i++)
+					if (!isspace(buffer2[i]))
+						break;
 				insert_author_oneline(list,
 						buffer + offset,
 						bob - buffer - offset,
-						buffer2, strlen(buffer2));
+						buffer2 + i, l2 - i);
+			}
 		}
 	}
 }

Re: [PATCH] remove "[PATCH]" prefix from shortlog output

[Johannes Schindelin]

Hi,

On Sun, 10 Dec 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > It tries not only to strip "[PATCH]", but also "[PATCH 0/n]" and basically 
> > every prefix beginning with "[PATCH" and ending in "]". I do not remember 
> > if I really tested that code, but it should work.
> 
> The problem is that you forgot that the lines are indented when
> acting as a filter.

Of course!

And your patch looks correct to me.
Ciao,

Re: [PATCH] remove "[PATCH]" prefix from shortlog output

[Nicolas Pitre]

On Mon, 11 Dec 2006, Johannes Schindelin wrote:

> Hi,
> 
> On Sun, 10 Dec 2006, Junio C Hamano wrote:
> 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > 
> > > It tries not only to strip "[PATCH]", but also "[PATCH 0/n]" and basically 
> > > every prefix beginning with "[PATCH" and ending in "]". I do not remember 
> > > if I really tested that code, but it should work.

Hmmm well, right.  And of course I failed to notice that code was there.

I think I was corrupted by the dark side and failed to properly "use the 
source" twice on that tool this week.

> > The problem is that you forgot that the lines are indented when
> > acting as a filter.

Which was indeed my use case.

> Of course!
> 
> And your patch looks correct to me.

Confirmed.