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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 5, 2008, 02:23 UTC
Message-ID: <7viqpz1hsa.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <4937B456.7080604@gmail.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 ;-)

Show 20 quoted lines

> +# 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.

Show 18 quoted lines

> +
> +
> +# 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.