how can I "bounce" a message?

[Mike Kupfer]

[changed "cc" to the dev list]

I've tested this with and w/o identities, but have not
tested it with mh-redistribute-full-contents set (if you have nmh/MH
compiled with BERK). I just unit-tested the core of that code.

Hi Jeff, I took a look at your changes, and they generally look good to
me. I do have a few comments and questions.

1. The handling of the "Resent-From" field doesn't seem quite right.
If there's no identity set, or the identity doesn't specify a "From",
then

(mh-get-header-field "Resent-From":)

will be invoked. This will return "" or the value of an existing
Resent-From header. Either one seems problematic.

2. In mh-select-identity, why is "None" allowed when mh-identity-local is
non-null, but not when mh-identity-local is null?

Style nits (all in mh-comp.el):

- lines 692-693: the final 2 parentheses should not be on their own line

- indentation at lines 704-709 needs fixing

- "^Resent-From%": should the "%" be "$"?

- lines 716, 734: line needs to be wrapped (the GNU coding standards
say to avoid going past column 79)

- line 949: delete semicolon after "regular"

- lines 945-948: should there be two spaces after a period at the end of
a sentence? This was just discussed on emacs-devel, where Eli Z. said
that the GNU coding standards call for two spaces. But we have

;; sentence-end-double-space: nil

in the local variables block of all our source files, so I'm confused.

cheers,
mike

------------------------------------------------------------------------------

[Jeffrey Honig]

Post by Mike Kupfer
Hi Jeff, I took a look at your changes, and they generally look good to
me. I do have a few comments and questions.

Many thanks for the review.

Post by Mike Kupfer
1. The handling of the "Resent-From" field doesn't seem quite right.
If there's no identity set, or the identity doesn't specify a "From",
then
(mh-get-header-field "Resent-From":)
will be invoked. This will return "" or the value of an existing
Resent-From header. Either one seems problematic.

In the non-BERK case, we'll be reading the Resent-From header from the user
or system's distcomps. MH/nmh defines this in the default system file and
I rely on this.

With the BERK case we read this from the distcomps once I fix the typo
below.

Post by Mike Kupfer
2. In mh-select-identity, why is "None" allowed when mh-identity-local is
non-null, but not when mh-identity-local is null?

I'll look at this later and get back to you.

Post by Mike Kupfer
- lines 692-693: the final 2 parentheses should not be on their own line

Fixed.

Post by Mike Kupfer
- indentation at lines 704-709 needs fixing
Fixed
- "^Resent-From%": should the "%" be "$"?
Fixed
- lines 716, 734: line needs to be wrapped (the GNU coding standards
say to avoid going past column 79)

Is anyone still using an ADM-3A?

Fixed.

Post by Mike Kupfer
- line 949: delete semicolon after "regular"

Fixed

Post by Mike Kupfer
- lines 945-948: should there be two spaces after a period at the end of
a sentence? This was just discussed on emacs-devel, where Eli Z. said
that the GNU coding standards call for two spaces. But we have
;; sentence-end-double-space: nil
in the local variables block of all our source files, so I'm confused.

Loading Image...

I'll defer to Bill Wohler on matters of typographic style.

Thanks!

Jeff

[Mike Kupfer]

Post by Jeffrey Honig
In the non-BERK case, we'll be reading the Resent-From header from the user
or system's distcomps. MH/nmh defines this in the default system file and
I rely on this.

Ah, I see.

Post by Jeffrey Honig
With the BERK case we read this from the distcomps once I fix the typo
below.

It looks to me like you'd still get the Resent-From from the email, or
"" if the email had never been resent before. In

((string-match field "^Resent-From%")
(setq from (or from value)))))))

the original value of "from" will trump whatever you get from the
distcomps file.

Post by Jeffrey Honig
http://arnoldzwicky.s3.amazonaws.com/BloomPeriods.jpg

:-D

mike

------------------------------------------------------------------------------

[Bill Wohler]

Post by Mike Kupfer

Post by Jeffrey Honig
In the non-BERK case, we'll be reading the Resent-From header from the user
or system's distcomps. MH/nmh defines this in the default system file and
I rely on this.

Ah, I see.

Post by Jeffrey Honig
With the BERK case we read this from the distcomps once I fix the typo
below.

It looks to me like you'd still get the Resent-From from the email, or
"" if the email had never been resent before. In
((string-match field "^Resent-From%")
(setq from (or from value)))))))
the original value of "from" will trump whatever you get from the
distcomps file.

Post by Jeffrey Honig
http://arnoldzwicky.s3.amazonaws.com/BloomPeriods.jpg

:-D

I'm comfortable being a bossy liberal moron. Beats being a Neanderthal
:-). Someday, the GNU Emacs folks may come around.

