Re: [PATCH] Fix git-archimport on empty summary

From: Junio C Hamano <junkio@cox.net>
Date: Feb 28, 2007, 19:29 UTC
Message-ID: <7vd53u13p3.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <es39f9$sqh$1@sea.gmane.org>

Paolo Bonzini <bonzini@gnu.org> writes:

Show 11 quoted lines

> The patch seems pretty obvious to me, and Martin Langhoff has already
> seen it in private e-mail.
>    
> Thanks,
>
> Paolo
>
> 2007-02-27  Paolo Bonzini  <bonzini@gnu.org>
>
>         * git-archimport (parselog): Cope with an empty summary.
>

I would have liked a proposed commit message that is more in line with the rest of the git.git history.

The patch seems pretty obvious and does not do any _worse_ than the existing code.

Show 15 quoted lines

> --- /usr/bin/git-archimport     2007-01-09 21:15:39.000000000 +0100
> +++ ./git-archimport    2007-02-27 14:28:33.000000000 +0100
> @@ -780,7 +780,11 @@
>      }
>
>      # post-processing:
> -    $ps->{summary} = join("\n",@{$ps->{summary}})."\n";
> +    if (defined $ps->{summary}) {
> +        $ps->{summary} = join("\n",@{$ps->{summary}})."\n";
> +    } else {
> +        $ps->{summary} = "\n";
> +    }
>      $ps->{message} = join("\n",@$log);
>
>      # skip Arch control files, unescape pika-escaped files

However, I see that the result is used this way:

    my $pid = open2(*READER, *WRITER,'git-commit-tree',$tree,@par) 
        or die $!;
    print WRITER $ps->{summary},"\n";
    print WRITER $ps->{message},"\n";

What's the arch way of formatting log messages? Are summary lines expected to be multi-line? I see that there is a continuation line handling that builds @{$ps->{summary}} array. What does a summary line like this mean to an arch person?

	Summary: Fix git-archimport's handling of a commit that
	 lack a "Summary:" line.
 	Creator: Paolo Bonzini <bonzini@gnu.org>

Does it mean the Summary is logically two lines, or it is logically one line but linewrapped into two physical lines? If that is the case, I would almost suggest to update the part your patch touches to read like:

	chomp(@$log);
	while ($_ = shift @$log) {
        	...
	}
	# drop leading empty lines from the log message
	while (@$log && $log->[0] ne '') {
        	shift @$log;
	}
	if (exists $ps->{summary} && @{$ps->{summary}}) {
        	$ps->{summary} = join(' ', @{$ps->{summary}});
        }
	else {
        	$ps->{summary} = $log->[0] . '...';
	}
	$ps->{message} = join("\n",@$log);

and then update the user of these two to:

    my $pid = open2(*READER, *WRITER,'git-commit-tree',$tree,@par) 
        or die $!;
    print WRITER $ps->{summary},"\n\n";
    print WRITER $ps->{message},"\n";

That way, you will always have a one-line that can sensibly serve as a summary to appear in "git shortlog".