All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Wong <e@80x24.org>
To: Samuel GROOT <samuel.groot@grenoble-inp.org>
Cc: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>,
	git@vger.kernel.org, erwan.mathoniere@grenoble-inp.org,
	jordan.de-gea@grenoble-inp.org, gitster@pobox.com,
	aaron@schrab.com, Tom RUSSELLO <tom.russello@grenoble-inp.org>,
	Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: [WIP-PATCH 1/2] send-email: create email parser subroutine
Date: Thu, 2 Jun 2016 19:58:55 +0000	[thread overview]
Message-ID: <20160602195855.GA17627@dcvr.yhbt.net> (raw)
In-Reply-To: <9535d962-5479-5a13-472e-cd558ef163e0@grenoble-inp.org>

Samuel GROOT <samuel.groot@grenoble-inp.org> wrote:
> On 05/29/2016 01:33 AM, Eric Wong wrote:
> >Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> >>Samuel GROOT <samuel.groot@grenoble-inp.org> writes:
> >>
> >>>Parsing and processing in send-email is done in the same loop.
> >>>
> >>>To make the code more maintainable, we create two subroutines:
> >>>- `parse_email` to separate header and body
> >>>- `parse_header` to retrieve data from header
> >>
> >>These routines are not specific to git send-email, nor to Git.
> >>
> >>Does it make sense to use an external library, like
> >>http://search.cpan.org/~rjbs/Email-Simple-2.210/lib/Email/Simple.pm ,
> >>either by depending on it, or by copying it in Git's source tree ?
> >
> >That might be overkill and increase installation/maintenance
> >burden.  Bundling it would probably be problematic to distros,
> >too.
> 
> We have 5 solutions here:
> 
>   1. Make a new dependence to Email::Simple.
> 
>   2. Bundle Email::Simple in Git's source tree.
> 
>   3. Use Email::Simple if installed, else use our library.
> 
>   4. Making our own email parser library.
> 
>   5. Duplicate parser loop as we did for our patch to implement
>      `--quote-email` as proposed in $gmane/295772 .
> 
> Obviously, option (5) is the easiest one for us, but it leaves refactoring
> for later, and option (1) is also easier but adds a new dependence which is
> not that good.

I would go with (5) for now and leave (4) for later (which
might just be moving the function to a new file).

> Since our project ends next week, we might not have enough time to finish
> developing a custom parser API so (4) is not a viable option for now but
> could be done in the future.
> 
> We could consider bundling Email::Simple as the best option, as it's
> developed since 2003 and might be safer to use than anything we could write
> in several weeks.

In an ideal world, (1) would be nice.  But (IMHO) git-send-email
should remain installable on non-ideal systems which do not
provide Email::Simple as a package.

(2) would probably be non-ideal for distro maintainers
(+Cc: Jonathan for opinions), and (3) is the most complex
and difficult-to-support.

  reply	other threads:[~2016-06-02 19:58 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-27 14:01 [WIP-PATCH 0/2] send-email: refactor the email parser loop Samuel GROOT
2016-05-27 14:01 ` [WIP-PATCH 1/2] send-email: create email parser subroutine Samuel GROOT
2016-05-28 15:22   ` Matthieu Moy
2016-05-28 23:33     ` Eric Wong
2016-05-29 17:15       ` Samuel GROOT
2016-05-29 17:53         ` Matthieu Moy
2016-05-30 13:28           ` Samuel GROOT
2016-06-02 16:57       ` Samuel GROOT
2016-06-02 19:58         ` Eric Wong [this message]
2016-05-27 14:01 ` [WIP-PATCH 2/2] send-email: use refactored subroutine to parse patches Samuel GROOT
2016-05-27 20:14 ` [WIP-PATCH 0/2] send-email: refactor the email parser loop Eric Wong
2016-05-28 15:04   ` Matthieu Moy
2016-05-29 17:21     ` Samuel GROOT
2016-05-29 18:05       ` Matthieu Moy
2016-05-30 14:01         ` Samuel GROOT
2016-05-30 14:20           ` Matthieu Moy
2016-05-30 18:28             ` Samuel GROOT
2016-05-30 19:29               ` Matthieu Moy

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=20160602195855.GA17627@dcvr.yhbt.net \
    --to=e@80x24.org \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=aaron@schrab.com \
    --cc=erwan.mathoniere@grenoble-inp.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jordan.de-gea@grenoble-inp.org \
    --cc=jrnieder@gmail.com \
    --cc=samuel.groot@grenoble-inp.org \
    --cc=tom.russello@grenoble-inp.org \
    /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.