[Jeffrey Honig]

Post by Mike Kupfer
It looks to me like you'd still get the Resent-From from the email, or
"" if the email had never been resent before. In
((string-match field "^Resent-From%")
(setq from (or from value)))))))
the original value of "from" will trump whatever you get from the
distcomps file.

Argh. I was trying to get by w/o rewriting the whole darn function to get
it right. I'll take another pass.

<insert whining about supporting the BERK option here>

Thanks

Jeff

[Jeffrey Honig]

Post by Jeffrey Honig
Argh. I was trying to get by w/o rewriting the whole darn function to get
it right. I'll take another pass.

Hacking away and thinking about this, I'm preferring code that reads the
distcomps file and processes each component as follows:

Resent-From - Use it if we do not have one from our identity
Resent-fcc - Use the version from the distcomps file
Resent-To, Resent-cc - Either prefer the version we entered, or use both.
Resent-.* - Use the version from the distcomps file
* - Ignored, not valid

The way nmh works is that if -to or -cc is specified on the command line,
it overrides the value in the distcomps file. Dist just displays it and
goes on to the next header. You can then edit the field from whatnow.
That is something we do not support.

So what to do with this corner case? I'm tempted to just include both if
there is a Resent-To or Resent-Cc in the distcomps file and the user
specified one.

We changed message composition to not take arguments, but to pop up the
draft and let the user enter the information (does it use pre-defined
fields from the components file?). Maybe that is the way we should go
long-term with mh-redistribute, but I'd rather fix this issue first.

Thanks

Jeff

[Jeffrey Honig]

Post by Mike Kupfer
2. In mh-select-identity, why is "None" allowed when mh-identity-local is
non-null, but not when mh-identity-local is null?

The answer to that is simple but not very helpful. I just copied the code
from mh-insert-identity.
So we would need to ask Bill Wohler why that's the case as git-blame points
to him.

However in mh-select-identity I always want None to be an option, so I'll
adjust the code accordingly and make sure I properly handle the None case.


Thanks

Jeff

[Bill Wohler]

Post by Mike Kupfer
2. In mh-select-identity, why is "None" allowed when mh-identity-local is
  non-null, but not when mh-identity-local is null?
The answer to that is simple but not very helpful.  I just copied the code from
mh-insert-identity.  
So we would need to ask Bill Wohler why that's the case as git-blame points to him.

We'd probably also have to ask Peter, who wrote this code originally.
Between the two of us, we might remember why...

Post by Mike Kupfer
However in mh-select-identity I always want None to be an option, so I'll adjust the
code accordingly and make sure I properly handle the None case.
Thanks
Jeff
--
http://www.honig.net/jch
GnuPG ID:14E29E13 <http://www.honig.net/jch/key.shtml>
----------------------------------------------------
----------------------------------------------------
------------------------------------------------------------------------------
_______________________________________________
mh-e-devel mailing list
https://lists.sourceforge.net/lists/listinfo/mh-e-devel

[Bill Wohler]

Yep, definitely. At least if it's in the Emacs Git repository, it will
end up in the next Emacs release.
Should we consider becoming an independent package that can be installed with the
package manager?  I'm not sure of all the tradeoffs, but we could push out updates
more frequently.

I now know what ELPA means.

It would be good to list the pros and cons. Since I'm unfamiliar with
the ELPA, the only thing I can think of is that this would take time,
and that's in short supply.

Is Gnus a package in the ELPA?

If we become a package instead of being a part of Emacs, would we move
our sources back to SourceForge from the Emacs repository?

[Mike Kupfer]

Post by Jeffrey Honig
Hacking away and thinking about this, I'm preferring code that reads the
Resent-From - Use it if we do not have one from our identity
Resent-fcc - Use the version from the distcomps file

[...]

Post by Jeffrey Honig
Resent-.* - Use the version from the distcomps file
* - Ignored, not valid

Those 4 sound fine to me.

Post by Jeffrey Honig
Resent-To, Resent-cc - Either prefer the version we entered, or use both.

[...]

Post by Jeffrey Honig
The way nmh works is that if -to or -cc is specified on the command line,
it overrides the value in the distcomps file. Dist just displays it and
goes on to the next header. You can then edit the field from whatnow.
That is something we do not support.
So what to do with this corner case? I'm tempted to just include both if
there is a Resent-To or Resent-Cc in the distcomps file and the user
specified one.

Hmm. With nmh 1.6, the default distcomps uses mh-format(5) constructs
to embed the -to or -cc arguments into the headers, e.g.,

%<{nmh-to}%(void(width))%(putaddr Resent-To: )%|Resent-To:%>

After processing by mh-bare-components, these will end up as
null-value headers, e.g.,

Resent-To:

I think mh-redistribute should replace those with the user-specified
values.

