From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: Git Mailing List <git@vger•kernel.org>,
Dan Carpenter <dan.carpenter@oracle•com>,
Mark Einon <mark.einon@gmail•com>,
Greg KH <gregkh@linuxfoundation•org>
Subject: Re: [RFC/PATCH] mailinfo: do not treat ">From" lines as in-body headers
Date: Mon, 15 Sep 2014 13:15:35 -0700 [thread overview]
Message-ID: <xmqq1trc63o8.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqha087lwv.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Mon, 15 Sep 2014 11:56:16 -0700")
Junio C Hamano <gitster@pobox•com> writes:
> Why cache.h when this is still only between mail{info,split}.c both
> of which do not really deal with any "Git" data?
>
> For mailsplit, we are trying to detect mbox boundary various MUAs
> would use in their output, and is_from_line() may be appropriate,
> but I am not sure if the same logic is appropriate for mailinfo.
> What are we trying to protect us against? Those who paste a single
> e-mail output from format-patch in full? Do people paste a single
> e-mail received to their mailbox from somebody else and do we need
> to protect against that, skipping the ">From " thing, while risking
> to skip "From me at 10:30 30 minutes..."?
>
> If we only want to skip ">?From" in pasted format-patch output, we
> would want a rule in mailinfo that is tighter than is_from_line() in
> mailsplit.
That is, something like this on top of your patch. Or is this a bit
too strict?
Makefile | 1 +
builtin/mailinfo.c | 3 ++-
builtin/mailsplit.c | 1 +
cache.h | 6 ------
mbox.c | 15 +++++++++++++++
5 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile
index dc5d2af..c0491a3 100644
--- a/Makefile
+++ b/Makefile
@@ -686,6 +686,7 @@ LIB_H += list-objects.h
LIB_H += ll-merge.h
LIB_H += log-tree.h
LIB_H += mailmap.h
+LIB_H += mbox.h
LIB_H += merge-blobs.h
LIB_H += merge-recursive.h
LIB_H += mergesort.h
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index a434d39..ccccd69 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -6,6 +6,7 @@
#include "builtin.h"
#include "utf8.h"
#include "strbuf.h"
+#include "mbox.h"
static FILE *cmitmsg, *patchfile, *fin, *fout;
@@ -329,7 +330,7 @@ static int check_header(const struct strbuf *line,
/* for inbody stuff */
if (starts_with(line->buf, ">From") && isspace(line->buf[5])) {
- ret = is_from_line(line->buf + 1, line->len - 1);
+ ret = is_format_patch_separator(line->buf + 1, line->len - 1);
goto check_header_out;
}
if (starts_with(line->buf, "[PATCH]") && isspace(line->buf[7])) {
diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 775588e..d8da1e4 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -8,6 +8,7 @@
#include "builtin.h"
#include "string-list.h"
#include "strbuf.h"
+#include "mbox.h"
static const char git_mailsplit_usage[] =
"git mailsplit [-d<prec>] [-f<n>] [-b] [--keep-cr] -o<directory> [(<mbox>|<Maildir>)...]";
diff --git a/cache.h b/cache.h
index eae58e7..fcb511d 100644
--- a/cache.h
+++ b/cache.h
@@ -1502,10 +1502,4 @@ void stat_validity_update(struct stat_validity *sv, int fd);
int versioncmp(const char *s1, const char *s2);
-/*
- * Returns true if the line appears to be an mbox "From" line starting a new
- * message.
- */
-int is_from_line(const char *line, int len);
-
#endif /* CACHE_H */
diff --git a/mbox.c b/mbox.c
index 75f3150..2ab2f85 100644
--- a/mbox.c
+++ b/mbox.c
@@ -30,3 +30,18 @@ int is_from_line(const char *line, int len)
/* Ok, close enough */
return 1;
}
+
+#define SAMPLE "From e6807f3efca28b30decfecb1732a56c7db1137ee Mon Sep 17 00:00:00 2001\n"
+int is_format_patch_separator(const char *line, int len)
+{
+ const char *cp;
+
+ if (len != strlen(SAMPLE))
+ return 0;
+ if (!skip_prefix(line, "From ", &cp))
+ return 0;
+ if (strspn(cp, "0123456789abcdef") != 40)
+ return 0;
+ cp += 40;
+ return !memcmp(SAMPLE + (cp - line), cp, strlen(SAMPLE) - (cp - line));
+}
next prev parent reply other threads:[~2014-09-15 20:15 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1410472786-14552-1-git-send-email-mark.einon@gmail.com>
[not found] ` <1410472786-14552-5-git-send-email-mark.einon@gmail.com>
2014-09-13 9:37 ` [PATCH 4/8] staging: et131x: Remove ununsed statistics Dan Carpenter
2014-09-13 15:45 ` Greg KH
2014-09-13 19:41 ` Dan Carpenter
2014-09-13 20:36 ` Jeff King
2014-09-13 20:47 ` Mark Einon
2014-09-13 20:57 ` Dan Carpenter
2014-09-13 21:06 ` Mark Einon
2014-09-13 21:09 ` Dan Carpenter
2014-09-13 21:25 ` [RFC/PATCH] mailinfo: do not treat ">From" lines as in-body headers Jeff King
2014-09-13 22:57 ` brian m. carlson
2014-09-14 0:47 ` Jeff King
2014-09-14 0:55 ` Junio C Hamano
2014-09-14 1:01 ` Jeff King
2014-09-14 1:30 ` Jeff King
2014-09-15 18:56 ` Junio C Hamano
2014-09-15 20:15 ` Junio C Hamano [this message]
2014-09-16 0:19 ` Jeff King
2014-09-16 18:01 ` Junio C Hamano
2014-09-16 18:41 ` Junio C Hamano
2014-09-16 20:29 ` Jeff King
2014-09-16 0:12 ` Jeff King
2014-09-15 17:55 ` 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=xmqq1trc63o8.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=dan.carpenter@oracle$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gregkh@linuxfoundation$(echo .)org \
--cc=mark.einon@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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