Re: [PATCH 2/2] mailinfo: Understand forwarded patches

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Matthew Wilcox <mawilcox@xxxxxxxxxxxxxxxxx> writes:

> From: Matthew Wilcox <mawilcox@xxxxxxxxxxxxx>
>
> Extend the --scissors mechanism to strip off the preamble created by
> forwarding a patch.  There are a couple of extra headers ("Sent" and
> "To") added by forwarding, but other than that, the --scissors option
> will now remove patches forwarded from Microsoft Outlook to a Linux
> email account.
>
> Signed-off-by: Matthew Wilcox <mawilcox@xxxxxxxxxxxxx>
> ---
>  mailinfo.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

To be quite honest, I am not enthused to even having to think about
this kind of change.  There seems to be no standard way MUAs produce
"forwarded" messages, and this adds support for only one specific
MUA, that happens to say "Original Message".  Why should such a thing
hardcoded in this codepath?

I think I am OK with a patch that lets users customize
is_scissors_line(), perhaps accepting a regexp that ought to match a
line to consider a scissors line.  When such a regexp is given,
check for a match before doing the "do we have a line filled with
'-' with the scissors marker and is the mark long enough?" check.

Then you can do

	mailinfo --scissors='^-{5}Original Message-{5}$'

or something like that.  Perhaps allow multiple regexps, or even
allow them to come from a multi-valued configuration variable.

> diff --git a/mailinfo.c b/mailinfo.c
> index 2059704a8..fc1275532 100644
> --- a/mailinfo.c
> +++ b/mailinfo.c
> @@ -332,7 +332,7 @@ static void cleanup_subject(struct mailinfo *mi, struct strbuf *subject)
>  
>  #define MAX_HDR_PARSED 10
>  static const char *header[MAX_HDR_PARSED] = {
> -	"From","Subject","Date",
> +	"From","Subject","Date","Sent","To",
>  };

This array lists fields whose value we _care_ about.  Do not put
random garbage whose value we do not use in it.

Even though I am not enthused to see support for messages forwarded
by Outlook bolted onto this codepath, I think it may make sense to
allow random garbage that looks like an e-mail header to appear
immediately after a scissors line (and ignore them).  For that,
perhaps you would instead want to extend is_inbody_header() so that
after it decides that the given line is *NOT* one of the header
field we care about, return a value that is not 0 or 1.  Its caller
currently expects to see 1 if it is a relevant in-body header line,
or 0 if the line signals the end of the in-body header block.  You'd
be adding another class of lines that are not a header line with a
meaning but do not terminate the in-body header block.


>  static inline int cmp_header(const struct strbuf *line, const char *hdr)
> @@ -685,6 +685,13 @@ static int is_scissors_line(const char *line)
>  			c++;
>  			continue;
>  		}
> +		if (!memcmp(c, "Original Message", 16)) {
> +			in_perforation = 1;
> +			perforation += 16;
> +			scissors += 16;
> +			c += 15;
> +			continue;
> +		}
>  		in_perforation = 0;
>  	}



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]