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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 4, 2008, 08:47 UTC
Message-ID: <7vvdu0e38a.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <49377ED8.4050905@gmail.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 ().

Show 6 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 if not $i;

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