Re: [nmh-workers] nmh 1.7.1: both bcc and dcc broken for mts sendmail/pipe

[Ralph Corderoy]

Hi az,

Thanks for the patch, Ken knows the topic better than me, but the man
page caught my eye.

>  If a \*(lqBcc:\*(rq field is encountered, its addresses will be used for
>  delivery, and the \*(lqBcc:\*(rq field will be removed from the message
> -sent to sighted recipients.  The blind recipients will receive an entirely
> -new message with a minimal set of headers.  Included in the body of the
> -message will be a copy of the message sent to the sighted recipients.
> +sent to sighted recipients. The blind recipients will receive an entirely
> +new message with a minimal set of headers. The body of this new message
> +will contain a copy of the message sent to the sighted recipients, either
> +marked up with the indicator text "Blind-Carbon-Copy" or encapsulated
> +as a MIME digest.

A man page is troff source and so the `text' parts aren't free-format
but instructions to the typesetter.  In particular, the end of a
sentence must be followed by two spaces to indicate to the formatter
that it is indeed the end of a sentence and not a word following an
abbreviation or something else that happens to end in a full stop.
This is independent of any preference for whether the post-sentence gap
should be wider than an inter-word one.

Because of this, it's often easier to end the line at the end of a
sentence.  This also makes diffs simpler because they tend to stay
within a sentence and not ripple through a re-formatted paragraph.
For a similar reason, long lines are sometimes broken after a comma.

The «"» around `Blind-Carbon-Copy' should be \(lq and \(rq, or the
equivalent strings for consistency with the style used at start of the
paragraph.

> +recipients. *WARNING* Recipients listed in the \*(lqDcc:\*(rq field receive 
> no

I'd suggest «.B Warning:» rather than ASCII mark-up and shouting.  :-)
Though, reading on, I realise that you copied how it was already done
elsewhere.

Given I'm writing about the patch anyway,

> -    fprintf (out, "BCC:\n");
> +
> +    /* for sendmail/pipe, insert all bcc recipients here so that the email 
> can be routed based on the bcc: header */
> +    if (sm_mts == MTS_SENDMAIL_PIPE)
> +    {
> +       char *allbcc = NULL;
> +       struct mailname *lp;
> +
> +       for (lp = localaddrs.m_next; lp; lp = lp->m_next)
> +       if (lp->m_bcc)
> +          allbcc = allbcc? add(concat(", ", lp->m_mbox, NULL), allbcc)
> +             : mh_xstrdup(lp->m_mbox);
> +       for (lp = netaddrs.m_next; lp; lp = lp->m_next)
> +       if (lp->m_bcc)
> +          allbcc = allbcc? add(
> +             concat(", ", lp->m_mbox, "@", lp->m_host, NULL),
> +             allbcc)
> +             : concat(lp->m_mbox, "@", lp->m_host, NULL);
> +       if (allbcc)
> +       {
> +       fprintf (out, "BCC: %s\n",allbcc);
> +       free(allbcc);
> +       }
> +    }
> +    else
> +    {
> +       fprintf (out, "BCC:\n");
> +    }

I'd be tempted to make it an if-then with no else clause by hoisting the
"BCC:" prefix and "\n" suffix outside of the if-then.

And it tends to simplify the code when having to join items with a
separator like ", " if it always starts every item, giving ", foo, bar",
and then if allbcc isn't NULL then `allbcc + 2' can be printed.  (It can
be "foo, bar, " instead but then the end of the string needs finding to
shorten it by two.)  I think this would remove the ternary operators and
duplicated terms like `lp->m_mbox, "@", lp->m_host, NULL' so the reader
has less to parse and check they're the same.

-- 
Cheers, Ralph.