{"thread":{"id":"27312","subject":"[PATCH v3 0/6] Add PCRE support to git-grep","startedAt":"2011-05-09T21:52:02Z","lastAt":"2011-05-10T06:11:25Z","messageCount":10,"participants":["Michał Kiedrowicz","Junio C Hamano","Michal Kiedrowicz"],"isPatch":true,"patchVersion":3,"patchTotal":6},"messages":[{"id":"167522","messageId":"1304977928-7142-1-git-send-email-michal.kiedrowicz@gmail.com","threadId":"27312","inReplyTo":null,"subject":"[PATCH v3 0/6] Add PCRE support to git-grep","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2011-05-09T21:52:02Z","receivedAt":"2011-05-09T21:52:02Z","isPatch":true,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"This is the 3rd attempt to add PCRE support to git-grep.\n\nChanges from v2:\n\n* Replaced NO_LIBPCRE with USE_LIBPCRE. This makes libpcre support\noptional. I also considered WITH_LIBPCRE but I see most #defines start\nwith USE_.\n\n* Renamed 'pcre_extra *extra' with 'pcre_extra *pcre_extra_info'. Now\nthis variable is more descriptive.\n\n* Reworded description of USE_LIBPCRE in Makefile and configure.ac\n\n* I _didn't_ change the die() message to contain only \"Perl-compatible\nregexes not supported\", but left \"cannot use Perl-compatible regexes\nwhen not compiled with USE_LIBPCRE\". In my opinion this is more\ninformative to the user. However, it's OK for me to change that if more\npeople prefer shorter version.\n\n* Removed die() from pcrematch() and free_pcre_regexp(), because die()\nis first called from compile_pcre_regexp(). I left die() there and not\nmoved it to option parsing because I'd like to keep as few '#ifdef\nUSE_LIBPCRE' as possible.\n\n* Added compile_regexp_failed() which handles both regcomp() and\npcre_compile() failures.\n\n* Added basic testcases. I'm not sure if I should repeat all\ngit-grep tests with -P enabled, this seems to be officiousness. Beyond\ntesting -P/--perl-regexp and their interaction with -i and -w, I also\ncheck if `git grep -F -P` and `git grep -E -P` was called, but do not\nprotect from `git grep -G -P` nor `git grep -E -G -P`. Fixing this\nwould require changing the way -G -E are parsed (currently they\nset/unset REG_EXTENDED bit in opts.regflags). Personally, if I were\nfixing this, I'd remove regflags completely from grep_opt and use it\nonly in functions which call regcomp()/regexec() directly. That would\nmake code more general. But it's also possible to just convert option\nparsing code to properly detect used flags. That said, I don't think the\nend result is worth the effort.\n\nI'd like to thank for all comments!\n\nMichał Kiedrowicz (6):\n grep: Fix a typo in a comment\n grep: Extract compile_regexp_failed() from compile_regexp()\n git-grep: Learn PCRE\n configure: Check for libpcre\n grep: Add basic tests\n git-grep: Bail out when -P is used with -F or -E\n\n Documentation/git-grep.txt | 6 ++\n Makefile | 15 +++++\n builtin/grep.c | 6 ++-\n config.mak.in | 1 +\n configure.ac | 40 ++++++++++++\n contrib/completion/git-completion.bash | 1 +\n grep.c | 102 ++++++++++++++++++++++++++++----\n grep.h | 9 +++\n t/README | 5 ++\n t/t7810-grep.sh | 54 +++++++++++++++++\n t/test-lib.sh | 1 +\n 11 files changed, 228 insertions(+), 12 deletions(-)\n\n-- \n1.7.3.4\n"},{"id":"167528","messageId":"1304977928-7142-2-git-send-email-michal.kiedrowicz@gmail.com","threadId":"27312","inReplyTo":"1304977928-7142-1-git-send-email-michal.kiedrowicz@gmail.com","subject":"[PATCH v3 1/6] grep: Fix a typo in a comment","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2011-05-09T21:52:03Z","receivedAt":"2011-05-09T21:52:03Z","isPatch":true,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"\nSigned-off-by: Michał Kiedrowicz \n---\n grep.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex d67baf9..250462e 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -898,7 +898,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,\n \t\tint hit;\n \n \t\t/*\n-\t\t * look_ahead() skips quicly to the line that possibly\n+\t\t * look_ahead() skips quickly to the line that possibly\n \t\t * has the next hit; don't call it if we need to do\n \t\t * something more than just skipping the current line\n \t\t * in response to an unmatch for the current line. E.g.\n-- \n1.7.3.4\n"},{"id":"167525","messageId":"1304977928-7142-3-git-send-email-michal.kiedrowicz@gmail.com","threadId":"27312","inReplyTo":"1304977928-7142-1-git-send-email-michal.kiedrowicz@gmail.com","subject":"[PATCH v3 2/6] grep: Extract compile_regexp_failed() from compile_regexp()","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2011-05-09T21:52:04Z","receivedAt":"2011-05-09T21:52:04Z","isPatch":true,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"This simplifies compile_regexp() a little and allows re-using error\nhandling code.\n\nSigned-off-by: Michał Kiedrowicz \n---\n grep.c | 25 ++++++++++++++++---------\n 1 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 250462e..870d10c 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -59,6 +59,21 @@ struct grep_opt *grep_opt_dup(const struct grep_opt *opt)\n \treturn ret;\n }\n \n+static NORETURN void compile_regexp_failed(const struct grep_pat *p,\n+\t\tconst char *error)\n+{\n+\tchar where[1024];\n+\n+\tif (p->no)\n+\t\tsprintf(where, \"In '%s' at %d, \", p->origin, p->no);\n+\telse if (p->origin)\n+\t\tsprintf(where, \"%s, \", p->origin);\n+\telse\n+\t\twhere[0] = 0;\n+\n+\tdie(\"%s'%s': %s\", where, p->pattern, error);\n+}\n+\n static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n {\n \tint err;\n@@ -73,17 +88,9 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n \terr = regcomp(&p->regexp, p->pattern, opt->regflags);\n \tif (err) {\n \t\tchar errbuf[1024];\n-\t\tchar where[1024];\n-\t\tif (p->no)\n-\t\t\tsprintf(where, \"In '%s' at %d, \",\n-\t\t\t\tp->origin, p->no);\n-\t\telse if (p->origin)\n-\t\t\tsprintf(where, \"%s, \", p->origin);\n-\t\telse\n-\t\t\twhere[0] = 0;\n \t\tregerror(err, &p->regexp, errbuf, 1024);\n \t\tregfree(&p->regexp);\n-\t\tdie(\"%s'%s': %s\", where, p->pattern, errbuf);\n+\t\tcompile_regexp_failed(p, errbuf);\n \t}\n }\n \n-- \n1.7.3.4\n"},{"id":"167527","messageId":"1304977928-7142-4-git-send-email-michal.kiedrowicz@gmail.com","threadId":"27312","inReplyTo":"1304977928-7142-1-git-send-email-michal.kiedrowicz@gmail.com","subject":"[PATCH v3 3/6] git-grep: Learn PCRE","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2011-05-09T21:52:05Z","receivedAt":"2011-05-09T21:52:05Z","isPatch":true,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"This patch teaches git-grep the --perl-regexp/-P options (naming\nborrowed from GNU grep) in order to allow specifying PCRE regexes on the\ncommand line.\n\nPCRE has a number of features which make them more handy to use than\nPOSIX regexes, like consistent escaping rules, extended character\nclasses, ungreedy matching etc.\n\ngit isn't build with PCRE support automatically. USE_LIBPCRE environment\nvariable must be enabled (like `make USE_LIBPCRE=YesPlease`).\n\nSigned-off-by: Michał Kiedrowicz \n---\n Documentation/git-grep.txt | 6 +++\n Makefile | 15 ++++++\n builtin/grep.c | 2 +\n contrib/completion/git-completion.bash | 1 +\n grep.c | 75 +++++++++++++++++++++++++++++++-\n grep.h | 9 ++++\n 6 files changed, 107 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 4a58378..e150c77 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -12,6 +12,7 @@ SYNOPSIS\n 'git grep' [-a | --text] [-I] [-i | --ignore-case] [-w | --word-regexp]\n \t [-v | --invert-match] [-h|-H] [--full-name]\n \t [-E | --extended-regexp] [-G | --basic-regexp]\n+\t [-P | --perl-regexp]\n \t [-F | --fixed-strings] [-n | --line-number]\n \t [-l | --files-with-matches] [-L | --files-without-match]\n \t [(-O | --open-files-in-pager) []]\n@@ -97,6 +98,11 @@ OPTIONS\n \tUse POSIX extended/basic regexp for patterns. Default\n \tis to use basic regexp.\n \n+-P::\n+--perl-regexp::\n+\tUse Perl-compatible regexp for patterns. Requires libpcre to be\n+\tcompiled in.\n+\n -F::\n --fixed-strings::\n \tUse fixed strings for patterns (don't interpret pattern\ndiff --git a/Makefile b/Makefile\nindex 3a1fe20..717a47c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -24,6 +24,12 @@ all::\n # Define NO_OPENSSL environment variable if you do not have OpenSSL.\n # This also implies BLK_SHA1.\n #\n+# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be\n+# able to use Perl-compatible regular expressions.\n+#\n+# Define LIBPCREDIR=/foo/bar if your libpcre header and library files are in\n+# /foo/bar/include and /foo/bar/lib directories.\n+#\n # Define NO_CURL if you do not have libcurl installed. git-http-pull and\n # git-http-push are not built, and you cannot use http:// and https://\n # transports.\n@@ -1251,6 +1257,15 @@ ifdef NO_LIBGEN_H\n \tCOMPAT_OBJS += compat/basename.o\n endif\n \n+ifdef USE_LIBPCRE\n+\tBASIC_CFLAGS += -DUSE_LIBPCRE\n+\tifdef LIBPCREDIR\n+\t\tBASIC_CFLAGS += -I$(LIBPCREDIR)/include\n+\t\tEXTLIBS += -L$(LIBPCREDIR)/$(lib) $(CC_LD_DYNPATH)$(LIBPCREDIR)/$(lib)\n+\tendif\n+\tEXTLIBS += -lpcre\n+endif\n+\n ifdef NO_CURL\n \tBASIC_CFLAGS += -DNO_CURL\n \tREMOTE_CURL_PRIMARY =\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 10a1f65..6831975 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -781,6 +781,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\tREG_EXTENDED),\n \t\tOPT_BOOLEAN('F', \"fixed-strings\", &opt.fixed,\n \t\t\t\"interpret patterns as fixed strings\"),\n+\t\tOPT_BOOLEAN('P', \"perl-regexp\", &opt.pcre,\n+\t\t\t\t\"use Perl-compatible regular expressions\"),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOLEAN('n', \"line-number\", &opt.linenum, \"show line numbers\"),\n \t\tOPT_NEGBIT('h', NULL, &opt.pathname, \"don't show filenames\", 1),\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 4b2654d..95790a1 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1487,6 +1487,7 @@ _git_grep ()\n \t\t\t--text --ignore-case --word-regexp --invert-match\n \t\t\t--full-name --line-number\n \t\t\t--extended-regexp --basic-regexp --fixed-strings\n+\t\t\t--perl-regexp\n \t\t\t--files-with-matches --name-only\n \t\t\t--files-without-match\n \t\t\t--max-depth\ndiff --git a/grep.c b/grep.c\nindex 870d10c..d03d9e2 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -74,6 +74,69 @@ static NORETURN void compile_regexp_failed(const struct grep_pat *p,\n \tdie(\"%s'%s': %s\", where, p->pattern, error);\n }\n \n+#ifdef USE_LIBPCRE\n+static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)\n+{\n+\tconst char *error;\n+\tint erroffset;\n+\tint options = 0;\n+\n+\tif (opt->ignore_case)\n+\t\toptions |= PCRE_CASELESS;\n+\n+\tp->pcre_regexp = pcre_compile(p->pattern, options, &error, &erroffset,\n+\t\t\tNULL);\n+\tif (!p->pcre_regexp)\n+\t\tcompile_regexp_failed(p, error);\n+\n+\tp->pcre_extra_info = pcre_study(p->pcre_regexp, 0, &error);\n+\tif (!p->pcre_extra_info && error)\n+\t\tdie(\"%s\", error);\n+}\n+\n+static int pcrematch(struct grep_pat *p, const char *line, const char *eol,\n+\t\tregmatch_t *match, int eflags)\n+{\n+\tint ovector[30], ret, flags = 0;\n+\n+\tif (eflags & REG_NOTBOL)\n+\t\tflags |= PCRE_NOTBOL;\n+\n+\tret = pcre_exec(p->pcre_regexp, p->pcre_extra_info, line, eol - line,\n+\t\t\t0, flags, ovector, ARRAY_SIZE(ovector));\n+\tif (ret < 0 && ret != PCRE_ERROR_NOMATCH)\n+\t\tdie(\"pcre_exec failed with error code %d\", ret);\n+\tif (ret > 0) {\n+\t\tret = 0;\n+\t\tmatch->rm_so = ovector[0];\n+\t\tmatch->rm_eo = ovector[1];\n+\t}\n+\n+\treturn ret;\n+}\n+\n+static void free_pcre_regexp(struct grep_pat *p)\n+{\n+\tpcre_free(p->pcre_regexp);\n+\tpcre_free(p->pcre_extra_info);\n+}\n+#else /* !USE_LIBPCRE */\n+static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)\n+{\n+\tdie(\"cannot use Perl-compatible regexes when not compiled with USE_LIBPCRE\");\n+}\n+\n+static int pcrematch(struct grep_pat *p, const char *line, const char *eol,\n+\t\tregmatch_t *match, int eflags)\n+{\n+\treturn 1;\n+}\n+\n+static void free_pcre_regexp(struct grep_pat *p)\n+{\n+}\n+#endif /* !USE_LIBPCRE */\n+\n static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n {\n \tint err;\n@@ -85,6 +148,11 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n \tif (p->fixed)\n \t\treturn;\n \n+\tif (opt->pcre) {\n+\t\tcompile_pcre_regexp(p, opt);\n+\t\treturn;\n+\t}\n+\n \terr = regcomp(&p->regexp, p->pattern, opt->regflags);\n \tif (err) {\n \t\tchar errbuf[1024];\n@@ -327,7 +395,10 @@ void free_grep_patterns(struct grep_opt *opt)\n \t\tcase GREP_PATTERN: /* atom */\n \t\tcase GREP_PATTERN_HEAD:\n \t\tcase GREP_PATTERN_BODY:\n-\t\t\tregfree(&p->regexp);\n+\t\t\tif (p->pcre_regexp)\n+\t\t\t\tfree_pcre_regexp(p);\n+\t\t\telse\n+\t\t\t\tregfree(&p->regexp);\n \t\t\tbreak;\n \t\tdefault:\n \t\t\tbreak;\n@@ -426,6 +497,8 @@ static int patmatch(struct grep_pat *p, char *line, char *eol,\n \n \tif (p->fixed)\n \t\thit = !fixmatch(p, line, eol, match);\n+\telse if (p->pcre_regexp)\n+\t\thit = !pcrematch(p, line, eol, match, eflags);\n \telse\n \t\thit = !regmatch(&p->regexp, line, eol, match, eflags);\n \ndiff --git a/grep.h b/grep.h\nindex 06621fe..cd055cd 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -1,6 +1,12 @@\n #ifndef GREP_H\n #define GREP_H\n #include \"color.h\"\n+#ifdef USE_LIBPCRE\n+#include \n+#else\n+typedef int pcre;\n+typedef int pcre_extra;\n+#endif\n \n enum grep_pat_token {\n \tGREP_PATTERN,\n@@ -33,6 +39,8 @@ struct grep_pat {\n \tsize_t patternlen;\n \tenum grep_header_field field;\n \tregex_t regexp;\n+\tpcre *pcre_regexp;\n+\tpcre_extra *pcre_extra_info;\n \tunsigned fixed:1;\n \tunsigned ignore_case:1;\n \tunsigned word_regexp:1;\n@@ -83,6 +91,7 @@ struct grep_opt {\n #define GREP_BINARY_TEXT\t2\n \tint binary;\n \tint extended;\n+\tint pcre;\n \tint relative;\n \tint pathname;\n \tint null_following_name;\n-- \n1.7.3.4\n"},{"id":"167524","messageId":"1304977928-7142-5-git-send-email-michal.kiedrowicz@gmail.com","threadId":"27312","inReplyTo":"1304977928-7142-1-git-send-email-michal.kiedrowicz@gmail.com","subject":"[PATCH v3 4/6] configure: Check for libpcre","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2011-05-09T21:52:06Z","receivedAt":"2011-05-09T21:52:06Z","isPatch":true,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"This patch adds checks for libpcre to configure. By default libpcre is\ndisabled, --with-libpcre enables it (if it works).\n\nSigned-off-by: Michał Kiedrowicz \n---\n config.mak.in | 1 +\n configure.ac | 40 ++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 41 insertions(+), 0 deletions(-)\n\ndiff --git a/config.mak.in b/config.mak.in\nindex e378534..f30130b 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -61,6 +61,7 @@ NO_INET_PTON=@NO_INET_PTON@\n NO_ICONV=@NO_ICONV@\n OLD_ICONV=@OLD_ICONV@\n NO_REGEX=@NO_REGEX@\n+USE_LIBPCRE=@USE_LIBPCRE@\n NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@\n INLINE=@INLINE@\n SOCKLEN_T=@SOCKLEN_T@\ndiff --git a/configure.ac b/configure.ac\nindex fafd815..048a1d4 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -220,6 +220,27 @@ AS_HELP_STRING([--with-openssl],[use OpenSSL library (default is YES)])\n AS_HELP_STRING([], [ARG can be prefix for openssl library and headers]),\\\n GIT_PARSE_WITH(openssl))\n #\n+# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be\n+# able to use Perl-compatible regular expressions.\n+#\n+# Define LIBPCREDIR=/foo/bar if your libpcre header and library files are in\n+# /foo/bar/include and /foo/bar/lib directories.\n+#\n+AC_ARG_WITH(libpcre,\n+AS_HELP_STRING([--with-libpcre],[support Perl-compatible regexes (default is NO)])\n+AS_HELP_STRING([], [ARG can be also prefix for libpcre library and headers]),\n+if test \"$withval\" = \"no\"; then \\\n+\tUSE_LIBPCRE=; \\\n+elif test \"$withval\" = \"yes\"; then \\\n+\tUSE_LIBPCRE=YesPlease; \\\n+else\n+\tUSE_LIBPCRE=YesPlease; \\\n+\tLIBPCREDIR=$withval; \\\n+\tAC_MSG_NOTICE([Setting LIBPCREDIR to $withval]); \\\n+\tGIT_CONF_APPEND_LINE(LIBPCREDIR=$withval); \\\n+fi \\\n+)\n+#\n # Define NO_CURL if you do not have curl installed. git-http-pull and\n # git-http-push are not built, and you cannot use http:// and https://\n # transports.\n@@ -435,6 +456,25 @@ AC_SUBST(NEEDS_SSL_WITH_CRYPTO)\n AC_SUBST(NO_OPENSSL)\n \n #\n+# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be\n+# able to use Perl-compatible regular expressions.\n+#\n+\n+if test -n \"$USE_LIBPCRE\"; then\n+\n+GIT_STASH_FLAGS($LIBPCREDIR)\n+\n+AC_CHECK_LIB([pcre], [pcre_version],\n+[USE_LIBPCRE=YesPlease],\n+[USE_LIBPCRE=])\n+\n+GIT_UNSTASH_FLAGS($LIBPCREDIR)\n+\n+AC_SUBST(USE_LIBPCRE)\n+\n+fi\n+\n+#\n # Define NO_CURL if you do not have libcurl installed. git-http-pull and\n # git-http-push are not built, and you cannot use http:// and https://\n # transports.\n-- \n1.7.3.4\n"},{"id":"167526","messageId":"1304977928-7142-6-git-send-email-michal.kiedrowicz@gmail.com","threadId":"27312","inReplyTo":"1304977928-7142-1-git-send-email-michal.kiedrowicz@gmail.com","subject":"[PATCH v3 5/6] grep: Add basic tests","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2011-05-09T21:52:07Z","receivedAt":"2011-05-09T21:52:07Z","isPatch":true,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"This modest patch adds simple tests for git grep -P/--perl-regexp and\nits interoperation with -i and -w.\n\nTests are only enabled when prerequisite LIBPCRE is defined (it's\nautomatically set based on USE_LIBPCRE in test-lib.sh).\n\nSigned-off-by: Michał Kiedrowicz \n---\n t/README | 5 +++++\n t/t7810-grep.sh | 38 ++++++++++++++++++++++++++++++++++++++\n t/test-lib.sh | 1 +\n 3 files changed, 44 insertions(+), 0 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 428ee05..238729c 100644\n--- a/t/README\n+++ b/t/README\n@@ -587,6 +587,11 @@ use these, and \"test_set_prereq\" for how to define your own.\n Test is not run by root user, and an attempt to write to an\n unwritable file is expected to fail correctly.\n \n+ - LIBPCRE\n+\n+ Git was compiled with USE_LIBPCRE=YesPlease. Wrap any tests\n+ that use git-grep --perl-regexp or git-grep -P in these.\n+\n Tips for Writing Tests\n ----------------------\n \ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 8184c26..e845218 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -26,6 +26,12 @@ test_expect_success setup '\n \t\techo foo mmap bar_mmap\n \t\techo foo_mmap bar mmap baz\n \t} >file &&\n+\t{\n+\t\techo Hello world\n+\t\techo HeLLo world\n+\t\techo Hello_world\n+\t\techo HeLLo_world\n+\t} >hello_world &&\n \techo vvv >v &&\n \techo ww w >w &&\n \techo x x xx x >x &&\n@@ -599,4 +605,36 @@ test_expect_success 'grep -e -- -- path' '\n \ttest_cmp expected actual\n '\n \n+cat >expected <actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success LIBPCRE 'grep -P pattern' '\n+\tgit grep -P \"\\p{Ps}.*?\\p{Pe}\" hello.c >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success LIBPCRE 'grep -P -i pattern' '\n+\t{\n+\t\techo \"hello.c:\tprintf(\\\"Hello world.\\n\\\");\"\n+\t} >expected &&\n+\tgit grep -P -i \"PRINTF\\([^\\d]+\\)\" hello.c >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success LIBPCRE 'grep -P -w pattern' '\n+\t{\n+\t\techo \"hello_world:Hello world\"\n+\t\techo \"hello_world:HeLLo world\"\n+\t} >expected &&\n+\tgit grep -P -w \"He((?i)ll)o\" hello_world >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex c5b18e2..368f7ae 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1069,6 +1069,7 @@ esac\n \n test -z \"$NO_PERL\" && test_set_prereq PERL\n test -z \"$NO_PYTHON\" && test_set_prereq PYTHON\n+test -n \"$USE_LIBPCRE\" && test_set_prereq LIBPCRE\n \n # Can we rely on git's output in the C locale?\n if test -n \"$GETTEXT_POISON\"\n-- \n1.7.3.4\n"},{"id":"167523","messageId":"1304977928-7142-7-git-send-email-michal.kiedrowicz@gmail.com","threadId":"27312","inReplyTo":"1304977928-7142-1-git-send-email-michal.kiedrowicz@gmail.com","subject":"[PATCH v3 6/6] git-grep: Bail out when -P is used with -F or -E","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2011-05-09T21:52:08Z","receivedAt":"2011-05-09T21:52:08Z","isPatch":true,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"This patch makes git-grep die() when -P is used on command line together\nwith -E/--extended-regexp or -F/--fixed-strings.\n\nThis also makes it bail out when grep.extendedRegexp is enabled.\n\nBut `git grep -G -P pattern` and `git grep -E -G -P pattern` still work\nbecause -G and -E set opts.regflags during parse_options() and there is\nno way to detect `-G` or `-E -G`.\n\nSigned-off-by: Michał Kiedrowicz \n---\n builtin/grep.c | 4 +++-\n t/t7810-grep.sh | 16 ++++++++++++++++\n 2 files changed, 19 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 6831975..8f26026 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -925,9 +925,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \n \tif (!opt.pattern_list)\n \t\tdie(_(\"no pattern given.\"));\n+\tif (opt.regflags != REG_NEWLINE && opt.pcre)\n+\t\tdie(_(\"cannot mix --extended-regexp and --perl-regexp\"));\n \tif (!opt.fixed && opt.ignore_case)\n \t\topt.regflags |= REG_ICASE;\n-\tif ((opt.regflags != REG_NEWLINE) && opt.fixed)\n+\tif ((opt.regflags != REG_NEWLINE || opt.pcre) && opt.fixed)\n \t\tdie(_(\"cannot mix --fixed-strings and regexp\"));\n \n #ifndef NO_PTHREADS\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex e845218..2a31eca 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -637,4 +637,20 @@ test_expect_success LIBPCRE 'grep -P -w pattern' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success LIBPCRE 'grep -P -F returns error' '\n+\ttest_expect_code 128 git grep -P -F main\n+'\n+\n+test_expect_success LIBPCRE 'grep -P -E returns error' '\n+\ttest_expect_code 128 git grep -P -E main\n+'\n+\n+test_expect_failure LIBPCRE 'grep -P -G returns error' '\n+\ttest_expect_code 128 git grep -P -G main\n+'\n+\n+test_expect_failure LIBPCRE 'grep -P -E -G returns error' '\n+\ttest_expect_code 128 git grep -P -E -G main\n+'\n+\n test_done\n-- \n1.7.3.4\n"},{"id":"167563","messageId":"7vfwonmikr.fsf@alter.siamese.dyndns.org","threadId":"27312","inReplyTo":"1304977928-7142-7-git-send-email-michal.kiedrowicz@gmail.com","subject":"Re: [PATCH v3 6/6] git-grep: Bail out when -P is used with -F or -E","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-10T01:48:36Z","receivedAt":"2011-05-10T01:48:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michał Kiedrowicz writes:\n\n> This also makes it bail out when grep.extendedRegexp is enabled.\n\nThat is a no-starter. \"git grep -P foo\" from the command line should just\nignore the configured default. It is not entirely your fault, as I think\nyou inherited the bug from the existing code that lets grep.extendedRegexp\ninteract with the \"--fixed\" option from the command line.\n\n> But `git grep -G -P pattern` and `git grep -E -G -P pattern` still work\n> because -G and -E set opts.regflags during parse_options() and there is\n> no way to detect `-G` or `-E -G`.\n\nHow about following the usual pattern of letting the last one win?\n\nPerhaps like this? This is not even compile tested, but should apply\ncleanly on top of, and can be squashed into, your 6/6. You of course\nwould need to rewrite the commit log message and documentation, if you\nsaid only one of these can be used.\n\nWe would need some tests for \"grep -P\", no? Please throw in the \"last one\nwins\" and \"command line defeats configuration\" when you add one.\n\nAlso I deliberately said \"--ignore-case and -P are not compatible (yet)\";\nshouldn't you be able to do ignore case fairly easily, I wonder? Isn't it\njust the matter of wrapping each one with \"(?i:\" and \")\" pair, or anything\nmore involved necessary?\n\n builtin/grep.c | 58 +++++++++++++++++++++++++++++++++++++++++++------------\n 1 files changed, 45 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 8e422b3..37f2331 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -753,6 +753,15 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tint i;\n \tint dummy;\n \tint use_index = 1;\n+\tenum {\n+\t\tpattern_type_unspecified = 0,\n+\t\tpattern_type_bre,\n+\t\tpattern_type_ere,\n+\t\tpattern_type_fixed,\n+\t\tpattern_type_pcre,\n+\t};\n+\tint pattern_type = pattern_type_unspecified;\n+\n \tstruct option options[] = {\n \t\tOPT_BOOLEAN(0, \"cached\", &cached,\n \t\t\t\"search in index instead of in the work tree\"),\n@@ -774,15 +783,18 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t\"descend at most levels\", PARSE_OPT_NONEG,\n \t\t\tNULL, 1 },\n \t\tOPT_GROUP(\"\"),\n-\t\tOPT_BIT('E', \"extended-regexp\", &opt.regflags,\n-\t\t\t\"use extended POSIX regular expressions\", REG_EXTENDED),\n-\t\tOPT_NEGBIT('G', \"basic-regexp\", &opt.regflags,\n-\t\t\t\"use basic POSIX regular expressions (default)\",\n-\t\t\tREG_EXTENDED),\n-\t\tOPT_BOOLEAN('F', \"fixed-strings\", &opt.fixed,\n-\t\t\t\"interpret patterns as fixed strings\"),\n-\t\tOPT_BOOLEAN('P', \"perl-regexp\", &opt.pcre,\n-\t\t\t\t\"use Perl-compatible regular expressions\"),\n+\t\tOPT_SET_INT('E', \"extended-regexp\", &pattern_type,\n+\t\t\t \"use extended POSIX regular expressions\",\n+\t\t\t pattern_type_ere),\n+\t\tOPT_SET_INT('G', \"basic-regexp\", &pattern_type,\n+\t\t\t \"use basic POSIX regular expressions (default)\",\n+\t\t\t pattern_type_bre),\n+\t\tOPT_SET_INT('F', \"fixed-strings\", &pattern_type,\n+\t\t\t \"interpret patterns as fixed strings\",\n+\t\t\t pattern_type_fixed),\n+\t\tOPT_SET_INT('P', \"perl-regexp\", &pattern_type,\n+\t\t\t \"use Perl-compatible regular expressions\",\n+\t\t\t pattern_type_pcre),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOLEAN('n', \"line-number\", &opt.linenum, \"show line numbers\"),\n \t\tOPT_NEGBIT('h', NULL, &opt.pathname, \"don't show filenames\", 1),\n@@ -888,6 +900,28 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_KEEP_DASHDASH |\n \t\t\t PARSE_OPT_STOP_AT_NON_OPTION |\n \t\t\t PARSE_OPT_NO_INTERNAL_HELP);\n+\tswitch (pattern_type) {\n+\tcase pattern_type_fixed:\n+\t\topt.fixed = 1;\n+\t\topt.pcre = 0;\n+\t\tbreak;\n+\tcase pattern_type_bre:\n+\t\topt.fixed = 0;\n+\t\topt.pcre = 0;\n+\t\topt.regflags &= ~REG_EXTENDED;\n+\t\tbreak;\n+\tcase pattern_type_ere:\n+\t\topt.fixed = 0;\n+\t\topt.pcre = 0;\n+\t\topt.regflags |= REG_EXTENDED;\n+\t\tbreak;\n+\tcase pattern_type_pcre:\n+\t\topt.fixed = 0;\n+\t\topt.pcre = 1;\n+\t\tbreak;\n+\tdefault:\n+\t\tbreak; /* nothing */\n+\t}\n \n \tif (use_index && !startup_info->have_repository)\n \t\t/* die the same way as if we did it at the beginning */\n@@ -925,12 +959,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \n \tif (!opt.pattern_list)\n \t\tdie(_(\"no pattern given.\"));\n-\tif (opt.regflags != REG_NEWLINE && opt.pcre)\n-\t\tdie(_(\"cannot mix --extended-regexp and --perl-regexp\"));\n \tif (!opt.fixed && opt.ignore_case)\n \t\topt.regflags |= REG_ICASE;\n-\tif ((opt.regflags != REG_NEWLINE || opt.pcre) && opt.fixed)\n-\t\tdie(_(\"cannot mix --fixed-strings and regexp\"));\n+\tif (opt.pcre && opt.ignore_case)\n+\t\tdie(_(\"--ignore-case and -P are not compatible (yet)\"));\n \n #ifndef NO_PTHREADS\n \tif (online_cpus() == 1 || !grep_threads_ok(&opt))\n"},{"id":"167569","messageId":"20110510072439.6b288715@mkiedrowicz","threadId":"27312","inReplyTo":"7vfwonmikr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 6/6] git-grep: Bail out when -P is used with -F or -E","fromName":"Michal Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2011-05-10T05:24:39Z","receivedAt":"2011-05-10T05:24:39Z","isPatch":true,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"On 09.05.2011 18:48:36 -0700 Junio C Hamano wrote:\n\n> Michał Kiedrowicz writes:\n> \n> > This also makes it bail out when grep.extendedRegexp is enabled.\n> \n> That is a no-starter. \"git grep -P foo\" from the command line should\n> just ignore the configured default. It is not entirely your fault,\n> as I think you inherited the bug from the existing code that lets\n> grep.extendedRegexp interact with the \"--fixed\" option from the\n> command line.\n> \n> > But `git grep -G -P pattern` and `git grep -E -G -P pattern` still\n> > work because -G and -E set opts.regflags during parse_options() and\n> > there is no way to detect `-G` or `-E -G`.\n> \n> How about following the usual pattern of letting the last one win?\n> \n> Perhaps like this? This is not even compile tested, but should apply\n> cleanly on top of, and can be squashed into, your 6/6. You of course\n> would need to rewrite the commit log message and documentation, if you\n> said only one of these can be used.\n\nSounds good, thanks.\n\n> \n> We would need some tests for \"grep -P\", no? \n\nWhat about those in patch 5/6?\n\n> Please throw in the\n> \"last one wins\" and \"command line defeats configuration\" when you add\n> one.\n> \n> Also I deliberately said \"--ignore-case and -P are not compatible\n> (yet)\"; shouldn't you be able to do ignore case fairly easily, I\n> wonder? Isn't it just the matter of wrapping each one with \"(?i:\"\n> and \")\" pair, or anything more involved necessary?\n\nIf you look at patch 3/6 you will see:\n\n+\n+\tif (opt->ignore_case)\n+\t\toptions |= PCRE_CASELESS;\n+\n+\tp->pcre_regexp = pcre_compile(p->pattern, options, &error,\n&erroffset,\n+\t\t\tNULL);\n\nand also\n\n+test_expect_success LIBPCRE 'grep -P -i pattern' '\n\nin patch 5/6 :). Or perhaps it doesn't work for you?\n\n> \n> builtin/grep.c | 58\n> +++++++++++++++++++++++++++++++++++++++++++------------ 1 files\n> changed, 45 insertions(+), 13 deletions(-)\n> \n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 8e422b3..37f2331 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -753,6 +753,15 @@ int cmd_grep(int argc, const char **argv, const\n> char *prefix) int i;\n> \tint dummy;\n> \tint use_index = 1;\n> +\tenum {\n> +\t\tpattern_type_unspecified = 0,\n> +\t\tpattern_type_bre,\n> +\t\tpattern_type_ere,\n> +\t\tpattern_type_fixed,\n> +\t\tpattern_type_pcre,\n> +\t};\n> +\tint pattern_type = pattern_type_unspecified;\n> +\n> \tstruct option options[] = {\n> \t\tOPT_BOOLEAN(0, \"cached\", &cached,\n> \t\t\t\"search in index instead of in the work\n> tree\"), @@ -774,15 +783,18 @@ int cmd_grep(int argc, const char\n> **argv, const char *prefix) \"descend at most levels\",\n> PARSE_OPT_NONEG, NULL, 1 },\n> \t\tOPT_GROUP(\"\"),\n> -\t\tOPT_BIT('E', \"extended-regexp\", &opt.regflags,\n> -\t\t\t\"use extended POSIX regular expressions\",\n> REG_EXTENDED),\n> -\t\tOPT_NEGBIT('G', \"basic-regexp\", &opt.regflags,\n> -\t\t\t\"use basic POSIX regular expressions\n> (default)\",\n> -\t\t\tREG_EXTENDED),\n> -\t\tOPT_BOOLEAN('F', \"fixed-strings\", &opt.fixed,\n> -\t\t\t\"interpret patterns as fixed strings\"),\n> -\t\tOPT_BOOLEAN('P', \"perl-regexp\", &opt.pcre,\n> -\t\t\t\t\"use Perl-compatible regular\n> expressions\"),\n> +\t\tOPT_SET_INT('E', \"extended-regexp\", &pattern_type,\n> +\t\t\t \"use extended POSIX regular expressions\",\n> +\t\t\t pattern_type_ere),\n> +\t\tOPT_SET_INT('G', \"basic-regexp\", &pattern_type,\n> +\t\t\t \"use basic POSIX regular expressions\n> (default)\",\n> +\t\t\t pattern_type_bre),\n> +\t\tOPT_SET_INT('F', \"fixed-strings\", &pattern_type,\n> +\t\t\t \"interpret patterns as fixed strings\",\n> +\t\t\t pattern_type_fixed),\n> +\t\tOPT_SET_INT('P', \"perl-regexp\", &pattern_type,\n> +\t\t\t \"use Perl-compatible regular\n> expressions\",\n> +\t\t\t pattern_type_pcre),\n> \t\tOPT_GROUP(\"\"),\n> \t\tOPT_BOOLEAN('n', \"line-number\", &opt.linenum, \"show\n> line numbers\"), OPT_NEGBIT('h', NULL, &opt.pathname, \"don't show\n> filenames\", 1), @@ -888,6 +900,28 @@ int cmd_grep(int argc, const\n> char **argv, const char *prefix) PARSE_OPT_KEEP_DASHDASH |\n> \t\t\t PARSE_OPT_STOP_AT_NON_OPTION |\n> \t\t\t PARSE_OPT_NO_INTERNAL_HELP);\n> +\tswitch (pattern_type) {\n> +\tcase pattern_type_fixed:\n> +\t\topt.fixed = 1;\n> +\t\topt.pcre = 0;\n> +\t\tbreak;\n> +\tcase pattern_type_bre:\n> +\t\topt.fixed = 0;\n> +\t\topt.pcre = 0;\n> +\t\topt.regflags &= ~REG_EXTENDED;\n> +\t\tbreak;\n> +\tcase pattern_type_ere:\n> +\t\topt.fixed = 0;\n> +\t\topt.pcre = 0;\n> +\t\topt.regflags |= REG_EXTENDED;\n> +\t\tbreak;\n> +\tcase pattern_type_pcre:\n> +\t\topt.fixed = 0;\n> +\t\topt.pcre = 1;\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tbreak; /* nothing */\n> +\t}\n> \n> \tif (use_index && !startup_info->have_repository)\n> \t\t/* die the same way as if we did it at the beginning\n> */ @@ -925,12 +959,10 @@ int cmd_grep(int argc, const char **argv,\n> const char *prefix) \n> \tif (!opt.pattern_list)\n> \t\tdie(_(\"no pattern given.\"));\n> -\tif (opt.regflags != REG_NEWLINE && opt.pcre)\n> -\t\tdie(_(\"cannot mix --extended-regexp and\n> --perl-regexp\")); if (!opt.fixed && opt.ignore_case)\n> \t\topt.regflags |= REG_ICASE;\n> -\tif ((opt.regflags != REG_NEWLINE || opt.pcre) && opt.fixed)\n> -\t\tdie(_(\"cannot mix --fixed-strings and regexp\"));\n> +\tif (opt.pcre && opt.ignore_case)\n> +\t\tdie(_(\"--ignore-case and -P are not compatible\n> (yet)\")); \n> #ifndef NO_PTHREADS\n> \tif (online_cpus() == 1 || !grep_threads_ok(&opt))\n> \n"},{"id":"167580","messageId":"7v4o53m6eq.fsf@alter.siamese.dyndns.org","threadId":"27312","inReplyTo":"20110510072439.6b288715@mkiedrowicz","subject":"Re: [PATCH v3 6/6] git-grep: Bail out when -P is used with -F or -E","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-10T06:11:25Z","receivedAt":"2011-05-10T06:11:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Kiedrowicz writes:\n\n>> We would need some tests for \"grep -P\", no? \n>\n> What about those in patch 5/6?\n\nThere are some, but we would also want to see negative cases where\ncompilation detects an incorrect regexp.\n\n>> Please throw in the\n>> \"last one wins\" and \"command line defeats configuration\" when you add\n>> one.\n\n> +test_expect_success LIBPCRE 'grep -P -i pattern' '\n>\n> in patch 5/6 :). Or perhaps it doesn't work for you?\n\nThese I overlooked. Will remove the \"-i -P does not work yet\".\n\nThanks.\n"}]}