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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 3, 2008, 23:23 UTC
Message-ID: <7v4p1kalno.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <4936EE68.6030009@gmail.com>

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

> From 57b5eab3f64a40ebe9aca122b5c6db1ab5c26116 Mon Sep 17 00:00:00 2001
> From: William Pursell <bill.pursell@gmail.com>
> Date: Wed, 3 Dec 2008 20:26:36 +0000
> Subject: [PATCH 2/2] Implemented 'g' command to goto a hunk.

s/ted/t/; or s/Implemented/Add/; s/goto/go to/;

Show 5 quoted lines

> When a minor change is made while the working directory is in a bit of a
> mess (and the user should have done a stash before making the minor
> edit, but didn't) it is somewhat difficult to wade through all of the
> hunks using git add --patch.  This allows one to jump to the hunk that
> needs to be staged without having to respond 'n' to each preceding hunk.

Yeah, even without forgotten stashing, you can be in a situation where you simply have many many changes all over in a file, and know exactly how the one you need to add to the index urgently looks like.

Show 9 quoted lines

> @@ -976,6 +980,27 @@ sub patch_update_file {
>  				}
>  				next;
>  			}
> +			elsif ($other =~ 'g' && $line =~ /^g(.*)/) {
> +				my $response = $1;
> +				my $i = 0;
> +				chomp $response;
> +				while (not $response) {

Did you mean "while ($response eq '')"? I do not think you want "g0<ret>" to fall into the loop.

> +					my $extra = "";
> +					$i = display_hunks (\@hunk, $i);

s/_hunks /_hunks/;

> +					$extra = "(<ret> to see more): " if ($i != $num);

This is probably just a matter of taste, but (1) Statement Modifiers are much harder to read than straightforward conditional blocks, and (2) loop termination condition is better written with magnitude comparison not with unequality test, when the variable approaches to the limit always from a known direction, so:

	if ($i < $num) {
        	$extra = "(<ret> to see more): ";
	}

> +					print "goto which hunk? $extra";

This placement of $extra looks a bit odd.

	goto which hunk? (<ret> to see more): *cursor blinking here*
        goto which hunk? *cursor blinking here*

Shouldn't it be like this?

	goto which hunk (<ret> to see more)? *cursor blinking here*

> +					$response = <STDIN>;
> +					chomp $response;
> +				}
> +				if ($response !~ /^\s*\d+$/) {

Why is " 1<ret>" allowed but not "1 <ret>"?

Show 11 quoted lines

> +					print STDERR "Invalid number: '$response'\n";
> +				} elsif (0 < $response && $response <= $num) {
> +					$ix = $response - 1;
> +				} else {
> +					print STDERR "Sorry, only $num hunks available.\n";
> +				}
> +				next;
> +			}
>  			elsif ($line =~ /^d/i) {
>  				while ($ix < $num) {
>  					if (!defined $hunk[$ix]{USE}) {