git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 1/2] gitweb: Lift any characters restriction on searched strings
@ 2007-08-25 22:18 Petr Baudis
  2007-08-25 22:18 ` [PATCH 2/2] gitweb: Clearly distinguish regexp / exact match searches Petr Baudis
  2007-08-27  1:02 ` [PATCH 1/2] gitweb: Lift any characters restriction on searched strings Jakub Narebski
  0 siblings, 2 replies; 6+ messages in thread
From: Petr Baudis @ 2007-08-25 22:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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.

Cc: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 gitweb/gitweb.perl |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 677f4fb..1ac4523 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -476,9 +476,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");
 	}

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] gitweb: Clearly distinguish regexp / exact match searches
  2007-08-25 22:18 [PATCH 1/2] gitweb: Lift any characters restriction on searched strings Petr Baudis
@ 2007-08-25 22:18 ` Petr Baudis
  2007-08-26  1:38   ` [PATCH] " Petr Baudis
  2007-08-27  1:02 ` [PATCH 1/2] gitweb: Lift any characters restriction on searched strings Jakub Narebski
  1 sibling, 1 reply; 6+ messages in thread
From: Petr Baudis @ 2007-08-25 22:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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 <jnareb@gmail.com>
Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 gitweb/gitweb.perl |   50 +++++++++++++++++++++++++++++++++-----------------
 1 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1ac4523..f7b5b4c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -473,13 +473,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
@@ -609,6 +611,7 @@ sub href(%) {
 		searchtype => "st",
 		snapshot_format => "sf",
 		extra_options => "opt",
+		search_use_regexp => "sr",
 	);
 	my %mapping = @mapping;
 
@@ -1937,7 +1940,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;
@@ -1947,7 +1950,7 @@ sub parse_commits {
 
 	open my $fd, "-|", git_cmd(), "rev-list",
 		"--header",
-		($arg ? ($arg) : ()),
+		@args,
 		("--max-count=" . $maxcount),
 		("--skip=" . $skip),
 		@extra_options,
@@ -2422,6 +2425,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";
 	}
@@ -5095,7 +5101,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) {
@@ -5185,7 +5191,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) {
@@ -5235,8 +5243,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>) {
@@ -5299,7 +5308,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;
@@ -5329,7 +5340,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);
@@ -5364,27 +5375,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) {
@@ -5392,7 +5407,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";

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] gitweb: Clearly distinguish regexp / exact match searches
  2007-08-25 22:18 ` [PATCH 2/2] gitweb: Clearly distinguish regexp / exact match searches Petr Baudis
@ 2007-08-26  1:38   ` Petr Baudis
  2007-09-01  0:13     ` Jakub Narebski
  0 siblings, 1 reply; 6+ messages in thread
From: Petr Baudis @ 2007-08-26  1:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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 <jnareb@gmail.com>
Signed-off-by: Petr Baudis <pasky@suse.cz>
---

Sorry, previous version kept git_feed() the user of parse_commits() with
unchanged usage.
---

 gitweb/gitweb.perl |   52 ++++++++++++++++++++++++++++++++++------------------
 1 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1ac4523..bdb0b1f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -473,13 +473,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
@@ -609,6 +611,7 @@ sub href(%) {
 		searchtype => "st",
 		snapshot_format => "sf",
 		extra_options => "opt",
+		search_use_regexp => "sr",
 	);
 	my %mapping = @mapping;
 
@@ -1937,7 +1940,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;
@@ -1947,7 +1950,7 @@ sub parse_commits {
 
 	open my $fd, "-|", git_cmd(), "rev-list",
 		"--header",
-		($arg ? ($arg) : ()),
+		@args,
 		("--max-count=" . $maxcount),
 		("--skip=" . $skip),
 		@extra_options,
@@ -2422,6 +2425,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";
 	}
