{"thread":{"id":"9641","subject":"[PATCH 1/2] gitweb: Lift any characters restriction on searched strings","startedAt":"2007-08-25T22:18:47Z","lastAt":"2007-09-01T00:13:02Z","messageCount":6,"participants":["Petr Baudis","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"51526","messageId":"20070825221847.11739.46741.stgit@rover","threadId":"9641","inReplyTo":null,"subject":"[PATCH 1/2] gitweb: Lift any characters restriction on searched strings","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2007-08-25T22:18:47Z","receivedAt":"2007-08-25T22:18:47Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Everything is already fully quoted along the way so I believe this to be\nunnecessary at this point. It would pose trouble for regexp searches.\n\nCc: Jakub Narebski <jnareb@gmail.com>\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n gitweb/gitweb.perl |    3 ---\n 1 files changed, 0 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 677f4fb..1ac4523 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -476,9 +476,6 @@ if (defined $searchtype) {\n our $searchtext = $cgi->param('s');\n our $search_regexp;\n if (defined $searchtext) {\n-\tif ($searchtype ne 'grep' and $searchtype ne 'pickaxe' and $searchtext =~ m/[^a-zA-Z0-9_\\.\\/\\-\\+\\:\\@ ]/) {\n-\t\tdie_error(undef, \"Invalid search parameter\");\n-\t}\n \tif (length($searchtext) < 2) {\n \t\tdie_error(undef, \"At least two characters are required for search parameter\");\n \t}\n"},{"id":"51527","messageId":"20070825221852.11739.6955.stgit@rover","threadId":"9641","inReplyTo":"20070825221847.11739.46741.stgit@rover","subject":"[PATCH 2/2] gitweb: Clearly distinguish regexp / exact match searches","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2007-08-25T22:18:53Z","receivedAt":"2007-08-25T22:18:53Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"This patch does a couple of things:\n\n* Makes commit/author/committer search case insensitive\n\n\tTo be consistent with the grep search; I see no convincing\n\treason for the search to be case sensitive, and you might\n\tget in trouble especially with contributors e.g. from Japan\n\tor France where they sometimes like to uppercase their last\n\tname.\n\n* Makes grep search by default grep for fixed strings\n\n\tSince we will have a checkbox.\n\n* Introduces 're' checkbox that enables POSIX extended regexp searches\n\n\tThis works for all the search types. The idea comes from Jakub.\n\nIt does not make much sense (and is not easy at all) to untangle most of\nthese changes from each other, thus they all go in a single patch.\n\nCc: Jakub Narebski <jnareb@gmail.com>\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n gitweb/gitweb.perl |   50 +++++++++++++++++++++++++++++++++-----------------\n 1 files changed, 33 insertions(+), 17 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 1ac4523..f7b5b4c 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -473,13 +473,15 @@ if (defined $searchtype) {\n \t}\n }\n \n+our $search_use_regexp = $cgi->param('sr');\n+\n our $searchtext = $cgi->param('s');\n our $search_regexp;\n if (defined $searchtext) {\n \tif (length($searchtext) < 2) {\n \t\tdie_error(undef, \"At least two characters are required for search parameter\");\n \t}\n-\t$search_regexp = quotemeta $searchtext;\n+\t$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;\n }\n \n # now read PATH_INFO and use it as alternative to parameters\n@@ -609,6 +611,7 @@ sub href(%) {\n \t\tsearchtype => \"st\",\n \t\tsnapshot_format => \"sf\",\n \t\textra_options => \"opt\",\n+\t\tsearch_use_regexp => \"sr\",\n \t);\n \tmy %mapping = @mapping;\n \n@@ -1937,7 +1940,7 @@ sub parse_commit {\n }\n \n sub parse_commits {\n-\tmy ($commit_id, $maxcount, $skip, $arg, $filename) = @_;\n+\tmy ($commit_id, $maxcount, $skip, $filename, @args) = @_;\n \tmy @cos;\n \n \t$maxcount ||= 1;\n@@ -1947,7 +1950,7 @@ sub parse_commits {\n \n \topen my $fd, \"-|\", git_cmd(), \"rev-list\",\n \t\t\"--header\",\n-\t\t($arg ? ($arg) : ()),\n+\t\t@args,\n \t\t(\"--max-count=\" . $maxcount),\n \t\t(\"--skip=\" . $skip),\n \t\t@extra_options,\n@@ -2422,6 +2425,9 @@ EOF\n \t\t      $cgi->sup($cgi->a({-href => href(action=>\"search_help\")}, \"?\")) .\n \t\t      \" search:\\n\",\n \t\t      $cgi->textfield(-name => \"s\", -value => $searchtext) . \"\\n\" .\n+\t\t      \"<span title=\\\"Extended regular expression\\\">\" .\n+\t\t      $cgi->checkbox(-name => 'sr', -value => 1, -checked => $search_use_regexp, -label => 're') .\n+\t\t      \"</span>\" .\n \t\t      \"</div>\" .\n \t\t      $cgi->end_form() . \"\\n\";\n \t}\n@@ -5095,7 +5101,7 @@ sub git_history {\n \t\t$ftype = git_get_type($hash);\n \t}\n \n-\tmy @commitlist = parse_commits($hash_base, 101, (100 * $page), \"--full-history\", $file_name);\n+\tmy @commitlist = parse_commits($hash_base, 101, (100 * $page), $file_name, \"--full-history\");\n \n \tmy $paging_nav = '';\n \tif ($page > 0) {\n@@ -5185,7 +5191,9 @@ sub git_search {\n \t\t\t$greptype = \"--committer=\";\n \t\t}\n \t\t$greptype .= $search_regexp;\n-\t\tmy @commitlist = parse_commits($hash, 101, (100 * $page), $greptype);\n+\t\tmy @commitlist = parse_commits($hash, 101, (100 * $page), undef,\n+\t\t                               $greptype, $search_use_regexp ? ('--extended-regexp') : (),\n+\t\t\t\t\t       '--regexp-ignore-case');\n \n \t\tmy $paging_nav = '';\n \t\tif ($page > 0) {\n@@ -5235,8 +5243,9 @@ sub git_search {\n \t\tmy $git_command = git_cmd_str();\n \t\tmy $searchqtext = $searchtext;\n \t\t$searchqtext =~ s/'/'\\\\''/;\n+\t\tmy $pickaxe_flags = $search_use_regexp ? '--pickaxe-regex' : '';\n \t\topen my $fd, \"-|\", \"$git_command rev-list $hash | \" .\n-\t\t\t\"$git_command diff-tree -r --stdin -S\\'$searchqtext\\'\";\n+\t\t\t\"$git_command diff-tree -r --stdin -S\\'$searchqtext\\' $pickaxe_flags\";\n \t\tundef %co;\n \t\tmy @files;\n \t\twhile (my $line = <$fd>) {\n@@ -5299,7 +5308,9 @@ sub git_search {\n \t\tmy $alternate = 1;\n \t\tmy $matches = 0;\n \t\t$/ = \"\\n\";\n-\t\topen my $fd, \"-|\", git_cmd(), 'grep', '-n', '-i', '-E', $searchtext, $co{'tree'};\n+\t\topen my $fd, \"-|\", git_cmd(), 'grep', '-n',\n+\t\t\t$search_use_regexp ? ('-E', '-i') : '-F',\n+\t\t\t$searchtext, $co{'tree'};\n \t\tmy $lastfile = '';\n \t\twhile (my $line = <$fd>) {\n \t\t\tchomp $line;\n@@ -5329,7 +5340,7 @@ sub git_search {\n \t\t\t\tprint \"<div class=\\\"binary\\\">Binary file</div>\\n\";\n \t\t\t} else {\n \t\t\t\t$ltext = untabify($ltext);\n-\t\t\t\tif ($ltext =~ m/^(.*)($searchtext)(.*)$/i) {\n+\t\t\t\tif ($ltext =~ m/^(.*)($search_regexp)(.*)$/i) {\n \t\t\t\t\t$ltext = esc_html($1, -nbsp=>1);\n \t\t\t\t\t$ltext .= '<span class=\"match\">';\n \t\t\t\t\t$ltext .= esc_html($2, -nbsp=>1);\n@@ -5364,27 +5375,31 @@ sub git_search_help {\n \tgit_header_html();\n \tgit_print_page_nav('','', $hash,$hash,$hash);\n \tprint <<EOT;\n+<p><strong>Pattern</strong> is by default a normal string that is matched precisely (but without\n+regard to case, except in the case of pickaxe). However, when you check the <em>re</em> checkbox,\n+the pattern entered is recognized as the POSIX extended\n+<a href=\"http://en.wikipedia.org/wiki/Regular_expression\">regular expression</a> (also case\n+insensitive).</p>\n <dl>\n <dt><b>commit</b></dt>\n-<dd>The commit messages and authorship information will be scanned for the given string.</dd>\n+<dd>The commit messages and authorship information will be scanned for the given pattern.</dd>\n EOT\n \tmy ($have_grep) = gitweb_check_feature('grep');\n \tif ($have_grep) {\n \t\tprint <<EOT;\n <dt><b>grep</b></dt>\n <dd>All files in the currently selected tree (HEAD unless you are explicitly browsing\n-    a different one) are searched for the given\n-<a href=\"http://en.wikipedia.org/wiki/Regular_expression\">regular expression</a>\n-(POSIX extended) and the matches are listed. On large\n-trees, this search can take a while and put some strain on the server, so please use it with\n-some consideration.</dd>\n+    a different one) are searched for the given pattern. On large trees, this search can take\n+a while and put some strain on the server, so please use it with some consideration. Note that\n+due to git-grep peculiarity, currently if regexp mode is turned off, the matches are\n+case-sensitive.</dd>\n EOT\n \t}\n \tprint <<EOT;\n <dt><b>author</b></dt>\n-<dd>Name and e-mail of the change author and date of birth of the patch will be scanned for the given string.</dd>\n+<dd>Name and e-mail of the change author and date of birth of the patch will be scanned for the given pattern.</dd>\n <dt><b>committer</b></dt>\n-<dd>Name and e-mail of the committer and date of commit will be scanned for the given string.</dd>\n+<dd>Name and e-mail of the committer and date of commit will be scanned for the given pattern.</dd>\n EOT\n \tmy ($have_pickaxe) = gitweb_check_feature('pickaxe');\n \tif ($have_pickaxe) {\n@@ -5392,7 +5407,8 @@ EOT\n <dt><b>pickaxe</b></dt>\n <dd>All commits that caused the string to appear or disappear from any file (changes that\n added, removed or \"modified\" the string) will be listed. This search can take a while and\n-takes a lot of strain on the server, so please use it wisely.</dd>\n+takes a lot of strain on the server, so please use it wisely. Note that since you may be\n+interested even in changes just changing the case as well, this search is case sensitive.</dd>\n EOT\n \t}\n \tprint \"</dl>\\n\";\n"},{"id":"51537","messageId":"20070826013831.17924.75365.stgit@rover","threadId":"9641","inReplyTo":"20070825221852.11739.6955.stgit@rover","subject":"[PATCH] gitweb: Clearly distinguish regexp / exact match searches","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2007-08-26T01:38:31Z","receivedAt":"2007-08-26T01:38:31Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"This patch does a couple of things:\n\n* Makes commit/author/committer search case insensitive\n\n\tTo be consistent with the grep search; I see no convincing\n\treason for the search to be case sensitive, and you might\n\tget in trouble especially with contributors e.g. from Japan\n\tor France where they sometimes like to uppercase their last\n\tname.\n\n* Makes grep search by default grep for fixed strings\n\n\tSince we will have a checkbox.\n\n* Introduces 're' checkbox that enables POSIX extended regexp searches\n\n\tThis works for all the search types. The idea comes from Jakub.\n\nIt does not make much sense (and is not easy at all) to untangle most of\nthese changes from each other, thus they all go in a single patch.\n\nCc: Jakub Narebski <jnareb@gmail.com>\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\nSorry, previous version kept git_feed() the user of parse_commits() with\nunchanged usage.\n---\n\n gitweb/gitweb.perl |   52 ++++++++++++++++++++++++++++++++++------------------\n 1 files changed, 34 insertions(+), 18 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 1ac4523..bdb0b1f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -473,13 +473,15 @@ if (defined $searchtype) {\n \t}\n }\n \n+our $search_use_regexp = $cgi->param('sr');\n+\n our $searchtext = $cgi->param('s');\n our $search_regexp;\n if (defined $searchtext) {\n \tif (length($searchtext) < 2) {\n \t\tdie_error(undef, \"At least two characters are required for search parameter\");\n \t}\n-\t$search_regexp = quotemeta $searchtext;\n+\t$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;\n }\n \n # now read PATH_INFO and use it as alternative to parameters\n@@ -609,6 +611,7 @@ sub href(%) {\n \t\tsearchtype => \"st\",\n \t\tsnapshot_format => \"sf\",\n \t\textra_options => \"opt\",\n+\t\tsearch_use_regexp => \"sr\",\n \t);\n \tmy %mapping = @mapping;\n \n@@ -1937,7 +1940,7 @@ sub parse_commit {\n }\n \n sub parse_commits {\n-\tmy ($commit_id, $maxcount, $skip, $arg, $filename) = @_;\n+\tmy ($commit_id, $maxcount, $skip, $filename, @args) = @_;\n \tmy @cos;\n \n \t$maxcount ||= 1;\n@@ -1947,7 +1950,7 @@ sub parse_commits {\n \n \topen my $fd, \"-|\", git_cmd(), \"rev-list\",\n \t\t\"--header\",\n-\t\t($arg ? ($arg) : ()),\n+\t\t@args,\n \t\t(\"--max-count=\" . $maxcount),\n \t\t(\"--skip=\" . $skip),\n \t\t@extra_options,\n@@ -2422,6 +2425,9 @@ EOF\n \t\t      $cgi->sup($cgi->a({-href => href(action=>\"search_help\")}, \"?\")) .\n \t\t      \" search:\\n\",\n \t\t      $cgi->textfield(-name => \"s\", -value => $searchtext) . \"\\n\" .\n+\t\t      \"<span title=\\\"Extended regular expression\\\">\" .\n+\t\t      $cgi->checkbox(-name => 'sr', -value => 1, -checked => $search_use_regexp, -label => 're') .\n+\t\t      \"</span>\" .\n \t\t      \"</div>\" .\n \t\t      $cgi->end_form() . \"\\n\";\n \t}\n@@ -5095,7 +5101,7 @@ sub git_history {\n \t\t$ftype = git_get_type($hash);\n \t}\n \n-\tmy @commitlist = parse_commits($hash_base, 101, (100 * $page), \"--full-history\", $file_name);\n+\tmy @commitlist = parse_commits($hash_base, 101, (100 * $page), $file_name, \"--full-history\");\n \n \tmy $paging_nav = '';\n \tif ($page > 0) {\n@@ -5185,7 +5191,9 @@ sub git_search {\n \t\t\t$greptype = \"--committer=\";\n \t\t}\n \t\t$greptype .= $search_regexp;\n-\t\tmy @commitlist = parse_commits($hash, 101, (100 * $page), $greptype);\n+\t\tmy @commitlist = parse_commits($hash, 101, (100 * $page), undef,\n+\t\t                               $greptype, $search_use_regexp ? ('--extended-regexp') : (),\n+\t\t\t\t\t       '--regexp-ignore-case');\n \n \t\tmy $paging_nav = '';\n \t\tif ($page > 0) {\n@@ -5235,8 +5243,9 @@ sub git_search {\n \t\tmy $git_command = git_cmd_str();\n \t\tmy $searchqtext = $searchtext;\n \t\t$searchqtext =~ s/'/'\\\\''/;\n+\t\tmy $pickaxe_flags = $search_use_regexp ? '--pickaxe-regex' : '';\n \t\topen my $fd, \"-|\", \"$git_command rev-list $hash | \" .\n-\t\t\t\"$git_command diff-tree -r --stdin -S\\'$searchqtext\\'\";\n+\t\t\t\"$git_command diff-tree -r --stdin -S\\'$searchqtext\\' $pickaxe_flags\";\n \t\tundef %co;\n \t\tmy @files;\n \t\twhile (my $line = <$fd>) {\n@@ -5299,7 +5308,9 @@ sub git_search {\n \t\tmy $alternate = 1;\n \t\tmy $matches = 0;\n \t\t$/ = \"\\n\";\n-\t\topen my $fd, \"-|\", git_cmd(), 'grep', '-n', '-i', '-E', $searchtext, $co{'tree'};\n+\t\topen my $fd, \"-|\", git_cmd(), 'grep', '-n',\n+\t\t\t$search_use_regexp ? ('-E', '-i') : '-F',\n+\t\t\t$searchtext, $co{'tree'};\n \t\tmy $lastfile = '';\n \t\twhile (my $line = <$fd>) {\n \t\t\tchomp $line;\n@@ -5329,7 +5340,7 @@ sub git_search {\n \t\t\t\tprint \"<div class=\\\"binary\\\">Binary file</div>\\n\";\n \t\t\t} else {\n \t\t\t\t$ltext = untabify($ltext);\n-\t\t\t\tif ($ltext =~ m/^(.*)($searchtext)(.*)$/i) {\n+\t\t\t\tif ($ltext =~ m/^(.*)($search_regexp)(.*)$/i) {\n \t\t\t\t\t$ltext = esc_html($1, -nbsp=>1);\n \t\t\t\t\t$ltext .= '<span class=\"match\">';\n \t\t\t\t\t$ltext .= esc_html($2, -nbsp=>1);\n@@ -5364,27 +5375,31 @@ sub git_search_help {\n \tgit_header_html();\n \tgit_print_page_nav('','', $hash,$hash,$hash);\n \tprint <<EOT;\n+<p><strong>Pattern</strong> is by default a normal string that is matched precisely (but without\n+regard to case, except in the case of pickaxe). However, when you check the <em>re</em> checkbox,\n+the pattern entered is recognized as the POSIX extended\n+<a href=\"http://en.wikipedia.org/wiki/Regular_expression\">regular expression</a> (also case\n+insensitive).</p>\n <dl>\n <dt><b>commit</b></dt>\n-<dd>The commit messages and authorship information will be scanned for the given string.</dd>\n+<dd>The commit messages and authorship information will be scanned for the given pattern.</dd>\n EOT\n \tmy ($have_grep) = gitweb_check_feature('grep');\n \tif ($have_grep) {\n \t\tprint <<EOT;\n <dt><b>grep</b></dt>\n <dd>All files in the currently selected tree (HEAD unless you are explicitly browsing\n-    a different one) are searched for the given\n-<a href=\"http://en.wikipedia.org/wiki/Regular_expression\">regular expression</a>\n-(POSIX extended) and the matches are listed. On large\n-trees, this search can take a while and put some strain on the server, so please use it with\n-some consideration.</dd>\n+    a different one) are searched for the given pattern. On large trees, this search can take\n+a while and put some strain on the server, so please use it with some consideration. Note that\n+due to git-grep peculiarity, currently if regexp mode is turned off, the matches are\n+case-sensitive.</dd>\n EOT\n \t}\n \tprint <<EOT;\n <dt><b>author</b></dt>\n-<dd>Name and e-mail of the change author and date of birth of the patch will be scanned for the given string.</dd>\n+<dd>Name and e-mail of the change author and date of birth of the patch will be scanned for the given pattern.</dd>\n <dt><b>committer</b></dt>\n-<dd>Name and e-mail of the committer and date of commit will be scanned for the given string.</dd>\n+<dd>Name and e-mail of the committer and date of commit will be scanned for the given pattern.</dd>\n EOT\n \tmy ($have_pickaxe) = gitweb_check_feature('pickaxe');\n \tif ($have_pickaxe) {\n@@ -5392,7 +5407,8 @@ EOT\n <dt><b>pickaxe</b></dt>\n <dd>All commits that caused the string to appear or disappear from any file (changes that\n added, removed or \"modified\" the string) will be listed. This search can take a while and\n-takes a lot of strain on the server, so please use it wisely.</dd>\n+takes a lot of strain on the server, so please use it wisely. Note that since you may be\n+interested even in changes just changing the case as well, this search is case sensitive.</dd>\n EOT\n \t}\n \tprint \"</dl>\\n\";\n@@ -5443,7 +5459,7 @@ sub git_feed {\n \n \t# log/feed of current (HEAD) branch, log of given branch, history of file/directory\n \tmy $head = $hash || 'HEAD';\n-\tmy @commitlist = parse_commits($head, 150, 0, undef, $file_name);\n+\tmy @commitlist = parse_commits($head, 150, 0, $file_name);\n \n \tmy %latest_commit;\n \tmy %latest_date;\n"},{"id":"51624","messageId":"fat7s1$t4a$1@sea.gmane.org","threadId":"9641","inReplyTo":"20070825221847.11739.46741.stgit@rover","subject":"Re: [PATCH 1/2] gitweb: Lift any characters restriction on searched strings","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-08-27T01:02:59Z","receivedAt":"2007-08-27T01:02:59Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Petr Baudis wrote:\n\n> Everything is already fully quoted along the way so I believe this to be\n> unnecessary at this point. It would pose trouble for regexp searches.\n\nAck. It looks correct.\n \n> Cc: Jakub Narebski <jnareb@gmail.com>\n> Signed-off-by: Petr Baudis <pasky@suse.cz>\n\nActually I have not received it by the mail, so \"Cc:\" in the body didn't\nwork. Did you use git-send-mail?\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"51631","messageId":"20070827033505.GP1219@pasky.or.cz","threadId":"9641","inReplyTo":"fat7s1$t4a$1@sea.gmane.org","subject":"Re: [PATCH 1/2] gitweb: Lift any characters restriction on searched strings","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2007-08-27T03:35:05Z","receivedAt":"2007-08-27T03:35:05Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Mon, Aug 27, 2007 at 03:02:59AM CEST, Jakub Narebski wrote:\n> Petr Baudis wrote:\n> \n> > Everything is already fully quoted along the way so I believe this to be\n> > unnecessary at this point. It would pose trouble for regexp searches.\n> \n> Ack. It looks correct.\n>  \n> > Cc: Jakub Narebski <jnareb@gmail.com>\n> > Signed-off-by: Petr Baudis <pasky@suse.cz>\n> \n> Actually I have not received it by the mail, so \"Cc:\" in the body didn't\n> work. Did you use git-send-mail?\n\nNo, I use StGIT. The last time I've sent this patch it had the Cc' so I\nthought \"cool, stg mail can do this automagically\", but apparently I\nprobably added that to its commandline manually...\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nEarly to rise and early to bed makes a male healthy and wealthy and dead.\n                -- James Thurber\n"},{"id":"52105","messageId":"fbaaqb$ked$1@sea.gmane.org","threadId":"9641","inReplyTo":"20070826013831.17924.75365.stgit@rover","subject":"Re: [PATCH] gitweb: Clearly distinguish regexp / exact match searches","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-09-01T00:13:02Z","receivedAt":"2007-09-01T00:13:02Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"[Cc: Petr Baudis <pasky@suse.cz>, git@vger.kernel.org]\n\nNice idea, although most probably post 1.5.3.\nAck, FWIW.\n\nPetr Baudis wrote:\n\n> This patch does a couple of things:\n> \n> * Makes commit/author/committer search case insensitive\n> \n>       To be consistent with the grep search; I see no convincing\n>       reason for the search to be case sensitive, and you might\n>       get in trouble especially with contributors e.g. from Japan\n>       or France where they sometimes like to uppercase their last\n>       name.\n> \n> * Makes grep search by default grep for fixed strings\n> \n>       Since we will have a checkbox.\n> \n> * Introduces 're' checkbox that enables POSIX extended regexp searches\n> \n>       This works for all the search types. The idea comes from Jakub.\n> \n> It does not make much sense (and is not easy at all) to untangle most of\n> these changes from each other, thus they all go in a single patch.\n\nThe last two changes certainly _have_ to go together, as they have\nno much sense individually. I'm not so sure about making commit search\ncase insensitive; I agree that it would be not easy to untangle those\nchanges from the regexp changes.\n\nNevertheless I'd separate one patch: changing usage of parse_commits\n(see comments below).\n\n> Cc: Jakub Narebski <jnareb@gmail.com>\n\nI didn't get a copy by email...\n\n> Signed-off-by: Petr Baudis <pasky@suse.cz>\n> ---\n> \n> Sorry, previous version kept git_feed() the user of parse_commits() with\n> unchanged usage.\n> ---\n> \n>  gitweb/gitweb.perl |   52 ++++++++++++++++++++++++++++++++++------------------\n>  1 files changed, 34 insertions(+), 18 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 1ac4523..bdb0b1f 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -473,13 +473,15 @@ if (defined $searchtype) {\n>       }\n>  }\n>  \n> +our $search_use_regexp = $cgi->param('sr');\n> +\n>  our $searchtext = $cgi->param('s');\n>  our $search_regexp;\n>  if (defined $searchtext) {\n>       if (length($searchtext) < 2) {\n>               die_error(undef, \"At least two characters are required for search parameter\");\n>       }\n> -     $search_regexp = quotemeta $searchtext;\n> +     $search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;\n>  }\n\nNice idea, good solution... although: yet another CGI parameter?\n\n> @@ -609,6 +611,7 @@ sub href(%) {\n>               searchtype => \"st\",\n>               snapshot_format => \"sf\",\n>               extra_options => \"opt\",\n> +             search_use_regexp => \"sr\",\n>       );\n>       my %mapping = @mapping;\n>  \n\nI would put 'search_use_regexp' after searchtype, not at the end of\nthe @mapping list.\n\n> @@ -1937,7 +1940,7 @@ sub parse_commit {\n>  }\n>  \n>  sub parse_commits {\n> -     my ($commit_id, $maxcount, $skip, $arg, $filename) = @_;\n> +     my ($commit_id, $maxcount, $skip, $filename, @args) = @_;\n>       my @cos;\n>  \n>       $maxcount ||= 1;\n> @@ -1947,7 +1950,7 @@ sub parse_commits {\n>  \n>       open my $fd, \"-|\", git_cmd(), \"rev-list\",\n>               \"--header\",\n> -             ($arg ? ($arg) : ()),\n> +             @args,\n>               (\"--max-count=\" . $maxcount),\n>               (\"--skip=\" . $skip),\n>               @extra_options,\n\nThis is a good idea. It not only allows passing more than single\nparameter to rev-list, but also make it easier to use \"no extra\noptions\" parse_commits invocation. Although it makes order of\ntypes of options different than in git-rev-list invocation.\n\nBut I'd rather have this, and changes in calling parse_commits,\nas a separate commit.\n\nWe could always use anonymous array reference if we want to pass\nmore than one option:\n\n  parse_commits(commit, maxcount, skip, [arg1, arg2], filename)\n\n> @@ -2422,6 +2425,9 @@ EOF\n>                     $cgi->sup($cgi->a({-href => href(action=>\"search_help\")}, \"?\")) .\n>                     \" search:\\n\",\n>                     $cgi->textfield(-name => \"s\", -value => $searchtext) . \"\\n\" .\n> +                   \"<span title=\\\"Extended regular expression\\\">\" .\n> +                   $cgi->checkbox(-name => 'sr', -value => 1, -checked => $search_use_regexp, -label => 're') .\n> +                   \"</span>\" .\n>                     \"</div>\" .\n>                     $cgi->end_form() . \"\\n\";\n>       }\n\nPerhaps this checkbox should be _below_ search form text field,\nrather than after it.\n\n> @@ -5095,7 +5101,7 @@ sub git_history {\n>               $ftype = git_get_type($hash);\n>       }\n>  \n> -     my @commitlist = parse_commits($hash_base, 101, (100 * $page), \"--full-history\", $file_name);\n> +     my @commitlist = parse_commits($hash_base, 101, (100 * $page), $file_name, \"--full-history\");\n>  \n>       my $paging_nav = '';\n>       if ($page > 0) {\n> @@ -5185,7 +5191,9 @@ sub git_search {\n>                       $greptype = \"--committer=\";\n>               }\n>               $greptype .= $search_regexp;\n> -             my @commitlist = parse_commits($hash, 101, (100 * $page), $greptype);\n> +             my @commitlist = parse_commits($hash, 101, (100 * $page), undef,\n> +                                            $greptype, $search_use_regexp ? ('--extended-regexp') : (),\n> +                                            '--regexp-ignore-case');\n>  \n>               my $paging_nav = '';\n>               if ($page > 0) {\n\nThe change makes it harder to write getting log of commits with\nsome extra option but without the path limiter.\n\n> @@ -5235,8 +5243,9 @@ sub git_search {\n>               my $git_command = git_cmd_str();\n>               my $searchqtext = $searchtext;\n>               $searchqtext =~ s/'/'\\\\''/;\n> +             my $pickaxe_flags = $search_use_regexp ? '--pickaxe-regex' : '';\n>               open my $fd, \"-|\", \"$git_command rev-list $hash | \" .\n> -                     \"$git_command diff-tree -r --stdin -S\\'$searchqtext\\'\";\n> +                     \"$git_command diff-tree -r --stdin -S\\'$searchqtext\\' $pickaxe_flags\";\n>               undef %co;\n>               my @files;\n>               while (my $line = <$fd>) {\n\nNice.\n\n[...]\n> @@ -5364,27 +5375,31 @@ sub git_search_help {\n>       git_header_html();\n>       git_print_page_nav('','', $hash,$hash,$hash);\n>       print <<EOT;\n> +<p><strong>Pattern</strong> is by default a normal string that is matched precisely (but without\n> +regard to case, except in the case of pickaxe). However, when you check the <em>re</em> checkbox,\n> +the pattern entered is recognized as the POSIX extended\n> +<a href=\"http://en.wikipedia.org/wiki/Regular_expression\">regular expression</a> (also case\n> +insensitive).</p>\n>  <dl>\n>  <dt><b>commit</b></dt>\n> -<dd>The commit messages and authorship information will be scanned for the given string.</dd>\n> +<dd>The commit messages and authorship information will be scanned for the given pattern.</dd>\n>  EOT\n>       my ($have_grep) = gitweb_check_feature('grep');\n>       if ($have_grep) {\n>               print <<EOT;\n>  <dt><b>grep</b></dt>\n>  <dd>All files in the currently selected tree (HEAD unless you are explicitly browsing\n> -    a different one) are searched for the given\n> -<a href=\"http://en.wikipedia.org/wiki/Regular_expression\">regular expression</a>\n> -(POSIX extended) and the matches are listed. On large\n> -trees, this search can take a while and put some strain on the server, so please use it with\n> -some consideration.</dd>\n> +    a different one) are searched for the given pattern. On large trees, this search can take\n> +a while and put some strain on the server, so please use it with some consideration. Note that\n> +due to git-grep peculiarity, currently if regexp mode is turned off, the matches are\n> +case-sensitive.</dd>\n>  EOT\n[...]\n\n> @@ -5392,7 +5407,8 @@ EOT\n>  <dt><b>pickaxe</b></dt>\n>  <dd>All commits that caused the string to appear or disappear from any file (changes that\n>  added, removed or \"modified\" the string) will be listed. This search can take a while and\n> -takes a lot of strain on the server, so please use it wisely.</dd>\n> +takes a lot of strain on the server, so please use it wisely. Note that since you may be\n> +interested even in changes just changing the case as well, this search is case sensitive.</dd>\n>  EOT\n>       }\n>       print \"</dl>\\n\";\n\n\nI'd rather have case-insensitive search _only_ for commit search\n(commit message, author, committer), and case-sensitive for the\nrest: pickaxe and grep search.\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"}]}