Re: gitlog-to-changelog option --tear-off

[Jim Meyering]

Werner Koch wrote:
> Is there a chance that my patch will be applied?

Hi Werner,
Thanks for the patch.

> Subject: [PATCH] gitlog-to-changelog: add option --tear-off.
>
> This option allows to have blurbs in a commit messages, which shall
> not be copied to the ChangeLog.  It can also be used to suppress an
> entire log entry.
> * build-aux/gitlog-to-changelog: New option --tear-off.
> ---
>  build-aux/gitlog-to-changelog |   24 ++++++++++++++++++++++++
>  1 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/build-aux/gitlog-to-changelog b/build-aux/gitlog-to-changelog
> index 38c6f3a..d55758b 100755
> --- a/build-aux/gitlog-to-changelog
> +++ b/build-aux/gitlog-to-changelog
> @@ -68,6 +68,8 @@ OPTIONS:
>                    header; the default is to cluster adjacent commit messages
>                    if their headers are the same and neither commit message
>                    contains multiple paragraphs.
> +   --tear-off   tear off all commit log lines after a '--' line and
> +                  skip log entries with the first body line being '--'.

Hard-coding "--"...
What do you think about factoring out the /--\s*/ regexp, or even
making it an option, in case someone has logs in which existing
"--" lines would cause parts of entries to be elided?

Does anyone else think they might use this option?

>     --since=DATE convert only the logs since DATE;
>                    the default is to convert all log entries.
>     --format=FMT set format string for commit subject and body;
> @@ -195,6 +197,7 @@ sub parse_amend_file($)
>    my $amend_file;
>    my $append_dot = 0;
>    my $cluster = 1;
> +  my $tear_off = 0;
>    GetOptions
>      (
>       help => sub { usage 0 },
> @@ -204,6 +207,7 @@ sub parse_amend_file($)
>       'amend=s' => \$amend_file,
>       'append-dot' => \$append_dot,
>       'cluster!' => \$cluster,
> +     'tear-off' => \$tear_off,
>      ) or usage 1;
>
>
> @@ -236,6 +240,13 @@ sub parse_amend_file($)
>        $n_read == $log_nbytes
>          or die "$ME:$.: unexpected EOF\n";
>
> +      # Skip log entries if the body starts with a tear off marker.
> +      if ($tear_off)
> +        {
> +          $log =~ /^.*\n\n.*\n--\s*/
> +            and goto SKIPCOMMIT;
> +        }

I would write the above differently:
(note the bug-fix, too: without the trailing \n I've appended,
 your version would elide any entry whose log body starts with "--",
 no matter what follows it)

      # Skip the entire log entry if the body starts with a tear off marker.
      $tear_off && $log =~ /^.*\n\n.*\n--\s*\n/
        and goto SKIPCOMMIT;

>        # Extract leading hash.
>        my ($sha, $rest) = split ':', $log, 2;
>        defined $sha
> @@ -286,6 +297,18 @@ sub parse_amend_file($)
>                         |Copyright-paperwork-exempt:[ ]
>                         )/x, @line;
>
> +      # Remove everything after a line with 2 dashes at the beginning.
> +      if ($tear_off)
> +        {
> +           my @tmpline;
> +           foreach (@line)
> +             {
> +              last if /^--\s*$/;
> +               push @tmpline,$_;
> +             }

Indentation nit, and I prefer to highlight conditionals
by putting the "then" part on a second line:

              foreach (@line)
                {
                  /^--\s*$/
                    and last;
                  push @tmpline, $_;
                }

> +           @line = @tmpline;
> +        }
> +
>        # Remove leading and trailing blank lines.
>        if (@line)
>          {
> @@ -353,6 +376,7 @@ sub parse_amend_file($)
>            print "\n", join ("\n", @line), "\n";
>          }
>
> +    SKIPCOMMIT:
>        defined ($in = <PIPE>)
>          or last;
>        $in ne "\n"
> --
> 1.7.7.1