Re: [PATCH 1/4] Add '--fixed-strings' option to "git log --grep" and friends

4 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 1/4] Add '--fixed-strings' option to "git log --grep" and friends

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:17

Jakub Narebski [off-list ref] 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 <redacted>
---
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.

Re: [PATCH 1/4] Add '--fixed-strings' option to "git log --grep" and friends

From: Jakub Narebski <hidden>
Date: 2016-06-15 22:44:17

Junio C Hamano wrote:
Jakub Narebski [off-list ref] writes:
quoted
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 <redacted>
---
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

[PATCH 1/4 (alternate)] Add '--fixed-strings' option to "git log --grep" and friends

From: Jakub Narebski <hidden>
Date: 2016-06-15 22:44:17

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

Re: [PATCH 1/4 (alternate)] Add '--fixed-strings' option to "git log --grep" and friends

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:18

Jakub Narebski [off-list ref] writes:
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help