Re: summaries in git add --patch[PATCH 1/2]

[Junio C Hamano <gitster@pobox.com>]

William Pursell <bill.pursell@gmail.com> writes:

> Thanks for pointing that out.  Settings changed.  I do appreciate
> you taking the time to essentially hold my hand through this
> process, and hope that I'm not causing you too much extra work.

Heh, I'll be saving extra work I have to do in the future by training you
how to produce patches line the ones I may write myself.  By doing so,
eventually I wouldn't have to code anything myself ;-)

> +# Generate a one line summary of a hunk.
> +sub summarize_hunk {
> +	my $rhunk = shift;
> +	my $summary = $rhunk->{TEXT}[0];
> +
> +	# Keep the line numbers, discard extra context.
> +	$summary =~ s/@@(.*?)@@.*/$1 /s;
> +	$summary .= " " x (20 - length $summary);
> +
> +	# Add some user context.
> +	for my $line (@{$rhunk->{TEXT}}) {
> +		if ($line =~ m/^[+-].*\w/) {
> +			$summary .= $line;
> +			last;
> +		}
> +	}
> +
> +	chomp $summary;
> +	return substr($summary, 0, 80) . "\n";
> +}

I'll queue the patches in this round as-is in 'pu' and merge to 'next', as
we should stop slushing around at some point and start polishing on a
solid ground.  But as you mentioned, these hardcoded 20 and 80 do not look
very nice.

I think the division of labor between the data producer (summarize_hunk)
and presenter (display_hunks) should be shifted somewhat, so that

 * summarize_hunk returns a two-tuple:

	[ $line_number_hint, $first_change ]

 * display_hunks runs summarize_hunk for all 20 hunks and gathers the
   return values before producing a single line of output, and then
   computes the maximum $line_number_hint to decide how many extra SP to
   use to pad it to uniform length (instead of " " x (20 - length)).
   After doing so, it loops over the hunks, using the collected return
   values and formats.

In later round of polishing, you might find out that some callers of
summarize_hunk may want to read the full line, not just the first 80
(perhaps they feed their output to "less -S").  By splitting the
responsibility between these functions in the way outlined above, you do
not have to modify summarize_hunk when that day comes.

> +
> +
> +# Print a one-line summary of each hunk in the array ref in
> +# the first argument, starting wih the index in the 2nd.
> +sub display_hunks {
> +	my ($hunks, $i) = @_;
> +	my $ctr = 0;
> +	$i ||= 0;
> +	for (; $i < @$hunks && $ctr < 20; $i++, $ctr++) {
> +		my $status = " ";
> +		if (defined $hunks->[$i]{USE}) {
> +			$status = $hunks->[$i]{USE} ? "+" : "-";
> +		}
> +		printf "%s%2d: %s",
> +			$status,
> +			$i + 1,
> +			summarize_hunk($hunks->[$i]);
> +	}

By the way, I do not think this will align if you have more than 100
hunks.  That is also a reason why I would suggest not to format/substr
inside the summarize_hunk function.