From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 04 Apr 2012 21:38:55 GMT
Subject: Re: [PATCH v3 1/8] gitweb: Use descriptive names in esc_html_hl_regions()
Message-ID: <7vwr5v6uts.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1333569433-3245-2-git-send-email-michal.kiedrowicz@gmail.com>

Michał Kiedrowicz <michal.kiedrowicz@gmail.com> writes:

> The $s->[0] and $s->[1] variables look a bit cryptic.  Let's rename them
> to $beg and $end so that it's clear what they do.

Why not $begin and $end?

>
> Suggested-by: Jakub Narębski <jnareb@gmail.com>
> Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
> ---
>  gitweb/gitweb.perl |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index a8b5fad..a3754ff 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1738,12 +1738,14 @@ sub esc_html_hl_regions {
>  	my $pos = 0;
>  
>  	for my $s (@sel) {
> -		$out .= esc_html(substr($str, $pos, $s->[0] - $pos))
> -			if ($s->[0] - $pos > 0);
> +		my ($beg, $end) = @$s;
> +
> +		$out .= esc_html(substr($str, $pos, $beg - $pos))
> +			if ($beg - $pos > 0);
>  		$out .= $cgi->span({-class => $css_class},
> -		                   esc_html(substr($str, $s->[0], $s->[1] - $s->[0])));
> +		                   esc_html(substr($str, $beg, $end - $beg)));
>  
> -		$pos = $s->[1];
> +		$pos = $end;
>  	}
>  	$out .= esc_html(substr($str, $pos))
>  		if ($pos < length($str));