{"thread":{"id":"12517","subject":"[PATCH] shortlog: take the first populated line of the description","startedAt":"2008-03-05T13:47:29Z","lastAt":"2008-03-05T22:14:10Z","messageCount":8,"participants":["Andy Whitcroft","Johannes Schindelin","Nicolas Pitre","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"71075","messageId":"1204727050.0@pinky","threadId":"12517","inReplyTo":null,"subject":"[PATCH] shortlog: take the first populated line of the description","fromName":"Andy Whitcroft","fromEmail":"apw@shadowen.org","sentAt":null,"receivedAt":"2008-03-05T13:47:29Z","isPatch":true,"sender":{"key":"apw@shadowen.org","avatar":"https://gravatar.com/avatar/d3088262854661a913ef35cc40fedcc270142d4461791142bc1ea0b2a4e2e147?d=mp&s=160"},"body":"\nWay back the perl version of shortlog would take the first populated line\nof the commit body. The builtin version mearly takes the first line.\nThis leads to empty shortlog entries when there is some viable text in\nthe commit.\n\nReinstate this behaviour igoring all lines with nothing but whitespace.\n\nSigned-off-by: Andy Whitcroft \n---\n\n\tThis seems to be an improvement, returning to the original\n\tbehaviour. I cannot think of any good reason not to take the first\n\tpopulated line for a shortlog.\tThe alternative less agressive\n\tcompromise might be to skip only completly empty lines at the\n\tstart, but I am not sure that adds any value.\n\n\tI seem to get a lot of these in converted SVN commits.\n\n\tComments?\n\n\t-apw\n---\n builtin-shortlog.c | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\ndiff --git a/builtin-shortlog.c b/builtin-shortlog.c\nindex af31aba..b22b0ed 100644\n--- a/builtin-shortlog.c\n+++ b/builtin-shortlog.c\n@@ -70,11 +70,12 @@ static void insert_one_record(struct shortlog *log,\n \telse\n \t\tfree(buffer);\n \n+\t/* Skip any leading whitespace, including any blank lines. */\n+\twhile (*oneline && isspace(*oneline))\n+\t\toneline++;\n \teol = strchr(oneline, '\\n');\n \tif (!eol)\n \t\teol = oneline + strlen(oneline);\n-\twhile (*oneline && isspace(*oneline) && *oneline != '\\n')\n-\t\toneline++;\n \tif (!prefixcmp(oneline, \"[PATCH\")) {\n \t\tchar *eob = strchr(oneline, ']');\n \t\tif (eob && (!eol || eob < eol))\n"},{"id":"71080","messageId":"alpine.LSU.1.00.0803051544160.15786@racer.site","threadId":"12517","inReplyTo":"1204727050.0@pinky","subject":"Re: [PATCH] shortlog: take the first populated line of the description","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-05T14:48:00Z","receivedAt":"2008-03-05T14:48:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 5 Mar 2008, Andy Whitcroft wrote:\n\n> diff --git a/builtin-shortlog.c b/builtin-shortlog.c\n> index af31aba..b22b0ed 100644\n> --- a/builtin-shortlog.c\n> +++ b/builtin-shortlog.c\n> @@ -70,11 +70,12 @@ static void insert_one_record(struct shortlog *log,\n> \telse\n> \t\tfree(buffer);\n> \n> +\t/* Skip any leading whitespace, including any blank lines. */\n> +\twhile (*oneline && isspace(*oneline))\n> +\t\toneline++;\n> \teol = strchr(oneline, '\\n');\n> \tif (!eol)\n> \t\teol = oneline + strlen(oneline);\n> -\twhile (*oneline && isspace(*oneline) && *oneline != '\\n')\n> -\t\toneline++;\n> \tif (!prefixcmp(oneline, \"[PATCH\")) {\n> \t\tchar *eob = strchr(oneline, ']');\n> \t\tif (eob && (!eol || eob < eol))\n\nWhy do you move the code around? Makes it harder to read your patch. \nBesides, you now strip empty lines at the beginning of the commit \nmessages, right? Who produces such a thing?\n\nCiao,\nDscho\n\n"},{"id":"71098","messageId":"20080305164316.GH17931@shadowen.org","threadId":"12517","inReplyTo":"alpine.LSU.1.00.0803051544160.15786@racer.site","subject":"Re: [PATCH] shortlog: take the first populated line of the description","fromName":"Andy Whitcroft","fromEmail":"apw@shadowen.org","sentAt":"2008-03-05T16:43:22Z","receivedAt":"2008-03-05T16:43:22Z","isPatch":true,"sender":{"key":"apw@shadowen.org","avatar":"https://gravatar.com/avatar/d3088262854661a913ef35cc40fedcc270142d4461791142bc1ea0b2a4e2e147?d=mp&s=160"},"body":"On Wed, Mar 05, 2008 at 03:48:00PM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Wed, 5 Mar 2008, Andy Whitcroft wrote:\n> \n> > diff --git a/builtin-shortlog.c b/builtin-shortlog.c\n> > index af31aba..b22b0ed 100644\n> > --- a/builtin-shortlog.c\n> > +++ b/builtin-shortlog.c\n> > @@ -70,11 +70,12 @@ static void insert_one_record(struct shortlog *log,\n> > \telse\n> > \t\tfree(buffer);\n> > \n> > +\t/* Skip any leading whitespace, including any blank lines. */\n> > +\twhile (*oneline && isspace(*oneline))\n> > +\t\toneline++;\n> > \teol = strchr(oneline, '\\n');\n> > \tif (!eol)\n> > \t\teol = oneline + strlen(oneline);\n> > -\twhile (*oneline && isspace(*oneline) && *oneline != '\\n')\n> > -\t\toneline++;\n> > \tif (!prefixcmp(oneline, \"[PATCH\")) {\n> > \t\tchar *eob = strchr(oneline, ']');\n> > \t\tif (eob && (!eol || eob < eol))\n> \n> Why do you move the code around? Makes it harder to read your patch. \n> Besides, you now strip empty lines at the beginning of the commit \n> messages, right? Who produces such a thing?\n\nI've not moved the code as such. I added a loop to drop the leading\nwhitespace. That made the second loop redundant as its job is already\ndone, so I killed it.\n\nThe point of the patch is to strip the empty lines at the start of\nthe commit. I am ending up with them in my repo mostly due to\nimcompetant users of SVN I suspect. The main driver is that I have\nthose and the original non-C version coped and the builtin does not.\n\nNow if people think that its a stupid idea I might suggest it could be\noptional?\n\n-apw\n"},{"id":"71100","messageId":"alpine.LSU.1.00.0803051750030.15786@racer.site","threadId":"12517","inReplyTo":"20080305164316.GH17931@shadowen.org","subject":"Re: [PATCH] shortlog: take the first populated line of the description","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-05T16:51:02Z","receivedAt":"2008-03-05T16:51:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 5 Mar 2008, Andy Whitcroft wrote:\n\n> On Wed, Mar 05, 2008 at 03:48:00PM +0100, Johannes Schindelin wrote:\n> \n> > On Wed, 5 Mar 2008, Andy Whitcroft wrote:\n> > \n> > > diff --git a/builtin-shortlog.c b/builtin-shortlog.c\n> > > index af31aba..b22b0ed 100644\n> > > --- a/builtin-shortlog.c\n> > > +++ b/builtin-shortlog.c\n> > > @@ -70,11 +70,12 @@ static void insert_one_record(struct shortlog *log,\n> > > \telse\n> > > \t\tfree(buffer);\n> > > \n> > > +\t/* Skip any leading whitespace, including any blank lines. */\n> > > +\twhile (*oneline && isspace(*oneline))\n> > > +\t\toneline++;\n> > > \teol = strchr(oneline, '\\n');\n> > > \tif (!eol)\n> > > \t\teol = oneline + strlen(oneline);\n> > > -\twhile (*oneline && isspace(*oneline) && *oneline != '\\n')\n> > > -\t\toneline++;\n> > > \tif (!prefixcmp(oneline, \"[PATCH\")) {\n> > > \t\tchar *eob = strchr(oneline, ']');\n> > > \t\tif (eob && (!eol || eob < eol))\n> > \n> > Why do you move the code around? Makes it harder to read your patch. \n> > Besides, you now strip empty lines at the beginning of the commit \n> > messages, right? Who produces such a thing?\n> \n> I've not moved the code as such.\n\nWell, it sure looks like that. Maybe you want to make it look like that \neven more?\n\n> The point of the patch is to strip the empty lines at the start of the \n> commit. I am ending up with them in my repo mostly due to imcompetant \n> users of SVN I suspect. The main driver is that I have those and the \n> original non-C version coped and the builtin does not.\n> \n> Now if people think that its a stupid idea I might suggest it could be \n> optional?\n\nNo, I think with an explanation like this in the commit message, it is \ngood as-is.\n\nCiao,\nDscho\n\n"},{"id":"71104","messageId":"alpine.LFD.1.00.0803051346200.2947@xanadu.home","threadId":"12517","inReplyTo":"1204727050.0@pinky","subject":"Re: [PATCH] shortlog: take the first populated line of the description","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-03-05T18:47:22Z","receivedAt":"2008-03-05T18:47:22Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 5 Mar 2008, Andy Whitcroft wrote:\n\n> \n> Way back the perl version of shortlog would take the first populated line\n> of the commit body. The builtin version mearly takes the first line.\n> This leads to empty shortlog entries when there is some viable text in\n> the commit.\n> \n> Reinstate this behaviour igoring all lines with nothing but whitespace.\n> \n> Signed-off-by: Andy Whitcroft \n> ---\n> \n> \tThis seems to be an improvement, returning to the original\n> \tbehaviour. I cannot think of any good reason not to take the first\n> \tpopulated line for a shortlog.\tThe alternative less agressive\n> \tcompromise might be to skip only completly empty lines at the\n> \tstart, but I am not sure that adds any value.\n> \n> \tI seem to get a lot of these in converted SVN commits.\n> \n> \tComments?\n\nMaybe it is the SVN conversion that needs fixing?\n\n\nNicolas\n"},{"id":"71127","messageId":"7vy78wlv46.fsf@gitster.siamese.dyndns.org","threadId":"12517","inReplyTo":"alpine.LFD.1.00.0803051346200.2947@xanadu.home","subject":"Re: [PATCH] shortlog: take the first populated line of the description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-05T21:27:21Z","receivedAt":"2008-03-05T21:27:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre writes:\n\n> On Wed, 5 Mar 2008, Andy Whitcroft wrote:\n> ...\n>> \t... I cannot think of any good reason not to take the first\n>> \tpopulated line for a shortlog.\tThe alternative less agressive\n>> \tcompromise might be to skip only completly empty lines at the\n>> \tstart, but I am not sure that adds any value.\n>> \n>> \tI seem to get a lot of these in converted SVN commits.\n>> \n>> \tComments?\n>\n> Maybe it is the SVN conversion that needs fixing?\n\nI thought about saying the same, but I am of two minds.\n\nIt is likely that you would want to clean-up when importing,\nespecially when you are planning to abandon the other system and\nswitch to git. But you may want to have an import that is as\nclose to the original as possible, excess blank lines in the log\nmessages and all.\n\nI think Andy's fix to make the output side take away\nunnecessary blank lines is unconditionally good.\n\nI've added these three lines at the end of the log message.\n\n This is often useful when dealing with commits imported from foreign SCMs\n that do not tidy up the log message of useless blank lines at the\n beginning.\n"},{"id":"71130","messageId":"alpine.LFD.1.00.0803051638150.2947@xanadu.home","threadId":"12517","inReplyTo":"7vy78wlv46.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] shortlog: take the first populated line of the description","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-03-05T21:47:29Z","receivedAt":"2008-03-05T21:47:29Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 5 Mar 2008, Junio C Hamano wrote:\n\n> Nicolas Pitre writes:\n> \n> > On Wed, 5 Mar 2008, Andy Whitcroft wrote:\n> > ...\n> >> \t... I cannot think of any good reason not to take the first\n> >> \tpopulated line for a shortlog.\tThe alternative less agressive\n> >> \tcompromise might be to skip only completly empty lines at the\n> >> \tstart, but I am not sure that adds any value.\n> >> \n> >> \tI seem to get a lot of these in converted SVN commits.\n> >> \n> >> \tComments?\n> >\n> > Maybe it is the SVN conversion that needs fixing?\n> \n> I thought about saying the same, but I am of two minds.\n> \n> It is likely that you would want to clean-up when importing,\n> especially when you are planning to abandon the other system and\n> switch to git. But you may want to have an import that is as\n> close to the original as possible, excess blank lines in the log\n> messages and all.\n\nI don't think excess blank lines at the _beginning_ of the commit log \nhas any value worth preserving though.\n\n> I think Andy's fix to make the output side take away\n> unnecessary blank lines is unconditionally good.\n\nIt is not \"bad\" in itself. However that feels like papering over \nanother problem which IMHO has greater merits to be fixed. We have \ngiven special semantics to the first line of a commit log in many other \nplaces now, so unless all those places are also made aware of \nsubstandard commit logs too, I think it would be more productive to make \nsure those logs are semi sensible upon entering Git in the first place \ninstead.\n\n\nNicolas\n"},{"id":"71131","messageId":"7vmypclsy5.fsf@gitster.siamese.dyndns.org","threadId":"12517","inReplyTo":"alpine.LFD.1.00.0803051638150.2947@xanadu.home","subject":"Re: [PATCH] shortlog: take the first populated line of the description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-05T22:14:10Z","receivedAt":"2008-03-05T22:14:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre writes:\n\n> On Wed, 5 Mar 2008, Junio C Hamano wrote:\n>\n>> I think Andy's fix to make the output side take away\n>> unnecessary blank lines is unconditionally good.\n>\n> It is not \"bad\" in itself. However that feels like papering over \n> another problem which IMHO has greater merits to be fixed. We have \n> given special semantics to the first line of a commit log in many other \n> places now, so unless all those places are also made aware of \n> substandard commit logs too, I think it would be more productive to make \n> sure those logs are semi sensible upon entering Git in the first place \n> instead.\n\nMy understanding of the current status when I wrote that message\nwas that everybody at commit.c layer (that is, everybody except\nshortlog) strips such garbage. Do you have a specific broken\none in mind?\n\nI suspect --pretty=format:%s might be broken; I didn't check.\n\n"}]}