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

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

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

>> How well does substr() work with utf-8 and other multi-byte encodings
>> these days, I have to wonder...
>
> Hopefully, it works well.

"Hopefully" is the last word I'd like to hear from submitters.  It would
be either "I do not know" or "I studied the topic and I know the code works".

> Here's another go, with your suggestions applied.

Sorry, this came too late for tonight's round.  I have a fixed-up one
based on your previous round parked in 'pu', which I'll be replacing with
this one (or your future re-submission if there is any) later in the week.

By the way, I noticed that you are sending your patches with:

    Content-Type: text/plain; charset=ISO-8859-1; format=flowed

Please don't.  format=flawed tends to destroy whitespaces (I fixed them up
by hand for the ones I parked in 'pu').

> +	# Add some user context, the first changed line that contains
> +	# some non-white character other than a bracket.
> +	for my $line (@{$rhunk->{TEXT}}) {
> +		if ($line =~ m/^([+-][][{}()\s]*[^][{}()\s])/) {

I would say "$line =~ /^[-+].*\w/" (i.e. match any +/- line that contains
a word letter) would be sufficient, and it would be much easier to read.

As you append the entire $line to $summary, there is no need to capture
with ().

> +# 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 if not $i;

I think "$i ||= 0" is more customary.