[PATCH 1/3] git-rev-list: Add regexp tuning options

Subsystems: documentation, the rest

DORMANTno replies

3 messages, 1 author, 2016-06-15 · open the first message on its own page

[PATCH 1/3] git-rev-list: Add regexp tuning options

From: Petr Baudis <hidden>
Date: 2016-06-15 22:43:11

This patch introduces --extended-regexp and --regexp-ignore-case options to
tune what kind of patterns the pattern-limiting options (--grep, --author,
...) accept.

Signed-off-by: Petr Baudis <redacted>
---

 Documentation/git-rev-list.txt |   10 ++++++++++
 revision.c                     |   12 ++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index c3c2043..0dba73f 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -25,6 +25,7 @@ SYNOPSIS
 	     [ \--cherry-pick ]
 	     [ \--encoding[=<encoding>] ]
 	     [ \--(author|committer|grep)=<pattern> ]
+	     [ \--regexp-ignore-case ] [ \--extended-regexp ]
 	     [ \--date={local|relative|default} ]
 	     [ [\--objects | \--objects-edge] [ \--unpacked ] ]
 	     [ \--pretty | \--header ]
@@ -214,6 +215,15 @@ limiting may be applied.
 	Limit the commits output to ones with log message that
 	matches the specified pattern (regular expression).
 
+--regexp-ignore-case::
+
+	Match the regexp limiting patterns without regard to letters case.
+
+--extended-regexp::
+
+	Consider the limiting patterns to be extended regular expressions
+	instead of the default basic regular expressions.
+
 --remove-empty::
 
 	Stop when a given path disappears from the tree.
diff --git a/revision.c b/revision.c
index 0125d41..0a29b53 100644
--- a/revision.c
+++ b/revision.c
@@ -881,6 +881,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 	const char **unrecognized = argv + 1;
 	int left = 1;
 	int all_match = 0;
+	int regflags = 0;
 
 	/* First, search for "--" */
 	seen_dashdash = 0;
@@ -1152,6 +1153,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				add_message_grep(revs, arg+7);
 				continue;
 			}
+			if (!prefixcmp(arg, "--extended-regexp")) {
+				regflags |= REG_EXTENDED;
+				continue;
+			}
+			if (!prefixcmp(arg, "--regexp-ignore-case")) {
+				regflags |= REG_ICASE;
+				continue;
+			}
 			if (!strcmp(arg, "--all-match")) {
 				all_match = 1;
 				continue;
@@ -1200,6 +1209,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		}
 	}
 
