On Fri, Feb 17, 2017 at 05:58:11PM +0100, Matthieu Moy wrote:
> > On Fri, Feb 17, 2017 at 02:16:42PM +0100, Matthieu Moy wrote:
> >> Johan Hovold <johan@xxxxxxxxxx> writes:
> >
> >> The "multiple emails per Cc: field" has been there for a while already
> >> (b1c8a11c8024 released in 2.6.0, sept 2015), some users may have got
> >> used to it. What you are proposing breaks their flow.
> >
> > Note that that commit never mentions multiple addresses in either
> > headers or body-tags -- it's all about being able to specify multiple
> > entries on the command line.
>
> Indeed. I'm not the author of the patch, but I was supervising the
> students who wrote it and "multiple addresses in Cc:" was not the goal,
> but a (IMHO positive) side effect we discovered after the fact.
Yeah, and the broken --suppress-cc=self I mention below is indicative
of that too.
> If I had a time machine, I'd probably go back then and forbid multiple
> addresses there, but ...
>
> > There does not seem to be single commit in the kernel where multiple
> > address are specified in a CC tag since after git-send-email started
> > allowing it, but there are ten commits before (to my surprise), and that
> > should be contrasted with at least 4178 commits with trailing comments
> > including a # sign.
>
> Hey, there's a life outside the kernel ;-).
Sure, but it's the origin of git as well as the tags we're discussing (I
believe).
My point of bringing it up was that the multiple addresses in a CC-tag
was indeed an unintended (and undocumented) side-effect and I doubt many
people have started using it given that it's sort of counter-intuitive
(again, compare with SoB).
If either the trailing comments or multiple addresses in a CC-tag has to
go, I think dropping the latter is clearly the best choice.
> >> 1) Stop calling Mail::Address even if available.[...]
> >
> > Right, that sounds like the right thing to do regardless.
> >
> >> 2) Modify our in-house parser to discard garbage after the >. [...]
> >
> > Sounds perfectly fine to me, and seems to work too after quick test.
>
> OK, sounds like the way to go.
>
> Do you want to work on a patch? If not, I should be able to do that
> myself. The code changes are straightforward, but we probably want a
> proper test for that.
Feel free to implement it this way if that's what people prefer. As long
as trailing comments are supported and discarded, I don't really have a
preference.
> > addresses in a Cc-tag in that it breaks --suppress-cc=self, but I guess
> > that can be fixed separately.
>
> OK. If it's unrelated enough, please start a separate thread to explain
> the problem (and/or write a patch ;-) ).
Well, it's related to the "offending" patch that added support for
multiple addresses in tags. By disallowing that, as my fix does, the
problem goes away.
# Now parse the message body
while(<$fh>) {
$message .= $_;
if (/^(Signed-off-by|Cc): (.*)$/i) {
chomp;
my ($what, $c) = ($1, $2);
chomp $c;
my $sc = sanitize_address($c);
if ($sc eq $sender) {
next if ($suppress_cc{'self'});
The problem here is that $sc will never match $sender when there are more
than one address in a tag. For example:
From: Johan Hovold <johan@xxxxxxxxxx>
...
Cc: alpha <test1@xxxxx>, Johan Hovold <johan@xxxxxxxxxx>
results in
sc = alpha <test1@xxxxx>, Johan Hovold <johan@xxxxxxxxxx>
sender = Johan Hovold <johan@xxxxxxxxxx>
so that --suppress-cc=self is not honoured.
Thanks,
Johan