All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "Роман Донченко" <dpb@corrigendum.ru>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Jay Soffian" <jaysoffian@gmail.com>,
	"Thomas Rast" <tr@thomasrast.ch>
Subject: Re: [PATCH] send-email: handle adjacent RFC 2047-encoded words properly
Date: Mon, 24 Nov 2014 18:03:47 -0500	[thread overview]
Message-ID: <20141124230346.GA10064@peff.net> (raw)
In-Reply-To: <op.xpudh8c3nngjn5@freezie>

On Mon, Nov 24, 2014 at 09:26:22PM +0300, Роман Донченко wrote:

> Yeah, I did realize that token is more restrictive than encoded-text, but I
> didn't want to stray too far from the subject line of the patch. What I'll
> probably do is split the patch into two, one for regex tweaking and one for
> multiple-word handling. And yeah, I'll try to make the two functions use the
> same regexes.

Thanks, I think that sounds like a good plan.

> >For your definition of encoded-text, RFC 2047 says:
> >
> >     encoded-text = 1*<Any printable ASCII character other than "?"
> >                          or SPACE>
> >
> >It looks like you pulled the definition of $et from is_rfc2047_quoted,
> >but I am not clear on where that original came from (it is from a3a8262,
> >but that commit message does not explain the regex).
> 
> No, it's actually an independent discovery. :-) I don't think it needs
> explanation, though - it's just a character class with two ranges covering
> every printable character but the question mark.

And now it is my turn to hang my head in shame. I viewed that as a set
of characters, rather than ranges. The "-" just blended into the mass of
punctuation, and I mistook the "!" for negation.

I wonder if it would be more readable as:

  [\x21-\x3e\x40-\x7e]

or something. I guess perl even has classes pre-made for "printable
ascii". I dunno. It may be OK as-is, too, and I just need to read more
carefully. :)

> >Also, I note that we handle 'q'-style encodings here, but not 'b'. I
> >wonder if it is worth adding that in while we are in the area (it is not
> >a big deal if you always send-email git-generated patches, as we never
> >generate it).
> 
> I could add "b" decoding, but since format-patch never generates "b"
> encodings, testing would be a problem. And I'd rather not do it without any
> tests.

I think you could include some literal fixtures in the test suite (t5100
already does this for mailinfo). But I don't think handling 'b' is a
requirement here. It's really orthogonal to your patches, and nobody has
actually asked for it, so I don't mind leaving it for another day.

-Peff

      reply	other threads:[~2014-11-24 23:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-23 23:50 [PATCH] send-email: handle adjacent RFC 2047-encoded words properly Роман Донченко
2014-11-24  7:27 ` Junio C Hamano
2014-11-24 15:44   ` Jeff King
2014-11-24 18:09   ` Роман Донченко
2014-11-24 15:36 ` Jeff King
2014-11-24 18:26   ` Роман Донченко
2014-11-24 23:03     ` Jeff King [this message]

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=20141124230346.GA10064@peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=dpb@corrigendum.ru \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jaysoffian@gmail.com \
    --cc=tr@thomasrast.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.