bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] gitlog-to-changelog: support multi-author commits.


From: Gary V. Vaughan
Subject: Re: [PATCH 1/2] gitlog-to-changelog: support multi-author commits.
Date: Thu, 17 Nov 2011 12:10:49 +0700

Hi Jim,

Thanks for the reviews.

On 17 Nov 2011, at 03:01, Jim Meyering wrote:
> Gary V. Vaughan wrote:
> ...
>> the parts that didn't work OOTB on my Mac to be portable.  Feel free to crib
>> those portable parts of this one into coreutils, or reformat this one to
>> coreutils style as you prefer.
>> 
>>> An alternative is to accept anything after the ":" and then, to use
>>> "s/\s*</  </" to ensure that the number of spaces before the "<" is two.
>>> 
>>>> +      s/^Co-authored-by:\s*([^<]+)\s+</\t    \1  </
>>> 
>>> Please use $1, not \1.
>> 
>> I thought $1 was a positional parameter?  I'm not sure what the advantage of
>> choosing a different back-reference syntax to other unix regexp using tools
>> is, but no matter... after taking your other suggestions, this line doesn't
>> exist any more :)
> 
> $1 is preferred in this context.
> I'm pretty sure Perl style guides say that, too.

Oh I'm not criticizing gnulib's use of $1 at all... but I do question the
wisdom of Perl adding yet another flavour of regexp syntax.  Still, that's
way off topic, and likely there are many layers of subtlety between my
understanding of Perl and the underlying reasons for a choice that seems
strange on the surface.

> Apply the following and you may push the result,
> but without the commit-msg script addition.
> 
> diff --git a/build-aux/gitlog-to-changelog b/build-aux/gitlog-to-changelog
> index 9d98b82..f5f6023 100755
> --- a/build-aux/gitlog-to-changelog
> +++ b/build-aux/gitlog-to-changelog
> @@ -259,9 +259,9 @@ sub parse_amend_file($)
>           s/^Co-authored-by:\s*/\t    /;
>           s/\s*</  </;
> 
> -          $_ =~ /<address@hidden>/
> -            or warn "$ME: warning: Missing email address for "
> -              . substr ($_, 5) . ".\n";
> +          /<address@hidden>/
> +            or warn "$ME: warning: missing email address for "
> +              . substr ($_, 5) . "\n";
>         }
> 
>       # If this header would be the same as the previous date/name/email/


Done.

Cheers,
-- 
Gary V. Vaughan (gary AT gnu DOT org)


reply via email to

[Prev in Thread] Current Thread [Next in Thread]