+	if (revs->grep_filter)
+		revs->grep_filter->regflags |= regflags;
+
 	if (show_merge)
 		prepare_show_merge(revs);
 	if (def && !revs->pending.nr) {

[PATCH 2/3] gitweb: Lift any characters restriction on searched strings

From: Petr Baudis <hidden>
Date: 2016-06-15 22:43:11

Everything is already fully quoted along the way so I believe this to be
unnecessary at this point. It would pose trouble for regexp searches.

Signed-off-by: Petr Baudis <redacted>
---

 gitweb/gitweb.perl |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0143183..4ccb4e3 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -404,9 +404,6 @@ if (defined $searchtype) {
 our $searchtext = $cgi->param('s');
 our $search_regexp;
 if (defined $searchtext) {
-	if ($searchtype ne 'grep' and $searchtype ne 'pickaxe' and $searchtext =~ m/[^a-zA-Z0-9_\.\/\-\+\:\@ ]/) {
-		die_error(undef, "Invalid search parameter");
-	}
 	if (length($searchtext) < 2) {
 		die_error(undef, "At least two characters are required for search parameter");
 	}

[PATCH 3/3] gitweb: Clearly distinguish regexp / exact match searches

From: Petr Baudis <hidden>
Date: 2016-06-15 22:43:11

This patch does a couple of things:

* Makes commit/author/committer search case insensitive

	To be consistent with the grep search; I see no convincing
	reason for the search to be case sensitive, and you might
	get in trouble especially with contributors e.g. from Japan
	or France where they sometimes like to uppercase their last
	name.

* Makes grep search by default grep for fixed strings

	Since we will have a checkbox.

* Introduces 're' checkbox that enables POSIX extended regexp searches

	This works for all the search types. The idea comes from Jakub.

It does not make much sense (and is not easy at all) to untangle most of
these changes from each other, thus they all go in a single patch.

Cc: Jakub Narebski <redacted>
Signed-off-by: Petr Baudis <redacted>
---

 gitweb/gitweb.perl |   50 +++++++++++++++++++++++++++++++++-----------------
 1 files changed, 33 insertions(+), 17 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4ccb4e3..846392e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -401,13 +401,15 @@ if (defined $searchtype) {
 	}
 }
 
+our $search_use_regexp = $cgi->param('sr');
+
 our $searchtext = $cgi->param('s');
 our $search_regexp;
 if (defined $searchtext) {
 	if (length($searchtext) < 2) {
 		die_error(undef, "At least two characters are required for search parameter");
 	}
-	$search_regexp = quotemeta $searchtext;
+	$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;
 }
 
 # now read PATH_INFO and use it as alternative to parameters
@@ -535,6 +537,7 @@ sub href(%) {
 		order => "o",
 		searchtext => "s",
 		searchtype => "st",
+		search_use_regexp => "sr",
 	);
 	my %mapping = @mapping;
 
@@ -1579,7 +1582,7 @@ sub parse_commit {
 }
 
 sub parse_commits {
-	my ($commit_id, $maxcount, $skip, $arg, $filename) = @_;
+	my ($commit_id, $maxcount, $skip, $filename, @args) = @_;
 	my @cos;
 
 	$maxcount ||= 1;
@@ -1589,7 +1592,7 @@ sub parse_commits {
 
 	open my $fd, "-|", git_cmd(), "rev-list",
 		"--header",
-		($arg ? ($arg) : ()),
+		@args,
 		("--max-count=" . $maxcount),
 		("--skip=" . $skip),
 		$commit_id,
@@ -2007,6 +2010,9 @@ EOF
 		      $cgi->sup($cgi->a({-href => href(action=>"search_help")}, "?")) .
 		      " search:\n",
 		      $cgi->textfield(-name => "s", -value => $searchtext) . "\n" .
+		      "<span title=\"Extended regular expression\">" .
+		      $cgi->checkbox(-name => 'sr', -value => 1, -checked => $search_use_regexp, -label => 're') .
+		      "</span>" .
 		      "</div>" .
 		      $cgi->end_form() . "\n";
 	}
@@ -4655,7 +4661,7 @@ sub git_history {
 		$ftype = git_get_type($hash);
 	}
 
-	my @commitlist = parse_commits($hash_base, 101, (100 * $page), "--full-history", $file_name);
+	my @commitlist = parse_commits($hash_base, 101, (100 * $page), $file_name, "--full-history");
 
 	my $paging_nav = '';
 	if ($page > 0) {
@@ -4745,7 +4751,9 @@ sub git_search {
 			$greptype = "--committer=";
 		}
 		$greptype .= $search_regexp;
-		my @commitlist = parse_commits($hash, 101, (100 * $page), $greptype);
+		my @commitlist = parse_commits($hash, 101, (100 * $page), undef,
+		                               $greptype, $search_use_regexp ? ('--extended-regexp') : (),
+					       '--regexp-ignore-case');
 
 		my $paging_nav = '';
 		if ($page > 0) {
@@ -4795,8 +4803,9 @@ sub git_search {
 		my $git_command = git_cmd_str();
 		my $searchqtext = $searchtext;
 		$searchqtext =~ s/'/'\\''/;
+		my $pickaxe_flags = $search_use_regexp ? '--pickaxe-regex' : '';
 		open my $fd, "-|", "$git_command rev-list $hash | " .
-			"$git_command diff-tree -r --stdin -S\'$searchqtext\'";
+			"$git_command diff-tree -r --stdin -S\'$searchqtext\' $pickaxe_flags";
 		undef %co;
 		my @files;
 		while (my $line = <$fd>) {
@@ -4859,7 +4868,9 @@ sub git_search {
 		my $alternate = 1;
 		my $matches = 0;
 		$/ = "\n";
-		open my $fd, "-|", git_cmd(), 'grep', '-n', '-i', '-E', $searchtext, $co{'tree'};
+		open my $fd, "-|", git_cmd(), 'grep', '-n',
+			$search_use_regexp ? ('-E', '-i') : '-F',
+			$searchtext, $co{'tree'};
 		my $lastfile = '';
 		while (my $line = <$fd>) {
 			chomp $line;
@@ -4889,7 +4900,7 @@ sub git_search {
 				print "<div class=\"binary\">Binary file</div>\n";
 			} else {
 				$ltext = untabify($ltext);
-				if ($ltext =~ m/^(.*)($searchtext)(.*)$/i) {
+				if ($ltext =~ m/^(.*)($search_regexp)(.*)$/i) {
 					$ltext = esc_html($1, -nbsp=>1);
 					$ltext .= '<span class="match">';
 					$ltext .= esc_html($2, -nbsp=>1);
@@ -4924,27 +4935,31 @@ sub git_search_help {
 	git_header_html();
 	git_print_page_nav('','', $hash,$hash,$hash);
 	print <<EOT;
+<p><strong>Pattern</strong> is by default a normal string that is matched precisely (but without
+regard to case, except in the case of pickaxe). However, when you check the <em>re</em> checkbox,
+the pattern entered is recognized as the POSIX extended
+<a href="http://en.wikipedia.org/wiki/Regular_expression">regular expression</a> (also case
+insensitive).</p>
 <dl>
 <dt><b>commit</b></dt>
-<dd>The commit messages and authorship information will be scanned for the given string.</dd>
+<dd>The commit messages and authorship information will be scanned for the given pattern.</dd>
 EOT
 	my ($have_grep) = gitweb_check_feature('grep');
 	if ($have_grep) {
 		print <<EOT;
 <dt><b>grep</b></dt>
 <dd>All files in the currently selected tree (HEAD unless you are explicitly browsing
-    a different one) are searched for the given
-<a href="http://en.wikipedia.org/wiki/Regular_expression">regular expression</a>
-(POSIX extended) and the matches are listed. On large
-trees, this search can take a while and put some strain on the server, so please use it with
-some consideration.</dd>
+    a different one) are searched for the given pattern. On large trees, this search can take
+a while and put some strain on the server, so please use it with some consideration. Note that
+due to git-grep peculiarity, currently if regexp mode is turned off, the matches are
+case-sensitive.</dd>
 EOT
 	}
 	print <<EOT;
 <dt><b>author</b></dt>
-<dd>Name and e-mail of the change author and date of birth of the patch will be scanned for the given string.</dd>
+<dd>Name and e-mail of the change author and date of birth of the patch will be scanned for the given pattern.</dd>
 <dt><b>committer</b></dt>
-<dd>Name and e-mail of the committer and date of commit will be scanned for the given string.</dd>
+<dd>Name and e-mail of the committer and date of commit will be scanned for the given pattern.</dd>
 EOT
 	my ($have_pickaxe) = gitweb_check_feature('pickaxe');
 	if ($have_pickaxe) {
@@ -4952,7 +4967,8 @@ EOT
 <dt><b>pickaxe</b></dt>
 <dd>All commits that caused the string to appear or disappear from any file (changes that
 added, removed or "modified" the string) will be listed. This search can take a while and
-takes a lot of strain on the server, so please use it wisely.</dd>
+takes a lot of strain on the server, so please use it wisely. Note that since you may be
+interested even in changes just changing the case as well, this search is case sensitive.</dd>
 EOT
 	}
 	print "</dl>\n";
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help