git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>,
	git@vger.kernel.org, Thomas Rast <trast@student.ethz.ch>,
	Ryan Anderson <rda@google.com>,
	Jay Soffian <jaysoffian@gmail.com>
Subject: Re: [PATCH 13/16] send-email: extract_valid_address use qr// regexes and /o
Date: Thu, 30 Sep 2010 18:56:17 +0000	[thread overview]
Message-ID: <AANLkTin_b6eSnHgY2YCOfN7E83gEzrOpktUD3heOnfQw@mail.gmail.com> (raw)
In-Reply-To: <7vocbfjg7x.fsf@alter.siamese.dyndns.org>

On Thu, Sep 30, 2010 at 17:25, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> ...
>> But we are unnecessarily compiling the sub-regexes each time. Not that
>> this is probably a performance critical piece of code, but your "/o" is
>> doing very little, and this is exactly the sort perl wankery that I find
>> interesting.
>
> Well, isn't the _sole_ point of using qr// to optimize by avoiding
> recompilation?  If this is not a performance critical section of the code,
> what is the point of this change?
>
> This [PATCH 13/16] and also [PATCH 12/16] rewrite strings using qr// but
> the patterns thus compiled are used exactly once before the control leaves
> the scope of the variables, so...

The point is to use Perl's data-types for what they're supposed to be
used for. In perl you *can* write:

    my $two = "1" + "1";

To get 2, but that's silly. You should just use integers instead of
string coercion:

    my $two = 1 + 1;

Similarly you *can* put regexes in strings, but perl has a first-class
datatype for it, so you should do:

    my $rx = qr/foo/;

Not:

    my $rx = "foo";

So it's just a change to use a better style. There are some tangible
advantages to using qr// over qq// (also known as ""), e.g. Emacs and
Vim will know to highlight the string as a regexp, and when the $rx is
interpolated Perl won't need to recompile it because it can just add a
pointer to the existing regex in the new pattern.

But the main reason here is to not write code that looks like it
targets Perl 5.5, but 5.8.

  reply	other threads:[~2010-09-30 18:56 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-30 13:42 [PATCH 00/16] git-send-email cleanups Ævar Arnfjörð Bjarmason
2010-09-30 13:42 ` [PATCH 01/16] send-email: use lexical filehandle for opendir Ævar Arnfjörð Bjarmason
2010-09-30 13:42 ` [PATCH 02/16] send-email: use lexical filehandles for $compose Ævar Arnfjörð Bjarmason
2010-09-30 13:42 ` [PATCH 03/16] send-email: use lexical filehandles during sending Ævar Arnfjörð Bjarmason
2010-09-30 13:42 ` [PATCH 04/16] send-email: get_patch_subject doesn't need a prototype Ævar Arnfjörð Bjarmason
2010-09-30 13:42 ` [PATCH 05/16] send-email: file_declares_8bit_cte " Ævar Arnfjörð Bjarmason
2010-09-30 13:42 ` [PATCH 06/16] send-email: unique_email_list " Ævar Arnfjörð Bjarmason
2010-09-30 13:43 ` [PATCH 07/16] send-email: cleanup_compose_files " Ævar Arnfjörð Bjarmason
2010-09-30 13:43 ` [PATCH 08/16] send-email: use \E***\Q instead of \*\*\* Ævar Arnfjörð Bjarmason
2010-09-30 13:43 ` [PATCH 09/16] send-email: sanitize_address use $foo, not "$foo" Ævar Arnfjörð Bjarmason
2010-09-30 13:43 ` [PATCH 10/16] send-email: sanitize_address use qq["foo"], not "\"foo\"" Ævar Arnfjörð Bjarmason
2010-09-30 13:43 ` [PATCH 11/16] send-email: use (?:) instead of () if no match variables are needed Ævar Arnfjörð Bjarmason
2010-09-30 13:43 ` [PATCH 12/16] send-email: is_rfc2047_quoted use qr// regexes Ævar Arnfjörð Bjarmason
2010-09-30 13:43 ` [PATCH 13/16] send-email: extract_valid_address use qr// regexes and /o Ævar Arnfjörð Bjarmason
2010-09-30 16:19   ` Jeff King
2010-09-30 16:33     ` Ævar Arnfjörð Bjarmason
2010-10-01  5:40       ` Jeff King
2010-09-30 17:25     ` Junio C Hamano
2010-09-30 18:56       ` Ævar Arnfjörð Bjarmason [this message]
2010-09-30 19:03   ` [PATCH v2 13/16] send-email: extract_valid_address use qr// regexes Ævar Arnfjörð Bjarmason
2010-09-30 13:43 ` [PATCH 14/16] send-email: send_message die on $!, not $? Ævar Arnfjörð Bjarmason
2010-09-30 13:43 ` [PATCH 15/16] send-email: make_message_id use "require" instead of "use" Ævar Arnfjörð Bjarmason
2010-09-30 13:43 ` [PATCH 16/16] send-email: use Perl idioms in while loop Ævar Arnfjörð Bjarmason
2010-09-30 14:30 ` [PATCH 00/16] git-send-email cleanups Brian Gernhardt
2010-09-30 14:52   ` Jeff King
2010-09-30 15:15     ` Brian Gernhardt
2010-09-30 15:11   ` Ævar Arnfjörð Bjarmason
2010-09-30 14:30 ` Jay Soffian
2010-09-30 16:21 ` Jeff King

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=AANLkTin_b6eSnHgY2YCOfN7E83gEzrOpktUD3heOnfQw@mail.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jaysoffian@gmail.com \
    --cc=peff@peff.net \
    --cc=rda@google.com \
    --cc=trast@student.ethz.ch \
    /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).