* [PATCH 0/4] Improve gitweb search, and other things
@ 2008-02-26 12:22 Jakub Narebski
2008-02-26 12:22 ` [PATCH 1/4] Add '--fixed-strings' option to "git log --grep" and friends Jakub Narebski
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Jakub Narebski @ 2008-02-26 12:22 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
Table of contents:
~~~~~~~~~~~~~~~~~~
[PATCH 1/4] Add '--fixed-strings' option to "git log --grep" and friends
[PATCH 2/4] gitweb: Change parse_commits signature to allow for
multiple options
[PATCH 3/4] gitweb: Simplify fixed string search
[PATCH 4/4] gitweb: Clearly distinguish regexp / exact match searches
Description of series:
~~~~~~~~~~~~~~~~~~~~~~~
When testing earlier improvements to gitweb commit search (searching
commit messages), I have noticed that searching for "don't" didn't
find anything from gitweb, while 'git log --grep="don't"' returns
quite a number of commits. After examination I have realized that it
was caused by the fact that 'quotemeta' in Perl quotes also "'"
(single quote character), and grep (which "git log --grep=<pattern>"
uses) doesn't do the unquoting for unnecessary quoted characters.
At first I have thought about implementing our own limited quoting
subroutine, quoteregexmeta() to quote only regular expression
meta-characters. The "grep" search (using git-grep) by default uses
extended POSIX regexps, so there would be need also for
quoteextremeta() subroutine, unless gitweb would pass '-E' option to
"git log" when searching/limiting output. This unfortunately needs
_three_ versions of search query: original search text, to be passed
for "next page" links, fill default/current value of search form text
(entry) field, and perhaps show in the page title; regexp quoted for
grep, to be passed to "git log --grep" and friends or "git grep";
regexp quoted for Perl (we can use quotemeta() here) for showing match
info (matched fragment). It is a bit complicated and error prone.
(Well, maybe all that is not really necessary...)
With git-grep search (tree search) and pickaxe search (diff search) it
is easy, as pickaxe search is by default fixed strings search, and
git-grep has -F / --fixed-strings option. Option which
"git log --grep=<pattern>" was lacking...
First commit in series adds then -F / --fixed-strings option for
searching commit messages by git-log / git-rev-list. It was quite
easy to do, thanks to well written infrastructure. This patch can
be seen as standalone patch (adding option for consistency), and I
have even send it as
Message-Id: <200802241647.08871.jnareb@gmail.com>
But I have thought the chance of it being accepted would be better
if there were some use case for it.
Therefore next commits in series makes use of just introduced ability
to use --fixed-strings option to consider the limiting patterns to be
fixed strings.
For this we have to pass both --grep=$seachstring (or equivalent for
other commit message search searchtypes) and --fixed-strings option.
Therefore parse_commits() subroutine calling convention, which allowed
to pass optionally only single extra option had to be changed.
Meanwhile when looking at gitweb installed at http://repo.or.cz I seen
that it includes checkbox to switch between fixed strings search, and
regexp search. (IIRC patch was send to git mailing list, and lost
somehow, not being resent enough for inclusion[*1*]). The repo.or.cz
gitweb is based on 'next' branch of http://repo.or.cz/git/gitweb.git
fork of git. This change also required more than one pattern limiting
option to be passed down to git-rev-list.
So I have then extracted this change, and put it as second commit in
this series. This made it easy to implement fixed strings search in
gitweb not by quoting regexp meta-characters, but by using just
implemented --fixed-strings option.
Finally I have though "why not", and cherry-picked Petr "Pasky" Baudis
addition of fixed-strings/regexp search checkbox. This made last,
fourth commit in this series.
[*1*] Petr "Pasky" Baudis is much less active on git mailing list lately.
Shortlog:
~~~~~~~~~
Jakub Narebski (3):
Add '--fixed-strings' option to "git log --grep" and friends
gitweb: Change parse_commits signature to allow for multiple options
gitweb: Simplify fixed string search
Petr Baudis (1):
gitweb: Clearly distinguish regexp / exact match searches
Diffstat:
~~~~~~~~~
Documentation/git-rev-list.txt | 1 +
Documentation/rev-list-options.txt | 5 +++
gitweb/gitweb.perl | 58 +++++++++++++++++++++++------------
revision.c | 10 +++++-
4 files changed, 53 insertions(+), 21 deletions(-)
--
1.5.4.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/4] Add '--fixed-strings' option to "git log --grep" and friends
2008-02-26 12:22 [PATCH 0/4] Improve gitweb search, and other things Jakub Narebski
@ 2008-02-26 12:22 ` Jakub Narebski
2008-02-27 1:03 ` Junio C Hamano
2008-02-26 12:22 ` [PATCH 2/4] gitweb: Change parse_commits signature to allow for multiple options Jakub Narebski
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Jakub Narebski @ 2008-02-26 12:22 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
Add support for -F | --fixed-strings option to "git log --grep"
and friends: "git log --author", "git log --committer=<pattern>".
Code is based on implementation of this option in "git grep".
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This would simplify ignore-case searching for a fixed string from
within gitweb, as gitweb wouldn't then have to deal with differences
in quoting and unquoting (if you quote character which doesn't need
quoting, would git (grep) unquote it?) between searched phrase,
basic/extended regular expression as understood by git/by grep,
and regular expressions in Perl (when showing matched info).
[I am not sure if the above paragraph should be added to commit
message, so it is in patch comments. Feel free to add it.]
This patch can be also seen as "consistency" patch: git-grep
implements --fixed-strings, but "git log --grep" didn't.
P.S. I see that neither git-log, nor git-grep are converted to
parseopt interface. What are plans for it, if any?
Documentation/git-rev-list.txt | 1 +
Documentation/rev-list-options.txt | 5 +++++
revision.c | 10 +++++++++-
3 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 5b96eab..a8d489f 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -31,6 +31,7 @@ SYNOPSIS
[ \--(author|committer|grep)=<pattern> ]
[ \--regexp-ignore-case | \-i ]
[ \--extended-regexp | \-E ]
+ [ \--fixed-strings | \-F ]
[ \--date={local|relative|default|iso|rfc|short} ]
[ [\--objects | \--objects-edge] [ \--unpacked ] ]
[ \--pretty | \--header ]
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index a8138e2..259072c 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -153,6 +153,11 @@ limiting may be applied.
Consider the limiting patterns to be extended regular expressions
instead of the default basic regular expressions.
+-F, --fixed-strings::
+
+ Consider the limiting patterns to be fixed strings (don't interpret
+ pattern as a regular expression).
+
--remove-empty::
Stop when a given path disappears from the tree.
diff --git a/revision.c b/revision.c
index d3e8658..5df7961 100644
--- a/revision.c
+++ b/revision.c
@@ -942,6 +942,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
int left = 1;
int all_match = 0;
int regflags = 0;
+ int fixed = 0;
/* First, search for "--" */
seen_dashdash = 0;
@@ -1238,6 +1239,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
regflags |= REG_ICASE;
continue;
}
+ if (!strcmp(arg, "--fixed-strings") ||
+ !strcmp(arg, "-F")) {
+ fixed = 1;
+ continue;
+ }
if (!strcmp(arg, "--all-match")) {
all_match = 1;
continue;
@@ -1293,8 +1299,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
}
}
- if (revs->grep_filter)
+ if (revs->grep_filter) {
revs->grep_filter->regflags |= regflags;
+ revs->grep_filter->fixed = fixed;
+ }
if (show_merge)
prepare_show_merge(revs);
--
1.5.4.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/4] gitweb: Change parse_commits signature to allow for multiple options
2008-02-26 12:22 [PATCH 0/4] Improve gitweb search, and other things Jakub Narebski
2008-02-26 12:22 ` [PATCH 1/4] Add '--fixed-strings' option to "git log --grep" and friends Jakub Narebski
@ 2008-02-26 12:22 ` Jakub Narebski
2008-02-26 12:22 ` [PATCH 3/4] gitweb: Simplify fixed string search Jakub Narebski
2008-02-26 12:22 ` [PATCH 4/4] gitweb: Clearly distinguish regexp / exact match searches Jakub Narebski
3 siblings, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2008-02-26 12:22 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis
Change order of parameters in parse_commits() to have $filename
before @args (extra options), to allow for multiple extra options,
for example both '--grep=<pattern>' and '--fixed-strings'.
Change all callers to follow new calling convention.
This is part of commit b98f0a7c in http://repo.or.cz/git/gitweb.git
gitweb: Clearly distinguish regexp / exact match searches
by Petr "Pasky" Baudis.
Signed-off-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
The paragraph starting with "This is part of commit..." can be now,
I think, safely removed, as the rest of mentioned commit is now fourth
(last) part of this patch series.
BTW. I'm not quite sure about authorship and DCO (signoff) for this
patch. It was a part of commit by pasky, but this commit was not
written by him. I have written it based on parts of mentioned commit,
following variable naming and such; therefore I assumed authorship for
this commit.
Original commit was signed off by pasky, and IMHO this signoff applies
also to the part of it, therefore I have used also pasky signoff.
gitweb/gitweb.perl | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index fc95e2c..3b4b15a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2079,7 +2079,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;
@@ -2089,7 +2089,7 @@ sub parse_commits {
open my $fd, "-|", git_cmd(), "rev-list",
"--header",
- ($arg ? ($arg) : ()),
+ @args,
("--max-count=" . $maxcount),
("--skip=" . $skip),
@extra_options,
@@ -5172,7 +5172,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) {
@@ -5255,7 +5255,7 @@ 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);
my $paging_nav = '';
if ($page > 0) {
@@ -5507,7 +5507,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;
--
1.5.4.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/4] gitweb: Simplify fixed string search
2008-02-26 12:22 [PATCH 0/4] Improve gitweb search, and other things Jakub Narebski
2008-02-26 12:22 ` [PATCH 1/4] Add '--fixed-strings' option to "git log --grep" and friends Jakub Narebski
2008-02-26 12:22 ` [PATCH 2/4] gitweb: Change parse_commits signature to allow for multiple options Jakub Narebski
@ 2008-02-26 12:22 ` Jakub Narebski
2008-02-26 12:22 ` [PATCH 4/4] gitweb: Clearly distinguish regexp / exact match searches Jakub Narebski
3 siblings, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2008-02-26 12:22 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
Use '--fixed-strings' option to git-rev-list to simplify and improve
searching commit messages (commit search). It allows to search for
example for "don't" successfully.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
The second part of this chunk follows "correct style in neighborhood
of changes" idea.
gitweb/gitweb.perl | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3b4b15a..90cf78e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5254,14 +5254,16 @@ sub git_search {
} elsif ($searchtype eq 'committer') {
$greptype = "--committer=";
}
- $greptype .= $search_regexp;
- my @commitlist = parse_commits($hash, 101, (100 * $page), undef, $greptype);
+ $greptype .= $searchtext;
+ my @commitlist = parse_commits($hash, 101, (100 * $page), undef,
+ $greptype, '--fixed-strings');
my $paging_nav = '';
if ($page > 0) {
$paging_nav .=
$cgi->a({-href => href(action=>"search", hash=>$hash,
- searchtext=>$searchtext, searchtype=>$searchtype)},
+ searchtext=>$searchtext,
+ searchtype=>$searchtype)},
"first");
$paging_nav .= " ⋅ " .
$cgi->a({-href => href(-replay=>1, page=>$page-1),
--
1.5.4.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4] gitweb: Clearly distinguish regexp / exact match searches
2008-02-26 12:22 [PATCH 0/4] Improve gitweb search, and other things Jakub Narebski
` (2 preceding siblings ...)
2008-02-26 12:22 ` [PATCH 3/4] gitweb: Simplify fixed string search Jakub Narebski
@ 2008-02-26 12:22 ` Jakub Narebski
3 siblings, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2008-02-26 12:22 UTC (permalink / raw)
To: git; +Cc: Petr Baudis, Petr Baudis, Jakub Narebski
From: Petr Baudis <pasky@ucw.cz>
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 search 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.
[jn: Cherry-picked from Pasky's http://repo.or.cz/git/gitweb.git]
Signed-off-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Cherry-picked, and resolved conflict.
gitweb/gitweb.perl | 44 ++++++++++++++++++++++++++++++--------------
1 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 90cf78e..20dc5d5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -472,13 +472,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
@@ -608,6 +610,7 @@ sub href(%) {
searchtype => "st",
snapshot_format => "sf",
extra_options => "opt",
+ search_use_regexp => "sr",
);
my %mapping = @mapping;
@@ -2584,6 +2587,10 @@ 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, -label => 're',
+ -checked => $search_use_regexp) .
+ "</span>" .
"</div>" .
$cgi->end_form() . "\n";
}
@@ -5256,7 +5263,8 @@ sub git_search {
}
$greptype .= $searchtext;
my @commitlist = parse_commits($hash, 101, (100 * $page), undef,
- $greptype, '--fixed-strings');
+ $greptype, '--regexp-ignore-case',
+ $search_use_regexp ? '--extended-regexp' : '--fixed-strings');
my $paging_nav = '';
if ($page > 0) {
@@ -5300,8 +5308,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>) {
@@ -5365,7 +5374,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;
@@ -5395,7 +5406,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);
@@ -5430,27 +5441,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) {
@@ -5458,7 +5473,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";
--
1.5.4.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] Add '--fixed-strings' option to "git log --grep" and friends
2008-02-26 12:22 ` [PATCH 1/4] Add '--fixed-strings' option to "git log --grep" and friends Jakub Narebski
@ 2008-02-27 1:03 ` Junio C Hamano
2008-02-27 1:37 ` Jakub Narebski
2008-02-27 9:20 ` [PATCH 1/4 (alternate)] " Jakub Narebski
0 siblings, 2 replies; 9+ messages in thread
From: Junio C Hamano @ 2008-02-27 1:03 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
Jakub Narebski <jnareb@gmail.com> writes:
> Add support for -F | --fixed-strings option to "git log --grep"
> and friends: "git log --author", "git log --committer=<pattern>".
> Code is based on implementation of this option in "git grep".
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This would simplify ignore-case searching for a fixed string from
> within gitweb, as gitweb wouldn't then have to deal with differences
> in quoting and unquoting (if you quote character which doesn't need
> quoting, would git (grep) unquote it?) between searched phrase,
> basic/extended regular expression as understood by git/by grep,
> and regular expressions in Perl (when showing matched info).
>
> [I am not sure if the above paragraph should be added to commit
> message, so it is in patch comments. Feel free to add it.]
I do not understand the issue from reading that paragraph, so it
probably means that (1) it does not help even if it is in the
commit log message, and/or (2) more readable explanation may
help in the commit log message ;-).
The rule for grep input should be known by anybody who writes
scripts around grep, so I do not think this patch is absolutely
necessary if this is only for gitweb. But for command line
end-user usage, fixed string search _might be_ useful, although
I've personally never felt need for that. So I am reluctant to
see it grab a short-and-sweet -F option letter that might have
better uses, but I do not have major objection against a more
explicit --fixed-strings.
By the way, do you allow the default regexp search in gitweb?
If so, how do you handle a malformed regexp that a user gives
you? For example,
$ git log --grep="don\('t" -1
barfs, and I suspect that you can catch the exit status 128 from
die() and say something other than "nothing found" if you really
wanted to.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] Add '--fixed-strings' option to "git log --grep" and friends
2008-02-27 1:03 ` Junio C Hamano
@ 2008-02-27 1:37 ` Jakub Narebski
2008-02-27 9:20 ` [PATCH 1/4 (alternate)] " Jakub Narebski
1 sibling, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2008-02-27 1:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > Add support for -F | --fixed-strings option to "git log --grep"
> > and friends: "git log --author", "git log --committer=<pattern>".
> > Code is based on implementation of this option in "git grep".
> >
> > Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> > ---
> > This would simplify ignore-case searching for a fixed string from
> > within gitweb, as gitweb wouldn't then have to deal with differences
> > in quoting and unquoting (if you quote character which doesn't need
> > quoting, would git (grep) unquote it?) between searched phrase,
> > basic/extended regular expression as understood by git/by grep,
> > and regular expressions in Perl (when showing matched info).
> >
> > [I am not sure if the above paragraph should be added to commit
> > message, so it is in patch comments. Feel free to add it.]
>
> I do not understand the issue from reading that paragraph, so it
> probably means that (1) it does not help even if it is in the
> commit log message, and/or (2) more readable explanation may
> help in the commit log message ;-).
What I meant here that gitweb using --fixed-strings option for
commit message search is example usage of this new feature.
Otherwise we would have to have in gitweb original $searchtext
(for links, description, page title, etc.), $search_grep_regexp
(for grep, or rather for "git log --grep" and friends, basic/extended
regexp meta quoted), and finally $search_regexp to be used in gitweb,
i.e. in Perl to show match.
> The rule for grep input should be known by anybody who writes
> scripts around grep, so I do not think this patch is absolutely
> necessary if this is only for gitweb.
I have written it this way not only because it is simpler than correct
escaping, but also because git-grep has this option (consistency).
Besides it was very easy to add.
> But for command line
> end-user usage, fixed string search _might be_ useful, although
> I've personally never felt need for that. So I am reluctant to
> see it grab a short-and-sweet -F option letter that might have
> better uses, but I do not have major objection against a more
> explicit --fixed-strings.
Feel free to drop support for '-F' short option then, both in code
and in documentation.
I have checked that git-log doesn't support '-F' short option;
additionally '-F' is used in git commands as '--file', i.e. "-F <file>"
to get contents (commit message, tag comment/message). Therefore it was
unlikely that "git log" and friends would acquire "-F <file>" option.
> By the way, do you allow the default regexp search in gitweb?
> If so, how do you handle a malformed regexp that a user gives
> you? For example,
>
> $ git log --grep="don\('t" -1
>
> barfs, and I suspect that you can catch the exit status 128 from
> die() and say something other than "nothing found" if you really
> wanted to.
Errr... to be sure I don't know. From what I have checked it shows
"nothing found", but I guess it could be more explicit.
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/4 (alternate)] Add '--fixed-strings' option to "git log --grep" and friends
2008-02-27 1:03 ` Junio C Hamano
2008-02-27 1:37 ` Jakub Narebski
@ 2008-02-27 9:20 ` Jakub Narebski
2008-02-27 19:47 ` Junio C Hamano
1 sibling, 1 reply; 9+ messages in thread
From: Jakub Narebski @ 2008-02-27 9:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Add support for '--fixed-strings' option to "git log --grep"
and friends: "git log --author", "git log --committer".
Code is based on implementation of this option in "git grep".
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
On Wed, 27 Feb 2008, Junio C Hamano wrote:
> So I am reluctant to
> see it grab a short-and-sweet -F option letter that might have
> better uses, but I do not have major objection against a more
> explicit --fixed-strings.
This version doesn't use '-F' short option.
Documentation/git-rev-list.txt | 1 +
Documentation/rev-list-options.txt | 5 +++++
revision.c | 9 ++++++++-
3 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 5b96eab..0291225 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -31,6 +31,7 @@ SYNOPSIS
[ \--(author|committer|grep)=<pattern> ]
[ \--regexp-ignore-case | \-i ]
[ \--extended-regexp | \-E ]
+ [ \--fixed-strings ]
[ \--date={local|relative|default|iso|rfc|short} ]
[ [\--objects | \--objects-edge] [ \--unpacked ] ]
[ \--pretty | \--header ]
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index a8138e2..826ac62 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -153,6 +153,11 @@ limiting may be applied.
Consider the limiting patterns to be extended regular expressions
instead of the default basic regular expressions.
+--fixed-strings::
+
+ Consider the limiting patterns to be fixed strings (don't interpret
+ pattern as a regular expression).
+
--remove-empty::
Stop when a given path disappears from the tree.
diff --git a/revision.c b/revision.c
index d3e8658..4daeac1 100644
--- a/revision.c
+++ b/revision.c
@@ -942,6 +942,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
int left = 1;
int all_match = 0;
int regflags = 0;
+ int fixed = 0;
/* First, search for "--" */
seen_dashdash = 0;
@@ -1238,6 +1239,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
regflags |= REG_ICASE;
continue;
}
+ if (!strcmp(arg, "--fixed-strings")) {
+ fixed = 1;
+ continue;
+ }
if (!strcmp(arg, "--all-match")) {
all_match = 1;
continue;
@@ -1293,8 +1298,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
}
}
- if (revs->grep_filter)
+ if (revs->grep_filter) {
revs->grep_filter->regflags |= regflags;
+ revs->grep_filter->fixed = fixed;
+ }
if (show_merge)
prepare_show_merge(revs);
--
1.5.4.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4 (alternate)] Add '--fixed-strings' option to "git log --grep" and friends
2008-02-27 9:20 ` [PATCH 1/4 (alternate)] " Jakub Narebski
@ 2008-02-27 19:47 ` Junio C Hamano
0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2008-02-27 19:47 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
Jakub Narebski <jnareb@gmail.com> writes:
>> So I am reluctant to
>> see it grab a short-and-sweet -F option letter that might have
>> better uses, but I do not have major objection against a more
>> explicit --fixed-strings.
>
> This version doesn't use '-F' short option.
Thanks for re-rolling, but after having slept on it, I think
your original is just fine, so I'll take it with short and sweet
"-F" option.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2008-02-27 19:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-26 12:22 [PATCH 0/4] Improve gitweb search, and other things Jakub Narebski
2008-02-26 12:22 ` [PATCH 1/4] Add '--fixed-strings' option to "git log --grep" and friends Jakub Narebski
2008-02-27 1:03 ` Junio C Hamano
2008-02-27 1:37 ` Jakub Narebski
2008-02-27 9:20 ` [PATCH 1/4 (alternate)] " Jakub Narebski
2008-02-27 19:47 ` Junio C Hamano
2008-02-26 12:22 ` [PATCH 2/4] gitweb: Change parse_commits signature to allow for multiple options Jakub Narebski
2008-02-26 12:22 ` [PATCH 3/4] gitweb: Simplify fixed string search Jakub Narebski
2008-02-26 12:22 ` [PATCH 4/4] gitweb: Clearly distinguish regexp / exact match searches Jakub Narebski
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 NNTP newsgroup(s).