[PATCH 2/3] Add / command in add --patch

William Pursell· Nov 27, 2008, 04:07 UTC · lore

This command allows the user to skip hunks that don't match the specified regex.

BUG:  if the user enters an invalid regex, perl will abort.
For example: /+\s*foo will abort with:
Quantifier follows nothing in regex
I am not a Perl hacker and would welcome suggestions
on the easiest way to deal with this.

Signed-off-by: William Pursell <bill.pursell@gmail.com>
---
  git-add--interactive.perl |   27 +++++++++++++++++++++++----
  1 files changed, 23 insertions(+), 4 deletions(-)

Show changes to git-add--interactive.perl +23 −4

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index f20b880..547b5c8 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -800,6 +800,7 @@ y - stage this hunk
  n - do not stage this hunk
  a - stage this and all the remaining hunks in the file
  d - do not stage this hunk nor any of the remaining hunks in the file
+/ - search for a hunk matching the given regex
  j - leave this hunk undecided, see next undecided hunk
  J - leave this hunk undecided, see next hunk
  k - leave this hunk undecided, see previous undecided hunk
@@ -876,12 +877,14 @@ sub patch_update_file {

  	$num = scalar @hunk;
  	$ix = 0;
+	my $search_s; # User entered string to match a hunk.

  	while (1) {
  		my ($prev, $next, $other, $undecided, $i);
  		$other = '';

  		if ($num <= $ix) {
+			undef $search_s;
  			$ix = 0;
  		}
  		for ($i = 0; $i < $ix; $i++) {
@@ -916,11 +919,24 @@ sub patch_update_file {
  			$other .= ',s';
  		}
  		$other .= ',e';
-		for (@{$hunk[$ix]{DISPLAY}}) {
-			print;
+
+		my $line;
+		if (defined $search_s) {
+			my $text = join ("", @{$hunk[$ix]{DISPLAY}});
+			if ($text !~ $search_s) {
+				$line = "j\n";
+			} else {
+				print $text;
+			}
+		} else {
+			for (@{$hunk[$ix]{DISPLAY}}) {
+				print;
+			}
+		}
+		if (!$line) {
+			print colored $prompt_color, "Stage this hunk [y,n,a,d,/$other,?]? ";
+			$line = <STDIN>;
  		}
-		print colored $prompt_color, "Stage this hunk [y,n,a,d$other,?]? ";
-		my $line = <STDIN>;
  		if ($line) {
  			if ($line =~ /^y/i) {
  				$hunk[$ix]{USE} = 1;
@@ -946,6 +962,9 @@ sub patch_update_file {
  				}
  				next;
  			}
+			elsif ($line =~ m|^/(.*)|) {
+				$search_s = qr{$1}m;
+			}
  			elsif ($other =~ /K/ && $line =~ /^K/) {
  				$ix--;
  				next;

-- 
1.6.0.4.782.geea74.dirty


-- 
William Pursell

Junio C Hamano· Nov 27, 2008, 22:50 UTC · re: William Pursell · lore

Re: [PATCH 2/3] Add / command in add --patch

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

Show 6 quoted lines

> This command allows the user to skip hunks that don't
> match the specified regex.
>
> BUG:  if the user enters an invalid regex, perl will abort.
> For example: /+\s*foo will abort with:
> Quantifier follows nothing in regex

I think that is a lessor bug that can be fixed more easily. I think the bigger problem with your patch is that it breaks the code structure.

If you look at the existing code, you would notice that the loop is structured in such a way that we show the hunk we currently have focus, get a command from the user, and the command decides what to do with the hunk we have focus (no-op for many of them, flip {USE} bit for some) and where to move the focus (many increments $ix, some decrements $ix). The "find" command is about not doing anything to {USE} bit and moving the focus to the hunk that has the text, so you have your additional code touching wrong section of the code.

I'd suggest redoing [2/3] like this.

 git-add--interactive.perl |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

Show changes to diff +24 −1

diff --git c/git-add--interactive.perl w/git-add--interactive.perl
index f20b880..17724d1 100755
--- c/git-add--interactive.perl
+++ w/git-add--interactive.perl
@@ -800,6 +800,7 @@ y - stage this hunk
 n - do not stage this hunk
 a - stage this and all the remaining hunks in the file
 d - do not stage this hunk nor any of the remaining hunks in the file
+/ - search for a hunk matching the given regex
 j - leave this hunk undecided, see next undecided hunk
 J - leave this hunk undecided, see next hunk
 k - leave this hunk undecided, see previous undecided hunk
@@ -919,7 +920,7 @@ sub patch_update_file {
 		for (@{$hunk[$ix]{DISPLAY}}) {
 			print;
 		}
-		print colored $prompt_color, "Stage this hunk [y,n,a,d$other,?]? ";
+		print colored $prompt_color, "Stage this hunk [y,n,a,d,/$other,?]? ";
 		my $line = <STDIN>;
 		if ($line) {
 			if ($line =~ /^y/i) {
@@ -946,6 +947,28 @@ sub patch_update_file {
 				}
 				next;
 			}
+			elsif ($line =~ m|^/(.*)|) {
+				my $search_string;
+				eval {
+					$search_string = qr{$1}m;
+				};
+				if ($@) {
+					my ($err,$exp) = ($@, $1);
+					$err =~ s/ at .*git-add--interactive line \d+, <STDIN> line \d+.*$//;
+					print STDERR "Malformed search regexp $exp: $err\n";
+					next;
+				}
+				my $iy = $ix;
+				while (1) {
+					my $text = join ("", @{$hunk[$iy]{TEXT}});
+					last if ($text =~ $search_string);
+					$iy++;
+					$iy = 0 if ($iy >= $num);
+					last if ($ix == $iy);
+				}
+				$ix = $iy;
+				next;
+			}
 			elsif ($other =~ /K/ && $line =~ /^K/) {
 				$ix--;
 				next;

patch, 3 partsAdd / command in add --patch