I haven't played around with dist(1) to see what happens if the user is
using a non-default distcomps file. We can't easily guarantee that MH-E
will produce exactly the same results as invoking dist(1) from the
command line, so just aiming for reasonable behavior sounds fine to me.

Combining the user-specified -to and -cc values with whatever's in the
distcomp file sounds okay to me.

I've verified that send(1) can handle multiple Resent-To lines, so you
wouldn't need to combine the values into a single Resent-mumble line.
Or at least that's the case for nmh 1.6. I didn't test GNU mailutils or
older versions of nmh.

Post by Jeffrey Honig
We changed message composition to not take arguments, but to pop up the
draft and let the user enter the information (does it use pre-defined
fields from the components file?).

Yes, the components file gets processed (via mh-bare-components), and
the user is given the processed file to edit.

Post by Jeffrey Honig
Maybe that is the way we should go
long-term with mh-redistribute, but I'd rather fix this issue first.

That does sound cleaner, though I've run into at least one gotcha. I
agree that it makes sense to defer that work.

The gotcha that I ran into is that the formatting language that is used
for nmh component files includes command-specific functions, and there
are inconsistencies between the different commands. For example, with
dist(1), {nmh-to} returns the -to address. But with comp(1), you'd use
{to}. We could ask the nmh developers for better consistency, but that
doesn't solve the problem for older nmh releases.

mike

------------------------------------------------------------------------------

[Jeffrey Honig]

I'm at the movies now, but I'm pretty sure that mh-bare-components or
mh-components-to-list? calls mh-format. I think I verified that last week
by looking at the code. I have observed proper behaviour from a custom
components file that uses the new rules.

mh-insert-header does not support adding additional headers, so my current
code parses the components and does the merging.

Thanks

Jeff

Post by Mike Kupfer

Post by Jeffrey Honig
Hacking away and thinking about this, I'm preferring code that reads the
Resent-From - Use it if we do not have one from our identity
Resent-fcc - Use the version from the distcomps file

[...]

Post by Jeffrey Honig
Resent-.* - Use the version from the distcomps file
* - Ignored, not valid

Those 4 sound fine to me.

Post by Jeffrey Honig
Resent-To, Resent-cc - Either prefer the version we entered, or use both.

[...]

Post by Jeffrey Honig
The way nmh works is that if -to or -cc is specified on the command line,
it overrides the value in the distcomps file. Dist just displays it and
goes on to the next header. You can then edit the field from whatnow.
That is something we do not support.
So what to do with this corner case? I'm tempted to just include both if
there is a Resent-To or Resent-Cc in the distcomps file and the user
specified one.

Hmm. With nmh 1.6, the default distcomps uses mh-format(5) constructs
to embed the -to or -cc arguments into the headers, e.g.,
%<{nmh-to}%(void(width))%(putaddr Resent-To: )%|Resent-To:%>
After processing by mh-bare-components, these will end up as
null-value headers, e.g.,
I think mh-redistribute should replace those with the user-specified
values.
I haven't played around with dist(1) to see what happens if the user is
using a non-default distcomps file. We can't easily guarantee that MH-E
will produce exactly the same results as invoking dist(1) from the
command line, so just aiming for reasonable behavior sounds fine to me.
Combining the user-specified -to and -cc values with whatever's in the
distcomp file sounds okay to me.
I've verified that send(1) can handle multiple Resent-To lines, so you
wouldn't need to combine the values into a single Resent-mumble line.
Or at least that's the case for nmh 1.6. I didn't test GNU mailutils or
older versions of nmh.

Post by Jeffrey Honig
We changed message composition to not take arguments, but to pop up the
draft and let the user enter the information (does it use pre-defined
fields from the components file?).

Yes, the components file gets processed (via mh-bare-components), and
the user is given the processed file to edit.

Post by Jeffrey Honig
Maybe that is the way we should go
long-term with mh-redistribute, but I'd rather fix this issue first.

That does sound cleaner, though I've run into at least one gotcha. I
agree that it makes sense to defer that work.
The gotcha that I ran into is that the formatting language that is used
for nmh component files includes command-specific functions, and there
are inconsistencies between the different commands. For example, with
dist(1), {nmh-to} returns the -to address. But with comp(1), you'd use
{to}. We could ask the nmh developers for better consistency, but that
doesn't solve the problem for older nmh releases.
mike

[Mike Kupfer]

Post by Jeffrey Honig
I'm at the movies now, but I'm pretty sure that mh-bare-components or
mh-components-to-list? calls mh-format.

mh-bare-components does the formatting by calling comp(1). IIUC,
there's no separate CLI for invoking the formatter. It has to be done
by invoking one of the other commands (comp, forw, dist).