@@ -5095,7 +5101,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) {
@@ -5185,7 +5191,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) {
@@ -5235,8 +5243,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>) {
@@ -5299,7 +5308,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;
@@ -5329,7 +5340,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);
@@ -5364,27 +5375,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) {
@@ -5392,7 +5407,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";
@@ -5443,7 +5459,7 @@ sub git_feed {
 
 	# log/feed of current (HEAD) branch, log of given branch, history of file/directory
 	my $head = $hash || 'HEAD';
-	my @commitlist = parse_commits($head, 150, 0, undef, $file_name);
+	my @commitlist = parse_commits($head, 150, 0, $file_name);
 
 	my %latest_commit;
 	my %latest_date;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] gitweb: Lift any characters restriction on searched strings
  2007-08-25 22:18 [PATCH 1/2] gitweb: Lift any characters restriction on searched strings Petr Baudis
  2007-08-25 22:18 ` [PATCH 2/2] gitweb: Clearly distinguish regexp / exact match searches Petr Baudis
@ 2007-08-27  1:02 ` Jakub Narebski
  2007-08-27  3:35   ` Petr Baudis
  1 sibling, 1 reply; 6+ messages in thread
From: Jakub Narebski @ 2007-08-27  1:02 UTC (permalink / raw)
  To: git

Petr Baudis wrote:

> 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.

Ack. It looks correct.
 
> Cc: Jakub Narebski <jnareb@gmail.com>
> Signed-off-by: Petr Baudis <pasky@suse.cz>

Actually I have not received it by the mail, so "Cc:" in the body didn't
work. Did you use git-send-mail?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] gitweb: Lift any characters restriction on searched strings
  2007-08-27  1:02 ` [PATCH 1/2] gitweb: Lift any characters restriction on searched strings Jakub Narebski
@ 2007-08-27  3:35   ` Petr Baudis
  0 siblings, 0 replies; 6+ messages in thread
From: Petr Baudis @ 2007-08-27  3:35 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

On Mon, Aug 27, 2007 at 03:02:59AM CEST, Jakub Narebski wrote:
> Petr Baudis wrote:
> 
> > 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.
> 
> Ack. It looks correct.
>  
> > Cc: Jakub Narebski <jnareb@gmail.com>
> > Signed-off-by: Petr Baudis <pasky@suse.cz>
> 
> Actually I have not received it by the mail, so "Cc:" in the body didn't
> work. Did you use git-send-mail?

No, I use StGIT. The last time I've sent this patch it had the Cc' so I
thought "cool, stg mail can do this automagically", but apparently I
probably added that to its commandline manually...

-- 
				Petr "Pasky" Baudis
Early to rise and early to bed makes a male healthy and wealthy and dead.
                -- James Thurber

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gitweb: Clearly distinguish regexp / exact match searches
  2007-08-26  1:38   ` [PATCH] " Petr Baudis
@ 2007-09-01  0:13     ` Jakub Narebski
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Narebski @ 2007-09-01  0:13 UTC (permalink / raw)
  To: git

[Cc: Petr Baudis <pasky@suse.cz>, git@vger.kernel.org]

Nice idea, although most probably post 1.5.3.
Ack, FWIW.

Petr Baudis wrote:

> 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.

The last two changes certainly _have_ to go together, as they have
no much sense individually. I'm not so sure about making commit search
case insensitive; I agree that it would be not easy to untangle those
changes from the regexp changes.

Nevertheless I'd separate one patch: changing usage of parse_commits
(see comments below).

> Cc: Jakub Narebski <jnareb@gmail.com>

I didn't get a copy by email...

> Signed-off-by: Petr Baudis <pasky@suse.cz>
> ---
> 
> Sorry, previous version kept git_feed() the user of parse_commits() with
> unchanged usage.
> ---
> 
>  gitweb/gitweb.perl |   52 ++++++++++++++++++++++++++++++++++------------------
>  1 files changed, 34 insertions(+), 18 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 1ac4523..bdb0b1f 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -473,13 +473,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;
>  }

Nice idea, good solution... although: yet another CGI parameter?

> @@ -609,6 +611,7 @@ sub href(%) {
>               searchtype => "st",
>               snapshot_format => "sf",
>               extra_options => "opt",
> +             search_use_regexp => "sr",
>       );
>       my %mapping = @mapping;
>  

I would put 'search_use_regexp' after searchtype, not at the end of
the @mapping list.

> @@ -1937,7 +1940,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;
> @@ -1947,7 +1950,7 @@ sub parse_commits {
>  
>       open my $fd, "-|", git_cmd(), "rev-list",
>               "--header",
> -             ($arg ? ($arg) : ()),
> +             @args,
>               ("--max-count=" . $maxcount),
>               ("--skip=" . $skip),
>               @extra_options,

This is a good idea. It not only allows passing more than single
parameter to rev-list, but also make it easier to use "no extra
options" parse_commits invocation. Although it makes order of
types of options different than in git-rev-list invocation.

But I'd rather have this, and changes in calling parse_commits,
as a separate commit.

We could always use anonymous array reference if we want to pass
more than one option:

  parse_commits(commit, maxcount, skip, [arg1, arg2], filename)

> @@ -2422,6 +2425,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";
>       }

Perhaps this checkbox should be _below_ search form text field,
rather than after it.

> @@ -5095,7 +5101,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) {
> @@ -5185,7 +5191,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) {

The change makes it harder to write getting log of commits with
some extra option but without the path limiter.

> @@ -5235,8 +5243,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>) {

Nice.

[...]
> @@ -5364,27 +5375,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
[...]

> @@ -5392,7 +5407,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";


I'd rather have case-insensitive search _only_ for commit search
(commit message, author, committer), and case-sensitive for the
rest: pickaxe and grep search.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-09-01  0:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-08-25 22:18 [PATCH 1/2] gitweb: Lift any characters restriction on searched strings Petr Baudis
2007-08-25 22:18 ` [PATCH 2/2] gitweb: Clearly distinguish regexp / exact match searches Petr Baudis
2007-08-26  1:38   ` [PATCH] " Petr Baudis
2007-09-01  0:13     ` Jakub Narebski
2007-08-27  1:02 ` [PATCH 1/2] gitweb: Lift any characters restriction on searched strings Jakub Narebski
2007-08-27  3:35   ` Petr Baudis

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).