Skip to contentSkip to conversation

In add --patch, Handle K,k,J,j slightly more gracefully.

PATCH In review William Pursell 2 participants 2 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 2/2 In add --patch, Handle K,k,J,j slightly more gracefully.

Instead of printing the help menu, this will print "No next hunk" and then process the given hunk again.

---
 git-add--interactive.perl |   35 +++++++++++++++++++----------------
 1 files changed, 19 insertions(+), 16 deletions(-)
git-add--interactive.perl +19 -16
index 7ad4ee0..6321bd9 100755
@@ -964,29 +964,32 @@ sub patch_update_file {
 			elsif ($line =~ m|^/(.*)|) {
 				$search_s = $1;
 			}
-			elsif ($other =~ /K/ && $line =~ /^K/) {
-				$ix--;
+			elsif ( $line =~ /^K/) {
+				$other =~ /K/ ?  $ix-- : print STDERR "No previous hunk\n";
 				next;
 			}
-			elsif ($other =~ /J/ && $line =~ /^J/) {
-				$ix++;
+			elsif ($line =~ /^J/) {
+				$other =~ /J/ ?  $ix++ : print STDERR "No next hunk\n";
 				next;
 			}
-			elsif ($other =~ /k/ && $line =~ /^k/) {
-				while (1) {
-					$ix--;
-					last if (!$ix ||
-						 !defined $hunk[$ix]{USE});
+			elsif ($line =~ /^k/) {
+				if ($other =~ /k/) {
+					while (1) {
+						$ix--;
+						last if (!$ix ||
+							 !defined $hunk[$ix]{USE});
+					}
+					next;
+				}
+				else {
+					print STDERR "No previous hunk\n";
 				}
-				next;
 			}
-			elsif ($other =~ /j/ && $line =~ /^j/) {
-				while (1) {
-					$ix++;
-					last if ($ix >= $num ||
-						 !defined $hunk[$ix]{USE});
+			elsif ($line =~ /^j/) {
+				if ($other !~ /j/) {
+					print STDERR "No next hunk\n";
+					next;
 				}
-				next;
 			}
 			elsif ($other =~ /s/ && $line =~ /^s/) {
 				my @split = split_hunk($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY});
Signature · 5 lines
1.6.0.4.781.gf2070.dirty


-- 
William Pursell
#1 lore raw
Quoted 3 lines · William Pursell

Instead of printing the help menu, this will print "No next hunk" and then process the given hunk again.
---

Missing sign-off.

git-add--interactive.perl sub patch_update_file { · line 968

Quoted 4 lines
git-add--interactive.perl +2 -2
-			elsif ($other =~ /K/ && $line =~ /^K/) {
-				$ix--;
+			elsif ( $line =~ /^K/) {
+				$other =~ /K/ ?  $ix-- : print STDERR "No previous hunk\n";

This may be cute but I think it is harder to read than necessary.

	if ($other =~ /K/) {
        	$ix--;
        } else {
		print STDERR "No previous hunk\n";
        }
Quoted 10 lines
git-add--interactive.perl +10 -0
+			elsif ($line =~ /^k/) {
+				if ($other =~ /k/) {
+					while (1) {
+						$ix--;
+						last if (!$ix ||
+							 !defined $hunk[$ix]{USE});
+					}
+					next;
+				}
+				else {

git-add--interactive.perl sub patch_update_file { · line 993

Quoted 10 lines
git-add--interactive.perl +5 -1
+					print STDERR "No previous hunk\n";
 				}
-				next;
 			}
+			elsif ($line =~ /^j/) {
+				if ($other !~ /j/) {
+					print STDERR "No next hunk\n";
+					next;
 				}
 			}

Doesn't this behave differently against an unavailable 'k' and 'j'? When the user says 'j' when no next hunk is available, it will ask again about that hunk, but when the user says 'k' at the first hunk, because you discarded 'next', it will ask about the next hunk, no? Shouldn't it ask again about the same hunk, against which the user gave an erroneous input?