Post by Jeffrey Honig
I have observed proper behaviour from a custom
components file that uses the new rules.

Come to think of it, send(1) might ignore Resent-foo lines that don't
include an address. As long as it works okay, I guess I won't worry
much about the details. :-)

mike

------------------------------------------------------------------------------

[Jeffrey Honig]

Mike,

I attached the latest version of my patch to
https://sourceforge.net/p/mh-e/bugs/468/ would you like to review it there,
or continue to provide comments in this thread.

Sorry for the delay, too many fun and not so fun things going on.

Thanks

Jeff
--
Jeffrey C. Honig <***@honig.net>
http://www.honig.net/jch
GnuPG ID:14E29E13 <http://www.honig.net/jch/key.shtml>

Post by Mike Kupfer

Post by Jeffrey Honig
I'm at the movies now, but I'm pretty sure that mh-bare-components or
mh-components-to-list? calls mh-format.

mh-bare-components does the formatting by calling comp(1). IIUC,
there's no separate CLI for invoking the formatter. It has to be done
by invoking one of the other commands (comp, forw, dist).

Post by Jeffrey Honig
I have observed proper behaviour from a custom
components file that uses the new rules.

Come to think of it, send(1) might ignore Resent-foo lines that don't
include an address. As long as it works okay, I guess I won't worry
much about the details. :-)
mike

[Mike Kupfer]

Post by Jeffrey Honig
I attached the latest version of my patch to
https://sourceforge.net/p/mh-e/bugs/468/ would you like to review it there,
or continue to provide comments in this thread.

Hi Jeff, thanks for the update. I got started reviewing but won't have
time to finish tonight. I'll try to finish over the weekend, and I'll
reply on this thread.

Post by Jeffrey Honig
Sorry for the delay, too many fun and not so fun things going on.

Sorry to hear about the not-so-fun things. No worries about the
delay.

cheers,
mike

------------------------------------------------------------------------------

[Mike Kupfer]

Post by Jeffrey Honig
I attached the latest version of my patch to
https://sourceforge.net/p/mh-e/bugs/468/ would you like to review it there,
or continue to provide comments in this thread.

I have two concerns, but otherwise it looks good to me.

1. In the call to mh-clean-msg-header, Message-Id has an "^" anchor,
but none of the other fields do. Shouldn't they all have "^"?

2. It doesn't looks like multiple instances of fields in the
distcomps file are handled correctly. This is something of an edge
case, so if you want to defer this to another bug, that's fine with
me.

Here's a more detailed description of the problem. Suppose the
distcomps file has 2 Resent-fcc lines, one for folderA and one for
folderB. If I invoke dist from the shell, I get 2 copies of the
message, one in each folder. But if I redistribute the message using
MH-E, mh-insert-fields will convert this to a single line

Resent-fcc: +folderA +folderB

and I get 1 copy in the (new) folder named "folderA +folderB".

There's a similar issue with Resent-To and Resent-cc. If I have 2
Resent-To lines in distcomps and invoke dist from the shell, both
addresses get a copy. With MH-E, it looks like the 1st Resent-To
address will be ignored (overwritten by the 2nd address).

cheers,
mike

[Jeffrey Honig]

Post by Mike Kupfer
1. In the call to mh-clean-msg-header, Message-Id has an "^" anchor,
but none of the other fields do. Shouldn't they all have "^"

Good catch, that's a bug.
2. It doesn't looks like multiple instances of fields in the

Post by Mike Kupfer
distcomps file are handled correctly. This is something of an edge
case, so if you want to defer this to another bug, that's fine with
me.
Here's a more detailed description of the problem. Suppose the
distcomps file has 2 Resent-fcc lines, one for folderA and one for
folderB. If I invoke dist from the shell, I get 2 copies of the
message, one in each folder. But if I redistribute the message using
MH-E, mh-insert-fields will convert this to a single line
Resent-fcc: +folderA +folderB
and I get 1 copy in the (new) folder named "folderA +folderB".
There's a similar issue with Resent-To and Resent-cc. If I have 2
Resent-To lines in distcomps and invoke dist from the shell, both
addresses get a copy. With MH-E, it looks like the 1st Resent-To
address will be ignored (overwritten by the 2nd address).

You pointed this out before and I did not fully absorb what you said.

This is an easy fix, except for From:. If the distcomps has one or more
From: lines and we have an identify configured, what do we want to do?
Have the identity override the From: line(s)? Merge them?

I would say that identity trumps most valid reasons for multiple From:
lines. Thoughts?

Thanks

Jeff

[Mike Kupfer]

Post by Jeffrey Honig
lines. Thoughts?

Agreed. I don't even understand what it would mean to have multiple
From: lines. If the user wants to use the From line from distcomps, she
can specify identity "None".

cheers,
mike