All of lore.kernel.org
 help / color / mirror / Atom feed
From: Samuel GROOT <samuel.groot@grenoble-inp.org>
To: git@vger.kernel.org
Cc: e@80x24.org, erwan.mathoniere@grenoble-inp.org,
	jordan.de-gea@grenoble-inp.org, matthieu.moy@grenoble-inp.fr,
	gitster@pobox.com, aaron@schrab.com,
	"Samuel GROOT # This line is ignored ." 
	<samuel.groot@grenoble-inp.org>
Subject: [WIP-PATCH 0/2] send-email: refactor the email parser loop
Date: Fri, 27 May 2016 16:01:02 +0200	[thread overview]
Message-ID: <20160527140104.11192-1-samuel.groot@grenoble-inp.org> (raw)

While working on the new option `quote-email`, we needed to parse an
email file. Since the work is already done, but the parsing and data
processing are in the same loop, we wanted to refactor the parser, to
make the code more maintainable.

This is still WIP, and one of the main issue (and we need your
advice on that), is that around 30 tests will fail, and most of them
are false-negatives: to pass, they rely on the format of what is
displayed by `git send-email`, not only data.


For example, several tests will fail because they do a strict compare
between `git send-email`'s output and:

   (mbox) Adding cc: A<author@example.com>  from line 'Cc: A<author@example.com>, One<one@example.com>'
   (mbox) Adding cc: One<one@example.com>  from line 'Cc: A<author@example.com>, One<one@example.com>'

Though `git send-email` now outputs something like:

   (mbox) Adding cc: A<author@example.com>  from line 'Cc: A<author@example.com>'
   (mbox) Adding cc: One<one@example.com>  from line 'Cc: One<one@example.com>'

The new behavior is explained because parsing and processing are not
done in the same function, and processing cannot know the exact line
the data came from.

We can think of several solutions:

   1. Modify the parser, to give the script the knowledge of the exact
      line the data came from.

   2. Hack the tests: modify the script using the parser to artificially
      recreate the supposedly parsed line.
      (e.g. with a list.join(', ')-like function)

   3. Modify the tests to replace exact cmp by greps.


IMHO, we should consider option 3, since the tests should rely on data
rather than exact outputs. It also makes the tests more maintainable,
in the sense that they will be more resilient to future output
modifications.

What do you think?


   [WIP-PATCH 1/2] send-email: create email parser subroutine
   [WIP-PATCH 2/2] send-email: use refactored subroutine to parse patches

git-send-email.perl | 284 ++++++++++++++++++++++++++++++++++++++++++++--------------------------------
  1 file changed, 164 insertions(+), 120 deletions(-)

             reply	other threads:[~2016-05-27 14:01 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-27 14:01 Samuel GROOT [this message]
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
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=20160527140104.11192-1-samuel.groot@grenoble-inp.org \
    --to=samuel.groot@grenoble-inp.org \
    --cc=aaron@schrab.com \
    --cc=e@80x24.org \
    --cc=erwan.mathoniere@grenoble-inp.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jordan.de-gea@grenoble-inp.org \
    --cc=matthieu.moy@grenoble-inp.fr \
    /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.