git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Michał Kiedrowicz" <michal.kiedrowicz@gmail.com>
To: Git List <git@vger.kernel.org>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Martin Langhoff" <martin.langhoff@gmail.com>,
	"Bert Wesarg" <bert.wesarg@googlemail.com>,
	"Johannes Sixt" <j.sixt@viscovery.net>,
	"Alex Riesen" <raa.lkml@gmail.com>,
	"Michał Kiedrowicz" <michal.kiedrowicz@gmail.com>
Subject: [PATCH v3 0/6] Add PCRE support to git-grep
Date: Mon,  9 May 2011 23:52:02 +0200	[thread overview]
Message-ID: <1304977928-7142-1-git-send-email-michal.kiedrowicz@gmail.com> (raw)

This is the 3rd attempt to add PCRE support to git-grep.

Changes from v2:

* Replaced NO_LIBPCRE with USE_LIBPCRE. This makes libpcre support
optional. I also considered WITH_LIBPCRE but I see most #defines start
with USE_.

* Renamed 'pcre_extra *extra' with 'pcre_extra *pcre_extra_info'. Now
this variable is more descriptive.

* Reworded description of USE_LIBPCRE in Makefile and configure.ac

* I _didn't_ change the die() message to contain only "Perl-compatible
regexes not supported", but left "cannot use Perl-compatible regexes
when not compiled with USE_LIBPCRE". In my opinion this is more
informative to the user. However, it's OK for me to change that if more
people prefer shorter version.

* Removed die() from pcrematch() and free_pcre_regexp(), because die()
is first called from compile_pcre_regexp(). I left die() there and not
moved it to option parsing because I'd like to keep as few '#ifdef
USE_LIBPCRE' as possible.

* Added compile_regexp_failed() which handles both regcomp() and
pcre_compile() failures.

* Added basic testcases. I'm not sure if I should repeat all
git-grep tests with -P enabled, this seems to be officiousness. Beyond
testing -P/--perl-regexp and their interaction with -i and -w,  I also
check if `git grep -F -P` and `git grep -E -P` was called, but do not
protect from `git grep -G -P` nor `git grep -E -G -P`. Fixing this
would require changing the way -G -E are parsed (currently they
set/unset REG_EXTENDED bit in opts.regflags). Personally, if I were
fixing this, I'd remove regflags completely from grep_opt and use it
only in functions which call regcomp()/regexec() directly. That would
make code more general. But it's also possible to just convert option
parsing code to properly detect used flags. That said, I don't think the
end result is worth the effort.

I'd like to thank for all comments!

Michał Kiedrowicz (6):
  grep: Fix a typo in a comment
  grep: Extract compile_regexp_failed() from compile_regexp()
  git-grep: Learn PCRE
  configure: Check for libpcre
  grep: Add basic tests
  git-grep: Bail out when -P is used with -F or -E

 Documentation/git-grep.txt             |    6 ++
 Makefile                               |   15 +++++
 builtin/grep.c                         |    6 ++-
 config.mak.in                          |    1 +
 configure.ac                           |   40 ++++++++++++
 contrib/completion/git-completion.bash |    1 +
 grep.c                                 |  102 ++++++++++++++++++++++++++++----
 grep.h                                 |    9 +++
 t/README                               |    5 ++
 t/t7810-grep.sh                        |   54 +++++++++++++++++
 t/test-lib.sh                          |    1 +
 11 files changed, 228 insertions(+), 12 deletions(-)

-- 
1.7.3.4

             reply	other threads:[~2011-05-09 21:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-09 21:52 Michał Kiedrowicz [this message]
2011-05-09 21:52 ` [PATCH v3 1/6] grep: Fix a typo in a comment Michał Kiedrowicz
2011-05-09 21:52 ` [PATCH v3 2/6] grep: Extract compile_regexp_failed() from compile_regexp() Michał Kiedrowicz
2011-05-09 21:52 ` [PATCH v3 3/6] git-grep: Learn PCRE Michał Kiedrowicz
2011-05-09 21:52 ` [PATCH v3 4/6] configure: Check for libpcre Michał Kiedrowicz
2011-05-09 21:52 ` [PATCH v3 5/6] grep: Add basic tests Michał Kiedrowicz
2011-05-09 21:52 ` [PATCH v3 6/6] git-grep: Bail out when -P is used with -F or -E Michał Kiedrowicz
2011-05-10  1:48   ` Junio C Hamano
2011-05-10  5:24     ` Michal Kiedrowicz
2011-05-10  6:11       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1304977928-7142-1-git-send-email-michal.kiedrowicz@gmail.com \
    --to=michal.kiedrowicz@gmail.com \
    --cc=bert.wesarg@googlemail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.net \
    --cc=martin.langhoff@gmail.com \
    --cc=raa.lkml@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).