🚀 go-pugleaf

On 2010-01-04, blueman <NOSPAM@nospam.com> wrote:
> HOWEVER, vm-pipe-message-part is skipping the initial 'From' line since
> it is using (vm-headers-of m) as the start rather than (vm-start-of m).
> Similarly, it uses (vm-text-end-of m) rather than (vm-end-of m)
>
> Since the usual reason for splitting/piping messages to an external
> shell command is to pass the message to some standard 3rd program, it
> would be helpful to use standard conventions -- by which I mean
> something relatively consistent with 'formail' or better yet the RFC's.
> In particular, when you ask for the "entire message", I at least expect
> the entire message.

VM is behaving correctly. The initial From line is NOT part of the RFC
standard, nor is it part of the message. It's an obsolete Unix mbox hack
that should have been killed off decades ago - the mbox format is
patently unsuitable, leading to abominations like From-stuffing.
I've had immense trouble with very broken mail systems (procmail) that
insist on putting the From in where it has no business - it caused
standards-compliant mail filters to break, because they saw a From
instead of a proper message.

> In my case, spamassassin thinks a collection of messages is just one big
> message since it doesn't see the 'From' line.
>
> I would suggest making the following change so that the default "entire
> message" truly includes the entire message and perhaps add an option
> with four universal arguments that gives the current default behavior.

Bad idea. If you want to have a command that does
"take bunch of messages, concatenate them into given (mbox in your case)
mailbox format, pipe resulting mbox to program"
then write a command to do that - don't break an existing well
specified command.

Julian Bradfield <jcb@inf.ed.ac.uk> writes:

> On 2010-01-04, blueman <NOSPAM@nospam.com> wrote:
>> HOWEVER, vm-pipe-message-part is skipping the initial 'From' line since
>> it is using (vm-headers-of m) as the start rather than (vm-start-of m).
>> Similarly, it uses (vm-text-end-of m) rather than (vm-end-of m)
>>
>> Since the usual reason for splitting/piping messages to an external
>> shell command is to pass the message to some standard 3rd program, it
>> would be helpful to use standard conventions -- by which I mean
>> something relatively consistent with 'formail' or better yet the RFC's.
>> In particular, when you ask for the "entire message", I at least expect
>> the entire message.
>
> VM is behaving correctly. The initial From line is NOT part of the RFC
> standard, nor is it part of the message. It's an obsolete Unix mbox hack
> that should have been killed off decades ago - the mbox format is
> patently unsuitable, leading to abominations like From-stuffing.
> I've had immense trouble with very broken mail systems (procmail) that
> insist on putting the From in where it has no business - it caused
> standards-compliant mail filters to break, because they saw a From
> instead of a proper message.
>
>> In my case, spamassassin thinks a collection of messages is just one big
>> message since it doesn't see the 'From' line.
>>
>> I would suggest making the following change so that the default "entire
>> message" truly includes the entire message and perhaps add an option
>> with four universal arguments that gives the current default behavior.
>
> Bad idea. If you want to have a command that does
> "take bunch of messages, concatenate them into given (mbox in your case)
> mailbox format, pipe resulting mbox to program"
> then write a command to do that - don't break an existing well
> specified command.

Except that I doubt too many people are using it since the command
itself is rather broken (at least if you try to use the optional
arguments)

Also, the command is *not* well specified. It says *entire* message
which includes the From lines and anything else.

If you want just the header or just the message then you can use the
already defined options with prefix args. If you want the sum total of
the headers (without the From) and the message (without any potential
trailers) then I am suggesting adding a 4th option.

Moreover, the multiple message functionality was actually a feature
request that I made of Robert a couple of years ago specifically for the
purpose of grabbing the *entire* message for use with spamassassin but
never got around to testing...

So I stand by my suggestion unless multiple others have coded on the
basis of this function -- especially since I seem to be one of the few
people who have really been using this code to any real extent unless
others have fixed the bugs without notifying the group...

I really think that the default should "do no harm" to the folder since
you may want to use this function to pipe portions of your mailbox
folder(s) non-destructively somewhere else... If you are not using mbox
format then this change shouldn't make any distance since your mta will
have stripped off the from. And if you are using mbox format, then you
probably want the from included or else you will be destructively
changing the output. Indeed, the casual user of mbox format will be
surprised to find his/her message format destroyed -- in fact, it took
me a while to figure out why spamassassin and formail were not able to
parse the extracted messages...

Do you have any specific use cases where the change I am proposing will
mess up existing code or applications? Theoretical problems encountered
elsewhere that may involve some theoretical future code is not
compelling especially since I am proposing to add an option that would
give you the sum total of the headers and footers (by adding a 4th
prefix). In fact, there would even be logic to building up the message
that way from its component parts...

Julian Bradfield <jcb@inf.ed.ac.uk> writes:
> On 2010-01-04, blueman <NOSPAM@nospam.com> wrote:
> VM is behaving correctly. The initial From line is NOT part of the RFC
> standard, nor is it part of the message. It's an obsolete Unix mbox hack
> that should have been killed off decades ago - the mbox format is
> patently unsuitable, leading to abominations like From-stuffing.

Depends on your use case -- I find it simple and easy-to-use without
either creating gazillions of files or needing some complex database to
maintain. Also, it is pretty much compatible with thunderbird which is
nice since it allows me to use it on the odd occassion when I need some
pretty formatting or other eye-candy functionality.

Now if I were an ISP or a large corporate employer, I would presumably
not use mbox... but then again I probably wouldn't be using vm either...

On 2010-01-04, blueman <NOSPAM@nospam.com> wrote:
> Julian Bradfield <jcb@inf.ed.ac.uk> writes:

> Also, the command is *not* well specified. It says *entire* message
> which includes the From lines and anything else.

No. THE FROM LINE IS NOT PART OF THE MESSAGE!
It's part of the folder's meta-data.
I keep my mailboxes in MMDF format - there is no conceivable sense in
which the enclosing ^A^A^A^A's can be considered "part of the
message". (Although VM "D" does, buggily in my view, display them in
the presentation buffer.)

> If you want just the header or just the message then you can use the
> already defined options with prefix args. If you want the sum total of
> the headers (without the From) and the message (without any potential
> trailers) then I am suggesting adding a 4th option.

That would be the "entire message" option, then - which should surely be the
default.
If you want the folder separators as well, define an option to include
them.

> Moreover, the multiple message functionality was actually a feature
> request that I made of Robert a couple of years ago specifically for the
> purpose of grabbing the *entire* message for use with spamassassin but
> never got around to testing...

Then you didn't ask for what you wanted. Beside which, spamassassin is
perfectably capable of taking an *individual* message in standard
form and processing it - indeed, that's what it does by default, isn't
it? If you want to feed it a folder in mbox format, don't you have to
give a special argument?


> So I stand by my suggestion unless multiple others have coded on the
> basis of this function -- especially since I seem to be one of the few
> people who have really been using this code to any real extent unless
> others have fixed the bugs without notifying the group...
>
> I really think that the default should "do no harm" to the folder since
> you may want to use this function to pipe portions of your mailbox
> folder(s) non-destructively somewhere else... If you are not using mbox
> format then this change shouldn't make any distance since your mta will
> have stripped off the from. And if you are using mbox format, then you
> probably want the from included or else you will be destructively
> changing the output. Indeed, the casual user of mbox format will be
> surprised to find his/her message format destroyed -- in fact, it took
> me a while to figure out why spamassassin and formail were not able to
> parse the extracted messages...

The message together with the folder data is not a legal
message. That's very destructive - I lost quite a bit of mail a few
years ago because I didn't notice that various filters were barfing on
the From line that procmail added.

If you absolutely NEED to contenate a bunch of messages and pipe the
resulting folder to a command, then do it.

> Do you have any specific use cases where the change I am proposing will
> mess up existing code or applications? Theoretical problems encountered

Yes: it will break every use I make of the | command in VM, where I'm
always piping to a progam that expects (or I want, in the case of
printing) to receive a message in RFC2822 form.

In any case, I don't understand your use case:
vm-pipe-message-to-command says that it runs the command separately
for each message - how are you using it?

blueman wrote:

> Since the usual reason for splitting/piping messages to an external
> shell command is to pass the message to some standard 3rd program, it
> would be helpful to use standard conventions -- by which I mean
> something relatively consistent with 'formail' or better yet the RFC's.
> In particular, when you ask for the "entire message", I at least expect
> the entire message.

Boy, you guys are really busy today, aren't you?

Unfortunately, it looks like 'formail' has let you down.  Julian is 
right.  RFC 2822, which specifies the format of internet messages, 
only allows headers and body in the messages.  The leading "From " 
line and the trailing empty line are *separators* used in the "mbox 
database" format, as specified in RFC 4155.  These are not part of the 
messages themselves.

Your change will break all users that are trying to interface to 
standards-compliant tools.

You can have either a 4-prefix-argument version or customization 
variables to include the separators in the "message".  Which would you 
prefer?

Cheers,
Uday

Julian Bradfield <jcb@inf.ed.ac.uk> writes:

> On 2010-01-04, blueman <NOSPAM@nospam.com> wrote:
>
> Then you didn't ask for what you wanted. Beside which, spamassassin is
> perfectably capable of taking an *individual* message in standard
> form and processing it - indeed, that's what it does by default, isn't
> it? If you want to feed it a folder in mbox format, don't you have to
> give a special argument?

I'm talking about the sa-learn Bayesian learning portion of
spamassassin. Feeding it one message at a time is exceedingly slow since
you have to restart sa-learn each time.

> The message together with the folder data is not a legal
> message. That's very destructive - I lost quite a bit of mail a few
> years ago because I didn't notice that various filters were barfing on
> the From line that procmail added.

And you can even more easily lose a sequence of messages by deleting the
separators between them. Just because you didn't notice the filters
doesn't mean that others won't have other problems...

> If you absolutely NEED to contenate a bunch of messages and pipe the
> resulting folder to a command, then do it.

Well, that was the whole reason that Robert *added* the command
vm-pipe-messages-to-command (where the 's' is plural) ... You keep
talking about this command as if it is yet to exist...
>
>> Do you have any specific use cases where the change I am proposing will
>> mess up existing code or applications? Theoretical problems encountered
>
> Yes: it will break every use I make of the | command in VM, where I'm
> always piping to a progam that expects (or I want, in the case of
> printing) to receive a message in RFC2822 form.

> In any case, I don't understand your use case:
> vm-pipe-message-to-command says that it runs the command separately
> for each message - how are you using it?

I am talking about (and using) the vm-pipe-messages-to-command...

And I would guess that if you used that command it would break at least
as many programs since external programs *need* to see a separator
because otherwise they have *no* way of separating messages. Indeed,
that is the whole purpose of message separators.

I can guarantee that when passing multiple concatenated messages (which is what
vm-pipe-messages-to-command does), it is much easier and more likely for
an external program to work with the separators than without the
separators. Indeed a concatenated sequence of alternating headers and
messages bodies is pretty meaningless if you can't tell where one
begins and the other ends. I would have thought that would be pretty
obvious...

It really seems that you are talking about a separate command and use
case. When using vm-pipe-message-to-command which seems to be your use
case, it makes sense to most of the time leave out the separators since
they are not needed when looking at a single message.

For my use case of using vm-pipe-messages-to-command, you need the
separators unless you want to treat it all like one big run-on chunk of
text. In which case why even have a notion of body or text at all or why
not just caesar-rotate it all ;)

Perhaps a compromise solution that would work for both cases would be to
add a flag to both routines that would toggle whether or not separators
are included -- technically, whether headers start at (vm-headers-of m)
vs. (vm-start-of m) and whether the body ends at (vm-text-end-of m)
vs. (vm-end-of m).

And based on your input, I would suggest that for
vm-pipe-message-to-command, this flag default to no separators and for
vm-pipe-messages-to-command it default to including separators. And
since it seems like you are not aware of let alone using
vm-pipe-messages-to-command and since the command has always been pretty
broken and is unlikely to have been used by many people if at all, this
change shouldn't create problems for you or others ;)

And I would be happy to write the trivial code change to make this all
work...

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:

> blueman wrote:
>
>> Since the usual reason for splitting/piping messages to an external
>> shell command is to pass the message to some standard 3rd program, it
>> would be helpful to use standard conventions -- by which I mean
>> something relatively consistent with 'formail' or better yet the RFC's.
>> In particular, when you ask for the "entire message", I at least expect
>> the entire message.
>
> Boy, you guys are really busy today, aren't you?
>
> Unfortunately, it looks like 'formail' has let you down.  Julian is
> right.  RFC 2822, which specifies the format of internet messages,
> only allows headers and body in the messages.  The leading "From "
> line and the trailing empty line are *separators* used in the "mbox
> database" format, as specified in RFC 4155.  These are not part of the
> messages themselves.
>
> Your change will break all users that are trying to interface to
> standards-compliant tools.
>
> You can have either a 4-prefix-argument version or customization
> variables to include the separators in the "message".  Which would you
> prefer?
>
Please see my other post. To me it seems like the difference is that
Julian is using vm-pipe-message-to-command where separators are not
needed and in his case at least has caused him confusion and data loss.

I am mostly interested in vm-pipe-messages-to-command where separators
(whether mbox or otherwise) are pretty essential if you want to preserve
any sense of where a message starts or ends.

My proposed compromise is to add an optional flag to the respective
routines that toggles the inclusion/exclusion of separators with the
default be to exclude separators in the single message case and include
them in the multiple message case... This "compromise" should result in
maximizing the possibility that piping to third party programs will work
with both commands.

On 2010-01-05, blueman <NOSPAM@nospam.com> wrote:
> Well, that was the whole reason that Robert *added* the command
> vm-pipe-messages-to-command (where the 's' is plural) ... You keep
> talking about this command as if it is yet to exist...

Oh dear. I owe you an apology.

Yes, I didn't notice the the "s". That's because it doesn't exist in
my VM (where almost nothing Rob added is included, except a few known
safe bits from rfaddons, so when I looked for it, auto-completion of
course gave me vm-pipe-message-to-command and my brain failed to
observe the inconsistency.

I apologize!

> I am talking about (and using) the vm-pipe-messages-to-command...
>
> And I would guess that if you used that command it would break at least
> as many programs since external programs *need* to see a separator
> because otherwise they have *no* way of separating messages. Indeed,
> that is the whole purpose of message separators.

However, looking at that function, I see that Rob has already
anticipated your need: you can simply set

(setq vm-pipe-messages-to-command-start
      "From Bogus From Line to Satisfy sa-learn\n")
or however much it needs to look like a real From line to satisfy 
sa-learn.

But now I understand what you're saying (and I promise to read more
carefully next time you say something!), I could see an argument for
changing THAT COMMAND (but absolutely not vm-pipe-message-part) so
that it uses the current default mailbox format. (Though what it
should do with maildir format, I don't know.)

blueman wrote:

> 
> I am mostly interested in vm-pipe-messages-to-command where separators
> (whether mbox or otherwise) are pretty essential if you want to preserve
> any sense of where a message starts or ends.

Well, if you had mentioned in the beginning that 
vm-pipe-messages-to-command isn't doing the expected thing, we would 
have understood you better.  We would have been more receptive as well :-)

So, what about the variables vm-pile-message-to-command-[start|end]? 
Aren't they working?

Cheers,
Uday

Julian Bradfield <jcb@inf.ed.ac.uk> writes:
> On 2010-01-05, blueman <NOSPAM@nospam.com> wrote:
>> Well, that was the whole reason that Robert *added* the command
>> vm-pipe-messages-to-command (where the 's' is plural) ... You keep
>> talking about this command as if it is yet to exist...
>
> Oh dear. I owe you an apology.
Appreciated but not necessary. 
>
> Yes, I didn't notice the the "s". That's because it doesn't exist in
> my VM (where almost nothing Rob added is included, except a few known
> safe bits from rfaddons, so when I looked for it, auto-completion of
> course gave me vm-pipe-message-to-command and my brain failed to
> observe the inconsistency.
Makes sense - though as an FYI, it has been in vm-save.el since 8.0 or so.
>

> However, looking at that function, I see that Rob has already
> anticipated your need: you can simply set
>
> (setq vm-pipe-messages-to-command-start
>       "From Bogus From Line to Satisfy sa-learn\n")
> or however much it needs to look like a real From line to satisfy 
> sa-learn.
>

That is helpful and would work in most cases (though sa-learn might
even benefit from a real 'From' line since it might be able to learn
from spoofed From_ vs. From:)

But more generally for those cases where you want to preserve the
original separators intact, I would propose the following simple
extension which doesn't change any existing functionality and which
would be consistent with the existing notation and common emacs/vm usage:

1. First add additional options to the underlying vm-pipe-message-part routine
   (you will see the logic of this and the number sequencing later)

(defun vm-pipe-message-part (m arg)
  "Return (START END) bounds for piping to external command, based on ARG."
  (cond ((equal prefix-arg '(4))
	 (list (vm-text-of m) (vm-text-end-of m)))
	((equal prefix-arg '(16))
	 (list (vm-headers-of m) (vm-text-of m)))
	((equal prefix-arg '(64))
	 (list (vm-vheaders-of m) (vm-text-end-of m)))
+	((equal prefix-arg '(256)) ; Same as '(4)' plus message end separator
+	 (list (vm-text-of m) (vm-end-of m)))
+	((equal prefix-arg '(1024)) ; Same as '(16)' plus message start separator
+	 (list (vm-start-of m) (vm-text-of m)))
+	((equal prefix-arg '(4096)) ; Same as '(64)' plus start & end separators
+	 (list (vm-start-of m) (vm-end-of m)))
	(t 
	 (list (vm-headers-of m) (vm-text-end-of m)))))

2. Now add the following explanation to the definition of
   vm-pipe-messages-to-command-start:

(defvar vm-pipe-messages-to-command-start ""
-  "*Inserted by `vm-pipe-messages-to-command' before a message.")
+  "*Inserted by `vm-pipe-messages-to-command' before a message.
+If set to 't', then vm-pipe-mesages-to-command includes the original message separators instead (and also ignores vm-pipe-message-to-command-end)"

3. Now make the following simple change to vm-pipe-messages-to-command
so that when the above variable is set to 't':
  - The prefix arg is multiplied by 256 [or set to 4096 if not an element of 
    (4 16 64)]
  - Insertions for the variable vm-pipe-message-to-command-start/end are 
    ignored. 

The result is that when the variable is set to 't' then 'vm-start-of' 
replaces 'vm-headers-of' and 'vm-end-of' replaces 'vm-text-end-of'


Note I am also including a critical bugfix (that I posted
over the weekend) where the macro variable ,no-wait wasn't quoted

(defun vm-pipe-messages-to-command (command &optional prefix-arg 
					    discard-output no-wait)
  "Run a shell command with contents from messages as input.

Similar to `vm-pipe-message-to-command', but it will call process
just once and pipe all messages to it.  For bulk operations this
is much faster than calling the command on each message.  This is
more like saving to a pipe.

Before a message it will insert `vm-pipe-messages-to-command-start'
and after a message `vm-pipe-messages-to-command-end'.

Output, if any, is displayed unless DISCARD-OUTPUT is t.

If NO-WAIT is t, then do not wait for process to finish, if it is
a function then call it with the COMMAND and OUTPUT-BUFFER as
arguments after the command finished."
  (interactive
   ;; protect value of last-command
   (let ((last-command last-command)
	 (this-command this-command))
     (vm-follow-summary-cursor)
     (vm-select-folder-buffer)
+	 ((vm-pipe-messages-to-command-start vm-pipe-messages-to-command-start))
+	 ((vm-pipe-messages-to-command-end vm-pipe-messages-to-command-end))
     (list (read-string "Pipe to command: " vm-last-pipe-command)
	   current-prefix-arg)))
+    (when (equal vm-pipe-messages-to-command-start t) ; Include original separators
+	  (if (member prefix-arg '(4 16 64))
+		  (setq prefix-arg (* prefix-arg 64))
+		  (setq prefix-arg 4096))
+	  (setq vm-pipe-messages-to-command-start "")
+	  (setq vm-pipe-messages-to-command-end ""))
  (vm-select-folder-buffer)
  (vm-check-for-killed-summary)
  (vm-error-if-folder-empty)
  (setq vm-last-pipe-command command)
  (let ((buffer (get-buffer-create "*Shell Command Output*"))
	(pop-up-windows (and pop-up-windows (eq vm-mutable-windows t)))
	;; prefix arg doesn't have "normal" meaning here, so only call
	;; vm-select-marked-or-prefixed-messages if we're using marks.
	(mlist (if (eq last-command 'vm-next-command-uses-marks)
		   (vm-select-marked-or-prefixed-messages 0)
		 (list (car vm-message-pointer))))
	m process)
    (save-excursion
      (set-buffer buffer)
      (erase-buffer))
    (setq process (start-process command buffer 
				 (or shell-file-name "sh")
				 shell-command-switch command))
    (set-process-sentinel 
     process 
     `(lambda (process status) 
	(setq status (process-status process))
	(if (eq 'exit status)
	    (if ,no-wait
		(vm-pipe-command-exit-handler 
		 process ,command ,discard-output 
		 (if (and ,no-wait (functionp ,no-wait))
-		     no-wait)))
+		     ,no-wait)))
	  (message "Command '%s' changed state to %s."
		   ,command status))))
    (while mlist
      (setq m (vm-real-message-of (car mlist)))
      (set-buffer (vm-buffer-of m))
	  (process-send-string process vm-pipe-messages-to-command-start)
      (save-restriction
	(widen)
	(let ((region (vm-pipe-message-part m prefix-arg)))
	  (process-send-region process (nth 0 region) (nth 1 region))))
	  (process-send-string process vm-pipe-messages-to-command-end)
      (setq mlist (cdr mlist)))

    (process-send-eof process)

    (when (not no-wait) 
      (while (and (eq 'run (process-status process)))
	(accept-process-output process)
	(sit-for 0))
      (vm-pipe-command-exit-handler process command discard-output))
    buffer))

Sorry, JJK.  I find this rather convoluted.  I have already added 
yesterday a variable called `vm-message-include-separators' to alter 
the meaning of what a "message" means.  All that is needed now is to 
call vm-pipe-message-to-command after setting this variable to t.

In fact, it would seem to make sense for vm-pipe-message-to-command to 
do this automatically whenever it is dealing with multiple messages. 
(I can't imagine why one would want a sequence of messages without any 
separators between them.  But, anyway, I won't split hairs about that!)

This will be in 8.1.1-devo version in a few minutes.

Cheers,
Uday

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
> Sorry, JJK.  I find this rather convoluted.  
Personally, I think my way of doing it is "clever" ;) since:
   - No new variables need to be used or learned
   - vm-pipe-message-part is extended in a transparent fashion so the
     added functionality can be used in other commands as needed
     (Also, my extension allows for retrieving headers, bodies, messages 
     each with or without the separator and without having to set a
     variable)
   - The changes to vm-pipe-messages-to-command are done cleanly without
     really even modifying any lines of existing code. It just
     changes/resets the value of 3 variables. In fact the change is so
     simple, it could be implemented with just a simple piece of around
     advice.


> I have already added yesterday a variable called
> `vm-message-include-separators' to alter the meaning of what a
> "message" means.  All that is needed now is to call
> vm-pipe-message-to-command after setting this variable to t.

OK. What happens to the variables vm-pipe-messages-to-command-start/end
when your variable is set to 't'? 

Note that by default vm-pipe-messages-to-command-end is set to "\n", so
if it is still used then you will be inserting an extra newline between
messages. 

If you don't automatically do something about it, then you have to
remember to set both variables to "" when you set
vm-message-include-separators=t or else you will have an extra newline
or other info inserted. This would not be a good idea since in addition
to being confusing, every time you want to "toggle" on/off this
functionality, you would now need to change (and potentially store) the
value 3 variables.

>
> In fact, it would seem to make sense for vm-pipe-message-to-command to
> do this automatically whenever it is dealing with multiple
> messages. (I can't imagine why one would want a sequence of messages
> without any separators between them. 
I could definitely imagine many cases:
1. If you want to grep for something in message bodies, you don't want
   the From_ separator (or even extra blank newlines)
2. Similarly if you just want to do something on the collection of all
   RFC headers (e.g., grep for all messages with something in the
   header), you  don't want the From_ line
3. If you are trying to know the exact total length of all the messges,
   again you don't want separators...
etc...

> But, anyway, I won't split hairs
> about that!)
>
> This will be in 8.1.1-devo version in a few minutes.
Thanks. Have you also included my bug fixes to:
     vm-pipe-command-exit-handler
     vm-pipe-messages-to-command

blueman wrote:
> Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
>> Sorry, JJK.  I find this rather convoluted.  
> Personally, I think my way of doing it is "clever" ;) since:

Indeed, it is.  But "cleverness" is damaging to software by entangling 
one thing with another.  If you change one thing, then everything else 
breaks.  So, the cleverest software is in fact streamlined software.

I was assuming that the only reason for the 
vm-pipe-messages-to-command-[start|end] variables is to provide 
separators for the vm-pipe-messages-to-command function.  If this 
function is given the stored separator lines instead, then the 
variables don't have a purpose any more and I would want to eliminate 
them.

>> I have already added yesterday a variable called
>> `vm-message-include-separators' to alter the meaning of what a
>> "message" means.  All that is needed now is to call
>> vm-pipe-message-to-command after setting this variable to t.

This wasn't enough, unfortunately.  There is an additional parameter 
`nowait' for the plural version of the function, which seems to 
require a more complicated protocol with the process.  So, I couldn't 
simply piggy back on the singular version of the function.  When I 
modified the plural version of the function to get rid of the extra 
separator lines, it hung on the examples I tried.  So, I will have to 
come back to this later.  Meanwhile, you seem to have something that 
works.  So, please go ahead and use it, and I will make sure that you 
will have a working version of the function when you have to upgrade.

>> This will be in 8.1.1-devo version in a few minutes.

Famous last words!

> Thanks. Have you also included my bug fixes to:
>      vm-pipe-command-exit-handler
>      vm-pipe-messages-to-command

I can't find them.  Did you post them to the newsgroups or Launchpad? 
   Actually, Launchpad is much better for posting bug reports because 
we can keep track of things better.  But newsgroups are better for 
discussion of course.

Cheers,
Uday

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:

> blueman wrote:
>> Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
>>> Sorry, JJK.  I find this rather convoluted.  
>> Personally, I think my way of doing it is "clever" ;) since:
>
> Indeed, it is.  But "cleverness" is damaging to software by entangling
> one thing with another.  If you change one thing, then everything else
> breaks.  So, the cleverest software is in fact streamlined software.

Well, the point is that my code doesn't mess anything up... but I
suppose you worry that it is dependent on the prefix numbering scheme.

>
> I was assuming that the only reason for the
> vm-pipe-messages-to-command-[start|end] variables is to provide
> separators for the vm-pipe-messages-to-command function.  If this
> function is given the stored separator lines instead, then the
> variables don't have a purpose any more and I would want to eliminate
> them.

I imagine they might still be useful if using a maildir-like format
which doesn't have a natural separator since each message is stored in a
separate file. I can also imagine many reasons for adding a unique
start/end if nothing less than for converting mail formats...

If you don't like the "cleverness" of my offset numbering scheme, I
would still recommend preserving the following portion of my solution:

  Use the notion of setting vm-pipe-messages-to-command-start to 't' as
  the way of signalling using "internal" separators in place of the ones
  specified as a string through that variable and it's cousin
  vm-pipe-messages-to-command-end

  This avoids endless proliferation of variables and helps explain the
  mutually exclusive tie between using internal separators
  vs. user-specified separators. Otherwise, you run the risk of users
  setting one variable without understanding the use of the other.

>>> I have already added yesterday a variable called
>>> `vm-message-include-separators' to alter the meaning of what a
>>> "message" means.  All that is needed now is to call
>>> vm-pipe-message-to-command after setting this variable to t.
>
> This wasn't enough, unfortunately.  There is an additional parameter
> nowait' for the plural version of the function, which seems to require
> a more complicated protocol with the process.  So, I couldn't simply
> piggy back on the singular version of the function.  When I modified
> the plural version of the function to get rid of the extra separator
> lines, it hung on the examples I tried.  So, I will have to come back
> to this later.  Meanwhile, you seem to have something that works.  So,
> please go ahead and use it, and I will make sure that you will have a
> working version of the function when you have to upgrade.

Well - nowait is broken which is part of what I submitted patches for.
>>> This will be in 8.1.1-devo version in a few minutes.
>
> Famous last words!
>
>> Thanks. Have you also included my bug fixes to:
>>      vm-pipe-command-exit-handler
>>      vm-pipe-messages-to-command
>
> I can't find them.  Did you post them to the newsgroups or Launchpad?
> Actually, Launchpad is much better for posting bug reports because we
> can keep track of things better.  But newsgroups are better for
> discussion of course.
>
I used your suggestion of using vm-submit-bug-report...but I guess you
didn't get it...

But in any case here are the 'diffs' for the 2 corrected functions:


;JJK: Fixes bug in existing routine (missing 2nd argument in defun line)
-(defun vm-pipe-command-exit-handler (process discard-output 
+(defun vm-pipe-command-exit-handler (process command discard-output 
					     &optional exit-handler)
  "Switch to output buffer of PROCESS if DISCARD-OUTPUT non-nil.
If non-nil call EXIT-HANDLER with the two arguments COMMAND and OUTPUT-BUFFER." 
  (let ((exit-code (process-exit-status process))
	(buffer (process-buffer process))
	(command (process-command process)))
  (if (not (zerop exit-code))
      (message "Command '%s' exit code is %d." command exit-code))
  (vm-display nil nil '(vm-pipe-message-to-command)
	      '(vm-pipe-message-to-command))
  (vm-switch-to-command-output-buffer command buffer discard-output)
  (if exit-handler
      (funcall exit-handler command buffer))))

;;JJK - Changes no-wait -> ,no-wait
;;;###autoload
(defun vm-pipe-messages-to-command (command &optional prefix-arg 
					    discard-output no-wait)
  "Run a shell command with contents from messages as input.

Similar to `vm-pipe-message-to-command', but it will call process
just once and pipe all messages to it.  For bulk operations this
is much faster than calling the command on each message.  This is
more like saving to a pipe.

Before a message it will insert `vm-pipe-messages-to-command-start'
and after a message `vm-pipe-messages-to-command-end'.

Output, if any, is displayed unless DISCARD-OUTPUT is t.

If NO-WAIT is t, then do not wait for process to finish, if it is
a function then call it with the COMMAND and OUTPUT-BUFFER as
arguments after the command finished."
  (interactive
   ;; protect value of last-command
   (let ((last-command last-command)
	 (this-command this-command))
     (vm-follow-summary-cursor)
     (vm-select-folder-buffer)
     (list (read-string "Pipe to command: " vm-last-pipe-command)
	   current-prefix-arg)))
  (vm-select-folder-buffer)
  (vm-check-for-killed-summary)
  (vm-error-if-folder-empty)
  (setq vm-last-pipe-command command)
  (let ((buffer (get-buffer-create "*Shell Command Output*"))
	(pop-up-windows (and pop-up-windows (eq vm-mutable-windows t)))
	;; prefix arg doesn't have "normal" meaning here, so only call
	;; vm-select-marked-or-prefixed-messages if we're using marks.
	(mlist (if (eq last-command 'vm-next-command-uses-marks)
		   (vm-select-marked-or-prefixed-messages 0)
		 (list (car vm-message-pointer))))
	m process)
    (save-excursion
      (set-buffer buffer)
      (erase-buffer))
    (setq process (start-process command buffer 
				 (or shell-file-name "sh")
				 shell-command-switch command))
    (set-process-sentinel 
     process 
     `(lambda (process status) 
	(setq status (process-status process))
	(if (eq 'exit status)
	    (if ,no-wait
		(vm-pipe-command-exit-handler 
		 process ,command ,discard-output 
		 (if (and ,no-wait (functionp ,no-wait))
-		     no-wait)))
+		     ,no-wait)))
	  (message "Command '%s' changed state to %s."
		   ,command status))))
    (while mlist
      (setq m (vm-real-message-of (car mlist)))
      (set-buffer (vm-buffer-of m))
      (process-send-string process vm-pipe-messages-to-command-start)
      (save-restriction
	(widen)
	(let ((region (vm-pipe-message-part m prefix-arg)))
	  (process-send-region process (nth 0 region) (nth 1 region))))
      (process-send-string process vm-pipe-messages-to-command-end)
      (setq mlist (cdr mlist)))

    (process-send-eof process)

    (when (not no-wait) 
      (while (and (eq 'run (process-status process)))
	(accept-process-output process)
	(sit-for 0))
      (vm-pipe-command-exit-handler process command discard-output))
    buffer))

Here is an alternative approach - that I think is better and more robust...

1. First change the definition string for the 
   vm-pipe-messages-to-command-start/end as follows:

(defvar vm-pipe-messages-to-command-start ""
  "*Inserted by `vm-pipe-messages-to-command' before a message.
If set to 't', then use the actual start message separator.")

(defvar vm-pipe-messages-to-command-end "\n"
  "*Inserted by `vm-pipe-messages-to-command' after a message.
If set to 't', then use the actual end message separator.")


2. Add these inline functions to vm-message.el

;;Start message separator
(defsubst vm-start-separator-of (message)
  (list (vm-start-of message)
		(vm-headers-of message))

;;End message separator
(defsubst vm-end-separator-of (message)
  (list (vm-text-end-of message)
		(vm-end-of message)))


3. Change vm-pipe-messages-to-command to either use the "global" 
   vm-pipe-messages-to-command-start/end variable (as is currently the
   case) or the per message separators depending on whether set to 't'
   or not

;;;###autoload
(defun vm-pipe-messages-to-command (command &optional prefix-arg 
					    discard-output no-wait)
  "Run a shell command with contents from messages as input.

Similar to `vm-pipe-message-to-command', but it will call process
just once and pipe all messages to it.  For bulk operations this
is much faster than calling the command on each message.  This is
more like saving to a pipe.

Before a message it will insert `vm-pipe-messages-to-command-start'
and after a message `vm-pipe-messages-to-command-end'.

Output, if any, is displayed unless DISCARD-OUTPUT is t.

If NO-WAIT is t, then do not wait for process to finish, if it is
a function then call it with the COMMAND and OUTPUT-BUFFER as
arguments after the command finished."
  (interactive
   ;; protect value of last-command
   (let ((last-command last-command)
	 (this-command this-command))
     (vm-follow-summary-cursor)
     (vm-select-folder-buffer)
     (list (read-string "Pipe to command: " vm-last-pipe-command)
	   current-prefix-arg)))
  (vm-select-folder-buffer)
  (vm-check-for-killed-summary)
  (vm-error-if-folder-empty)
  (setq vm-last-pipe-command command)
  (let ((buffer (get-buffer-create "*Shell Command Output*"))
	(pop-up-windows (and pop-up-windows (eq vm-mutable-windows t)))
	;; prefix arg doesn't have "normal" meaning here, so only call
	;; vm-select-marked-or-prefixed-messages if we're using marks.
	(mlist (if (eq last-command 'vm-next-command-uses-marks)
		   (vm-select-marked-or-prefixed-messages 0)
		 (list (car vm-message-pointer))))
	m process)
    (save-excursion
      (set-buffer buffer)
      (erase-buffer))
    (setq process (start-process command buffer 
				 (or shell-file-name "sh")
				 shell-command-switch command))
    (set-process-sentinel 
     process 
     `(lambda (process status) 
	(setq status (process-status process))
	(if (eq 'exit status)
	    (if ,no-wait
		(vm-pipe-command-exit-handler 
		 process ,command ,discard-output 
		 (if (and ,no-wait (functionp ,no-wait))
+		     ,no-wait)))
	  (message "Command '%s' changed state to %s."
		   ,command status))))
    (while mlist
      (setq m (vm-real-message-of (car mlist)))
      (set-buffer (vm-buffer-of m))
-      (process-send-string process vm-pipe-messages-to-command-start)
      (save-restriction
	(widen)
+	(if (equal vm-pipe-messages-to-command-start t)
+		  (apply 'process-send-region process (vm-start-separator-of m))
+		  (process-send-string process vm-pipe-messages-to-command-start))
-	(let ((region (vm-pipe-message-part m prefix-arg)))
-	  (process-send-region process (nth 0 region) (nth 1 region)))
+   (apply 'process-send-region process (vm-pipe-message-part m prefix-arg))
+   (if (equal vm-pipe-messages-to-command-end t)
+		  (apply 'process-send-region process (vm-end-separator-of m))
+		  (process-send-string process vm-pipe-messages-to-command-end)))
-      (process-send-string process vm-pipe-messages-to-command-end)
      (setq mlist (cdr mlist)))

    (process-send-eof process)

    (when (not no-wait) 
      (while (and (eq 'run (process-status process)))
	(accept-process-output process)
	(sit-for 0))
      (vm-pipe-command-exit-handler process command discard-output))
    buffer))


NOTE: I haven't tested the above code and there may be syntax
errors... but I will test if people agree that this is the right approach...

Brief testing show it works -- indeed piping all messages in a folder to
an external file matches exactly the original file -- which is what I
think we want...

Just one minor syntax error - I left off the final closing paren from
the following:

;;Start message separator
(defsubst vm-start-separator-of (message)
  (list (vm-start-of message)
		(vm-headers-of message)))

blueman wrote:

> NOTE: I haven't tested the above code and there may be syntax
> errors... but I will test if people agree that this is the right approach...

Indeed, it is nice!  You see, cleverness can be put to use in many 
ways :-)

Cheers,
Uday

blueman wrote:

> I used your suggestion of using vm-submit-bug-report...but I guess you
> didn't get it...

No, I didn't get it.  It is not stuck at the launchpad site either.

I just tested the launchpad mailing list by sending a blank bug report 
and it came through within a few minutes.  So, it is working.  Not 
clear what happened to your bug report.

Cheers,
Uday

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
> blueman wrote:
>
>> NOTE: I haven't tested the above code and there may be syntax
>> errors... but I will test if people agree that this is the right approach...
>
> Indeed, it is nice!  You see, cleverness can be put to use in many
> ways :-)
>
> Cheers,
> Uday

Thanks for the kind words and I believe that this back-and-forth has
resulted in a cleaner/nicer result for all..

blueman wrote:
> Brief testing show it works -- indeed piping all messages in a folder to
> an external file matches exactly the original file -- which is what I
> think we want...

It didn't work for me.  I needed to make at least the following fixes.

Running it on cat (on Windows Cygwin) still hangs for me.  I think it 
has something to do with the asynchronous handler.  But I will punt 
that issue for now.

Cheers,
Uday


--- vm-save.el.blueman	2010-01-06 14:21:26.000000000 +0000
+++ vm-save.el	2010-01-06 15:31:42.000000000 +0000
@@ -642,20 +642,21 @@
  	   current-prefix-arg)))
    (vm-pipe-message-to-command command prefix-arg t))

-(defun vm-pipe-command-exit-handler (process discard-output
+(defun vm-pipe-command-exit-handler (process command discard-output
  					     &optional exit-handler)
-  "Switch to output buffer of PROCESS if DISCARD-OUTPUT non-nil.
+"Switch to output buffer of PROCESS that ran COMMAND, if
+DISCARD-OUTPUT non-nil.
  If non-nil call EXIT-HANDLER with the two arguments COMMAND and 
OUTPUT-BUFFER."
    (let ((exit-code (process-exit-status process))
  	(buffer (process-buffer process))
-	(command (process-command process)))
+	(process-command (process-command process)))
    (if (not (zerop exit-code))
        (message "Command '%s' exit code is %d." command exit-code))
    (vm-display nil nil '(vm-pipe-message-to-command)
  	      '(vm-pipe-message-to-command))
    (vm-switch-to-command-output-buffer command buffer discard-output)
    (if exit-handler
-      (funcall exit-handler command buffer))))
+      (funcall exit-handler process-command buffer))))

  (defvar vm-pipe-messages-to-command-start ""
    "*The leading message separator to be used by












> 
> Just one minor syntax error - I left off the final closing paren from
> the following:
> 
> ;;Start message separator
> (defsubst vm-start-separator-of (message)
>   (list (vm-start-of message)
> 		(vm-headers-of message)))

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
> blueman wrote:
>> Brief testing show it works -- indeed piping all messages in a folder to
>> an external file matches exactly the original file -- which is what I
>> think we want...
>
> It didn't work for me.  I needed to make at least the following fixes.
>
> Running it on cat (on Windows Cygwin) still hangs for me.  I think it
> has something to do with the asynchronous handler.  But I will punt
> that issue for now.
>
> Cheers,
> Uday
>
Please try with the two patches I submitted. They really just correct
probably typos. I don't think the changes you made are necessary. But
please try my changes first...
---------------------------------------------------------------------

; Fixes missing 'command' argument in defun
(defun vm-pipe-command-exit-handler (process command discard-output 
					     &optional exit-handler)
  "Switch to output buffer of PROCESS if DISCARD-OUTPUT non-nil.
If non-nil call EXIT-HANDLER with the two arguments COMMAND and OUTPUT-BUFFER." 
  (let ((exit-code (process-exit-status process))
	(buffer (process-buffer process))
	(command (process-command process)))
  (if (not (zerop exit-code))
      (message "Command '%s' exit code is %d." command exit-code))
  (vm-display nil nil '(vm-pipe-message-to-command)
	      '(vm-pipe-message-to-command))
  (vm-switch-to-command-output-buffer command buffer discard-output)
  (if exit-handler
      (funcall exit-handler command buffer))))

;-------------------------------------------------------------------------
; Adds my new functionality and fixes unquoted ,no-wait argument
(defun vm-pipe-messages-to-command (command &optional prefix-arg 
					    discard-output no-wait)
  "Run a shell command with contents from messages as input.

Similar to `vm-pipe-message-to-command', but it will call process
just once and pipe all messages to it.  For bulk operations this
is much faster than calling the command on each message.  This is
more like saving to a pipe.

Before a message it will insert `vm-pipe-messages-to-command-start'
and after a message `vm-pipe-messages-to-command-end'.

Output, if any, is displayed unless DISCARD-OUTPUT is t.

If NO-WAIT is t, then do not wait for process to finish, if it is
a function then call it with the COMMAND and OUTPUT-BUFFER as
arguments after the command finished."
  (interactive
   ;; protect value of last-command
   (let ((last-command last-command)
	 (this-command this-command))
     (vm-follow-summary-cursor)
     (vm-select-folder-buffer)
     (list (read-string "Pipe to command: " vm-last-pipe-command)
	   current-prefix-arg)))
  (vm-select-folder-buffer)
  (vm-check-for-killed-summary)
  (vm-error-if-folder-empty)
  (setq vm-last-pipe-command command)
  (let ((buffer (get-buffer-create "*Shell Command Output*"))
	(pop-up-windows (and pop-up-windows (eq vm-mutable-windows t)))
	;; prefix arg doesn't have "normal" meaning here, so only call
	;; vm-select-marked-or-prefixed-messages if we're using marks.
	(mlist (if (eq last-command 'vm-next-command-uses-marks)
		   (vm-select-marked-or-prefixed-messages 0)
		 (list (car vm-message-pointer))))
	m process)
    (save-excursion
      (set-buffer buffer)
      (erase-buffer))
    (setq process (start-process command buffer 
				 (or shell-file-name "sh")
				 shell-command-switch command))
    (set-process-sentinel 
     process 
     `(lambda (process status) 
	(setq status (process-status process))
	(if (eq 'exit status)
	    (if ,no-wait
		(vm-pipe-command-exit-handler 
		 process ,command ,discard-output 
		 (if (and ,no-wait (functionp ,no-wait))
		     ,no-wait)))
	  (message "Command '%s' changed state to %s."
		   ,command status))))
    (while mlist
      (setq m (vm-real-message-of (car mlist)))
      (set-buffer (vm-buffer-of m))

      (save-restriction
	(widen)
	(if (equal vm-pipe-messages-to-command-start t)
		  (apply 'process-send-region process (vm-start-separator-of m))
		  (process-send-string process vm-pipe-messages-to-command-start))
	(apply 'process-send-region process (vm-pipe-message-part m prefix-arg))
   (if (equal vm-pipe-messages-to-command-end t)
		  (apply 'process-send-region process (vm-end-separator-of m))
		  (process-send-string process vm-pipe-messages-to-command-end)))
      (setq mlist (cdr mlist)))

    (process-send-eof process)

    (when (not no-wait) 
      (while (and (eq 'run (process-status process)))
	(accept-process-output process)
	(sit-for 0))
      (vm-pipe-command-exit-handler process command discard-output))
    buffer))

blueman wrote:

>>
> Please try with the two patches I submitted. 

Hi, I don't know what you mean by "submitted".  I haven't yet received 
anything from you via launchpad.net.

Whatever you posted here has been put in.  But in your version of the 
exit-handler below, notice that you added a 'command' argument but 
never used it!

Cheers,
Uday



> ---------------------------------------------------------------------
> 
> ; Fixes missing 'command' argument in defun
> (defun vm-pipe-command-exit-handler (process command discard-output 
> 					     &optional exit-handler)
>   "Switch to output buffer of PROCESS if DISCARD-OUTPUT non-nil.
> If non-nil call EXIT-HANDLER with the two arguments COMMAND and OUTPUT-BUFFER." 
>   (let ((exit-code (process-exit-status process))
> 	(buffer (process-buffer process))
> 	(command (process-command process)))
>   (if (not (zerop exit-code))
>       (message "Command '%s' exit code is %d." command exit-code))
>   (vm-display nil nil '(vm-pipe-message-to-command)
> 	      '(vm-pipe-message-to-command))
>   (vm-switch-to-command-output-buffer command buffer discard-output)
>   (if exit-handler
>       (funcall exit-handler command buffer))))
> 

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:

> blueman wrote:
>
>>>
>> Please try with the two patches I submitted. 
>
> Hi, I don't know what you mean by "submitted".  I haven't yet received
> anything from you via launchpad.net.
>
> Whatever you posted here has been put in.  But in your version of the
> exit-handler below, notice that you added a 'command' argument but
> never used it!

Sure it is used:
>>       (funcall exit-handler command buffer))))
Plus, even if it weren't used, it still would create a problem since the
function vm-pipe-command-exit-handler is called with 'command' placed in
the second position.

Again, I am quite positive that it works fine with my edits. If you
can't get it working, send me some debug traces and/or sample code and I
will try to help figure out why it is not working on your setup.

>
>
>> ---------------------------------------------------------------------
>>
>> ; Fixes missing 'command' argument in defun
>> (defun vm-pipe-command-exit-handler (process command discard-output
>> &optional exit-handler)
>>   "Switch to output buffer of PROCESS if DISCARD-OUTPUT non-nil.
>> If non-nil call EXIT-HANDLER with the two arguments COMMAND and
>> OUTPUT-BUFFER."   (let ((exit-code (process-exit-status process))
>> 	(buffer (process-buffer process))
>> 	(command (process-command process)))
>>   (if (not (zerop exit-code))
>>       (message "Command '%s' exit code is %d." command exit-code))
>>   (vm-display nil nil '(vm-pipe-message-to-command)
>> 	      '(vm-pipe-message-to-command))
>>   (vm-switch-to-command-output-buffer command buffer discard-output)
>>   (if exit-handler
>>       (funcall exit-handler command buffer))))
>>

blueman wrote:

>> Whatever you posted here has been put in.  But in your version of the
>> exit-handler below, notice that you added a 'command' argument but
>> never used it!
> 
> Sure it is used:
>>>       (funcall exit-handler command buffer))))

Right, you fell into the trap!  This is inside the scope of let, which 
rebinds 'command'.

I just ran 'cat' with the |s operation, and it broke.

process-command apparently returns a list of strings, which wasn't 
what was expected.

Cheers,
Uday

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
> blueman wrote:
>
>>> Whatever you posted here has been put in.  But in your version of the
>>> exit-handler below, notice that you added a 'command' argument but
>>> never used it!
>>
>> Sure it is used:
>>>>       (funcall exit-handler command buffer))))
>
> Right, you fell into the trap!  This is inside the scope of let, which
> rebinds 'command'.
>
> I just ran 'cat' with the |s operation, and it broke.
>
> process-command apparently returns a list of strings, which wasn't
> what was expected.
>
> Cheers,
> Uday

Well you clipped out the other part of my message:
     Plus, even if it weren't used, it still would create a problem since the
     function vm-pipe-command-exit-handler is called with 'command' placed in
     the second position.

So, either you add it here or you remove it from the two places in
vm-pipe-messages-to-command that call it.


		(vm-pipe-command-exit-handler 
		 process ,command ,discard-output 
		 (if (and ,no-wait (functionp ,no-wait))
		     ,no-wait)))

[remember you need to add the comma before the final no-wait above]


      (vm-pipe-command-exit-handler process command discard-output))


So maybe better to remove it from these two places -- then that would
make all my 'bug fixes' limited to vm-pipe-messages-to-command:
  1. Add comma before ,no-wait
  2. *Delete* command from the above two places

I will test it to be sure tonight...

blueman <NOSPAM@nospam.com> writes:
> So maybe better to remove it from these two places -- then that would
> make all my 'bug fixes' limited to vm-pipe-messages-to-command:
>   1. Add comma before ,no-wait
>   2. *Delete* command from the above two places
>
> I will test it to be sure tonight...

Quick test shows it works!

So the sum total of my patches and changes to fix the bugs and extend
the functionality to allow inclusion of existing pre & post separators
is confined to just minor changes in the following code:

(defvar vm-pipe-messages-to-command-start ""
  "*Inserted by `vm-pipe-messages-to-command' before a message.
If set to 't', then use the actual start message separator.")

(defvar vm-pipe-messages-to-command-end "\n"
  "*Inserted by `vm-pipe-messages-to-command' after a message.
If set to 't', then use the actual end message separator.")


;;;###autoload
(defun vm-pipe-messages-to-command (command &optional prefix-arg 
					    discard-output no-wait)
  "Run a shell command with contents from messages as input.

Similar to `vm-pipe-message-to-command', but it will call process
just once and pipe all messages to it.  For bulk operations this
is much faster than calling the command on each message.  This is
more like saving to a pipe.

Before a message it will insert `vm-pipe-messages-to-command-start'
and after a message `vm-pipe-messages-to-command-end'.

Output, if any, is displayed unless DISCARD-OUTPUT is t.

If NO-WAIT is t, then do not wait for process to finish, if it is
a function then call it with the COMMAND and OUTPUT-BUFFER as
arguments after the command finished."
  (interactive
   ;; protect value of last-command
   (let ((last-command last-command)
	 (this-command this-command))
     (vm-follow-summary-cursor)
     (vm-select-folder-buffer)
     (list (read-string "Pipe to command: " vm-last-pipe-command)
	   current-prefix-arg)))
  (vm-select-folder-buffer)
  (vm-check-for-killed-summary)
  (vm-error-if-folder-empty)
  (setq vm-last-pipe-command command)
  (let ((buffer (get-buffer-create "*Shell Command Output*"))
	(pop-up-windows (and pop-up-windows (eq vm-mutable-windows t)))
	;; prefix arg doesn't have "normal" meaning here, so only call
	;; vm-select-marked-or-prefixed-messages if we're using marks.
	(mlist (if (eq last-command 'vm-next-command-uses-marks)
		   (vm-select-marked-or-prefixed-messages 0)
		 (list (car vm-message-pointer))))
	m process)
    (save-excursion
      (set-buffer buffer)
      (erase-buffer))
    (setq process (start-process command buffer 
				 (or shell-file-name "sh")
				 shell-command-switch command))
    (set-process-sentinel 
     process 
     `(lambda (process status) 
	(setq status (process-status process))
	(if (eq 'exit status)
	    (if ,no-wait
		(vm-pipe-command-exit-handler 
		 process ,discard-output 
		 (if (and ,no-wait (functionp ,no-wait))
		     ,no-wait)))
	  (message "Command '%s' changed state to %s."
		   ,command status))))
    (while mlist
      (setq m (vm-real-message-of (car mlist)))
      (set-buffer (vm-buffer-of m))

      (save-restriction
	(widen)
	(if (equal vm-pipe-messages-to-command-start t)
		  (apply 'process-send-region process (vm-start-separator-of m))
		  (process-send-string process vm-pipe-messages-to-command-start))
	(apply 'process-send-region process (vm-pipe-message-part m prefix-arg))
   (if (equal vm-pipe-messages-to-command-end t)
		  (apply 'process-send-region process (vm-end-separator-of m))
		  (process-send-string process vm-pipe-messages-to-command-end)))
      (setq mlist (cdr mlist)))

    (process-send-eof process)

    (when (not no-wait) 
      (while (and (eq 'run (process-status process)))
	(accept-process-output process)
	(sit-for 0))
      (vm-pipe-command-exit-handler process discard-output))
    buffer))

blueman wrote:

> 
> Quick test shows it works!

Perhaps it does.  But I already added a fix a day or two ago, tested 
it and committed to the repository on launchpad.  Unfortunately, this 
to-and-fro hasn't contributed all that much to the problem, after the 
initial discussion.  There are some 20 odd messages in this thread.  I 
don't think this mode of communication has been very effective.

So, for the future, let us follow the procedure that Robert had set up 
and documented in the README file.  That would be faster and easier 
for me to deal with.

Cheers,
Uday

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
> blueman wrote:
>
>>
>> Quick test shows it works!
>
> Perhaps it does.  But I already added a fix a day or two ago, tested
> it and committed to the repository on launchpad.  Unfortunately, this
> to-and-fro hasn't contributed all that much to the problem, after the
> initial discussion.  There are some 20 odd messages in this thread.  I
> don't think this mode of communication has been very effective.

Personally, I find that comment unfair. I have:
       - Identified multiple show-stopper bugs in the routine
       - Coded bug fixes
       - Suggested and discussed extensions to make it usable on
         multiple messages rather than running them together
       - Coded the extension

I have shared my code the whole way through and have accepted input from
you and Julian which ultimately made it better.

At the same time, you mention making fixes which you haven't shared here
and which I can't seem to even find on launchpad (I am looking at
http://bazaar.launchpad.net/~reddyuday/viewmail/8.1/changes) But based
upon your allusions to rewriting code and adding variables, it seems
like you may have done something more convoluted than necessary which
could have other unforseen consequences (indeed all that was necessary
was removing two words and quoting a third).

> So, for the future, let us follow the procedure that Robert had set up
> and documented in the README file.  That would be faster and easier
> for me to deal with.

May I remind you what the README says:
      
        If you have any problems or meet a bug it is best to discuss
        them on the USENET groups gnu.emacs.vm.bugs or
        gnu.emacs.vm.info!  

This is Robert's first and primary suggestion and is indeed what we are
doing now!!!! For good reason, Robert suggested this as the first course
of action since it allows everybody to see and join in the discussion as
much or as little as they please.

        Also search the groups before posting as there might have been
        some discussion and a fix before.

I even did this before posting!!!!

       In addition to discussing the problems on the newsgroups, you are
       encouraged to file a bug report using the Emacs function M-x
       vm-submit-bug-report This function formats an email report
       including the entire state of VM which can be used to diagnose
       and fix the bug.

I did this "additional encouragement" though it seems like your address
didn't accept my email without notifying me back of an error. 

Also, bug reports with long traces are good for complicated bugs where
you are asking someone else to do the work. Here I diagnosed and fixed
the bug myself so I fail to see the need for submitting all the
environment data etc. 

Finally, the bug reports don't seem to be public so that there is no way
for others to see the reports and contribute to their resolution. If you
want to set up a bugzilla site, we could have the same 20 message
back-and-forth we had on the newsgroup on a bugzilla thread.

[Steps on soapbox...]
I am plenty capable of coding everything I want or need in elisp and
have been doing so for more than 20 years. My inputs to the vm project
are sprinkled across a number of core files. I always try to give back
more in code and bug fixes than I request.

If you don't want my help in contributing further, I am happy to keep my
bug identifications, bug fixes and contributions to myself. I happen to
have spent a fair bit of time working on this and other bugs this week
and don't appreciate being publicly told that such conversations are
unhelpful. I didn't just complain about bugs or ask for help or new
features -- but went and debugged them myself and wrote the code to fix
or extend things. 

Certainly this thread would have been shorter had I taken the approach
of almost all other posters in terms of just complaining about bugs and
leaving it to others to fix. If that is what you want, I would be happy
to oblige and let you do all the work. But then don't be surprised when
you don't find others stepping up to volunteer to help you as
maintainers.

[Steps off soapbox...]

Sorry - but your note really struck me the wrong way... such comments
are usually reserved for trolls, OT threads, rants, or people who
just plain complain without contributing.

Good luck

blueman <NOSPAM@nospam.com> writes:
> Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
>> blueman wrote:
>>
>>>
>>> Quick test shows it works!
>>
>> Perhaps it does.  But I already added a fix a day or two ago, tested
>> it and committed to the repository on launchpad.  Unfortunately, this
>> to-and-fro hasn't contributed all that much to the problem, after the
>> initial discussion.  There are some 20 odd messages in this thread.  I
>> don't think this mode of communication has been very effective.
>
> Personally, I find that comment unfair. I have:
>        - Identified multiple show-stopper bugs in the routine
>        - Coded bug fixes
>        - Suggested and discussed extensions to make it usable on
>          multiple messages rather than running them together
>        - Coded the extension
>
> I have shared my code the whole way through and have accepted input from
> you and Julian which ultimately made it better.
>
> At the same time, you mention making fixes which you haven't shared here
> and which I can't seem to even find on launchpad (I am looking at
> http://bazaar.launchpad.net/~reddyuday/viewmail/8.1/changes) But based
> upon your allusions to rewriting code and adding variables, it seems
> like you may have done something more convoluted than necessary which
> could have other unforseen consequences (indeed all that was necessary
> was removing two words and quoting a third).

OK - I found your revisions.

The way you implemented it is really just a kludge which adds an
additional unnecessary variable vm-message-include-separators. 

Moreover, as I metioned in one of my earlier "unproductive" posts, your
method actually ends up adding an extra newline to each message (and
hence doesn't do what you claim it does) unless the user also remembers
to change the value of vm-pipe-message-to-command-end from "\n" to nil
-- which is hardly obvious and which you don't document. This also makes
me wonder how well you tested the function because if you had done the
minimal test of diff'ing a pipe to 'cat' of all messages in a folder
against the original folder contents you would have noticed this
difference and probably not have coded the change this way.

Additonally, you added a comment to the vm-pipe-message-to-command
function whereas the real need for the separators is in the
vm-pipe-messages-to-command function where you are combining multiple
messages in one pipe. In fact, Robert's
vm-pipe-messages-to-command-start/end variables *only* apply to the
vm-pipe-messages-to-command function.

Finally, your method doesn't allow for adding the "From_" line in the
case where you are just looking at the headers (prefix-arg 16 or 64) and
there are use cases where you might want to see the From_ and From:
lines when looking just at the non-body part of messages.

Also if you look through the "20-odd" messages, the first half were
spent productively generating the solution that I ultimately coded
(which you yourself acknowledged "Indeed, it is nice!") -- yet you
didn't include the fix in your patch but instead added an inelegant
kludge. 

And the second half of the messages were devoted to fixing the actual
show-stopper bug in the code which causes it to crash when you supply
either of the optional arguments. However, despite my supplying the bug
fixes multiple times on this thread (and in my bug report) you have not
added it to the code. Indeed I don't know how you can claim to have
properly tested this function without checking the optional variables
given that they account for much of the code complexity and thus deserve
to be tested.

So, yes this thread is a waste -- but only because you don't seem to
have listened to any of the productive suggestions included in the
thread... Even worse, in my opinion, your fix added an inelegant and
arguably broken kludge to an otherwise elegant function and to add
insult to injury the vm-pipe-messages-to-command function is itself
still fundamentally broken.

blueman wrote:

>> Perhaps it does.  But I already added a fix a day or two ago, tested
>> it and committed to the repository on launchpad.  Unfortunately, this
>> to-and-fro hasn't contributed all that much to the problem, after the
>> initial discussion.  There are some 20 odd messages in this thread.  I
>> don't think this mode of communication has been very effective.
> 
> Personally, I find that comment unfair. I have:
>        - Identified multiple show-stopper bugs in the routine
>        - Coded bug fixes
>        - Suggested and discussed extensions to make it usable on
>          multiple messages rather than running them together
>        - Coded the extension
> 
> I have shared my code the whole way through and have accepted input from
> you and Julian which ultimately made it better.

Agreed that you have done all of these.  But, after we have decided on 
a reasonable course of action (your proposal on 5 Jan, 23:50 UTC), 
things didn't go well.  You have posted the fixes here.  It wasn't a 
patch file.  So, I had to insert it manually into the code  base and 
test it.  When I tested it, it didn't work.  So I debugged it, found 
the fixes, committed them to the code base, and also posted a patch 
file for you (based on the code that you posted).  So, it is easy 
enough for you to test my version.  Did you test it?  Did you go back 
and retest your code posted here (on 6th Jan, 16:42 UTC)?

Your response was

    "Please try with the two patches I submitted. They really just
     correct probably typos. I don't think the changes you made are
     necessary. But please try my changes first..."

This is not helpful.  I have tried the "two patches you submitted", 
found problems, and sent you back the corrections.  Did you apply them 
and test them?  If you didn't do so, the further exchange didn't make 
any contribution.  In fact, it wasn't clear if we were even talking 
about the same things any more.

(My apologies for having misremembered that I committed the changes to 
launchpad.  I posted the patches here and was waiting to hear from 
you.  I have now uploaded my version to launchpad so that you can test 
it properly.)

I cannot be spending as much time on each and every issue.  If you 
want to make a contribution, please state your bug clearly and provide 
a *patchfile*, which I can apply and test, and after I accept it, we 
can move on.  I am also at liberty to revise your patches so as to 
maintain the consistency and coherence of the code base.  If you send 
me patches that fail my testing, that will make it harder for me to 
accept patches from you in future!  So, take the time to test them 
properly.  I am not going anywhere.

Cheers,
Uday

blueman wrote:

>> So, for the future, let us follow the procedure that Robert had set up
>> and documented in the README file.  That would be faster and easier
>> for me to deal with.
> 
> May I remind you what the README says:

It also says:

> BZR (http://bazaar-vcs.org/)
> 
> We maintain the source code using Bazaar.
> 
> If you want to send us changes or start hacking on your own you may want to 
> branch from the follwing URL:
> 
>   http://bazaar.launchpad.net/~vm/vm/trunk
> 
> 
> # create your own branch from the trunk
> # the 'lp' URL's do they same job as the 'http' URL's above.
> bzr get lp:vm
> (or)
> bzr get lp:~vm/vm/trunk
> # get updates
> bzr pull
> # start hacking 
> xemacs vm-pgg.el
> # commit your changes 
> bzr commit
> # Generate a bundle of your changes for merging
> bzr bundle-revisions --output=xy-changes.diff
> # Attach the bundle to a mail (rather than doing cut&paste) and send
> # it to vm @ lists.launchpad.net with a descriptive subject.  

This is an efficient and clean procedure for sharing fixes or 
contributions.  I used it myself for years while Rob was in charge. 
Samuel Bronson and Brent Goodrick even maintained their branches at 
launchpad (see https://code.launchpad.net/viewmail).  When Rob went 
inactive in 2008, they kept on going, and all their fixes were there 
for me to find and merge when I got involved.  This was of great help! 
  I couldn't have probably released 8.1.0 without all their contributions.

>       
>         If you have any problems or meet a bug it is best to discuss
>         them on the USENET groups gnu.emacs.vm.bugs or
>         gnu.emacs.vm.info!  
> 
> This is Robert's first and primary suggestion and is indeed what we are
> doing now!!!! For good reason, Robert suggested this as the first course
> of action since it allows everybody to see and join in the discussion as
> much or as little as they please.
> 
>         Also search the groups before posting as there might have been
>         some discussion and a fix before.
> 
> I even did this before posting!!!!
> 
>        In addition to discussing the problems on the newsgroups, you are
>        encouraged to file a bug report using the Emacs function M-x
>        vm-submit-bug-report This function formats an email report
>        including the entire state of VM which can be used to diagnose
>        and fix the bug.
> 
> I did this "additional encouragement" though it seems like your address
> didn't accept my email without notifying me back of an error. 

These are procedures for people that merely report bugs.  But you are 
going beyond reporting bugs.  You want to contribute code.  The README 
file does not say contribute code by posting it to the newsgroups. 
Code needs a much more careful treatment.

I am not sure what you mean by "your address" in the above. 
vm@lists.launchpad.net is the public address of the VM development 
team.  It is unfortunate that launchpad dropped your message, assuming 
that is what it did, but I suggest you test it out and ensure that you 
are able to communicate with the VM development team.
There is no reason why you shouldn't register on launchpad.

> Finally, the bug reports don't seem to be public so that there is no way
> for others to see the reports and contribute to their resolution. If you
> want to set up a bugzilla site, we could have the same 20 message
> back-and-forth we had on the newsgroup on a bugzilla thread.

The launchpad bug reports are public, but email addresses are kept 
private.  People can also flag their issues as security issues if they 
don't want them to be publicly visible.  I don't mind moving to 
bugzilla if we find launchpad bugs wanting.  But, at the moment, we 
are already distributed over two sites (launchpad and savannah) and I 
wouldn't want to add a third site without good reason.

> [Steps on soapbox...]
> I am plenty capable of coding everything I want or need in elisp and
> have been doing so for more than 20 years. My inputs to the vm project
> are sprinkled across a number of core files. I always try to give back
> more in code and bug fixes than I request.
> 
> If you don't want my help in contributing further, I am happy to keep my
> bug identifications, bug fixes and contributions to myself. I happen to
> have spent a fair bit of time working on this and other bugs this week
> and don't appreciate being publicly told that such conversations are
> unhelpful. I didn't just complain about bugs or ask for help or new
> features -- but went and debugged them myself and wrote the code to fix
> or extend things. 
> 
> Certainly this thread would have been shorter had I taken the approach
> of almost all other posters in terms of just complaining about bugs and
> leaving it to others to fix. If that is what you want, I would be happy
> to oblige and let you do all the work. But then don't be surprised when
> you don't find others stepping up to volunteer to help you as
> maintainers.
> 
> [Steps off soapbox...]

There is no problem with you reporting bugs, and also no problem with 
you contributing fixes.  The only problem arises if you are adamant 
that the fixes should be implemented your way.  Even more so if your 
fixes fail my testing.

If you provide solutions to problems that *other* people are facing, 
you will definitely receive their thanks.  I regularly do.

If you provide solutions that reduce the amount of time I have to 
spend on solving problems, you will definitely receive my thanks. 
But, if you prolong the time I have to spend on problems, it is 
difficult to be thankful for that!

> Sorry - but your note really struck me the wrong way... such comments
> are usually reserved for trolls, OT threads, rants, or people who
> just plain complain without contributing.

I admit to having been irritated.  But this was because I got the 
impression that you weren't treating the issue with as much care as I 
was putting into it.

Cheers,
Uday

blueman wrote:
> 
> OK - I found your revisions.

Actually, you didn't.  These were the old revisions that I had made 
before you even told us that you were interested in the 
vm-pipe-messages-to-command (the plural version of the operation).

I uploaded the current revision this morning.  I had intended to wait 
for your feedback before I uploaded the changes.  But...

> The way you implemented it is really just a kludge which adds an
> additional unnecessary variable vm-message-include-separators. 

You seem to love to argue about code.  I don't.  The code I like is 
simple, direct and clean.  It has a manual that is short, clear and 
understandable.  It has as few features as possible to serve the 
function it is designed to do, but these features would be as powerful 
as possible without being complex.  Unfortunately, VM is already well 
past all of these things.  I am only trying not to make it any worse 
than it is.

If I had my own way, I would never had added the plural version of the 
command (which has 3 versions) or its associated variables (2 of 
them).  I would have made the changes that you find in revision 664, 
which includes just this one variable vm-message-includes-separators 
and its effect on vm-pipe-message-to-command.

My reasoning would be as follows.  The official definition of a 
"message" includes headers and body, but not the separators.  But, 
apparently, some external applications want the separators as well. 
To allow the users to use those applications, I would provide a 
variable that includes separators along with the messages.  The users 
that need to use those applications would set that variable, either in 
their .vm file or in their own user-defined functions, and the job is 
done.  End of story.

If you still want to maintain that this is a "kludge", explain your 
reasoning.

----

But I am not proposing to delete your favourite functions and 
variables.  They were already there when I started and they will 
continue to be there.  I am only trying not to make it any worse than 
it is.  I don't have a plan to clean up VM.  It is too big a program 
for that.

----

One final remark.  You have mentioned that I was engaging in some kind 
of a public rebuke.  But it seems to me that you are one that is 
repeatedly choosing public fora to make your points.  People that are 
focused on solving problems are quite happy to report them on 
launchpad or send us a bug report, get the problems fixed and go on 
with their lives.  What is it that you are trying to do?

Cheers,
Uday

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
> blueman wrote:
> The launchpad bug reports are public, but email addresses are kept
> private.  People can also flag their issues as security issues if they
> don't want them to be publicly visible.  I don't mind moving to
> bugzilla if we find launchpad bugs wanting.  But, at the moment, we
> are already distributed over two sites (launchpad and savannah) and I
> wouldn't want to add a third site without good reason.

That's reasonable - I think it is just that as an old-timer, I have been
in the habit of discussing bugs here (as per the README). That is the
way I interacted first with Kyle and then with Robert going back 20
years, though admittedly I have been quiet for the past two, mostly
because my News provider didn't provide these groups for a while.

> There is no reason why you shouldn't register on launchpad.
Agreed.  I was not familiar with launchpad and didn't realize until now that
it provides a reasonable bug tracking interaction facility 

Still it is a bit confusing to me to have two places where bugs are
discussed/submitted. It's not clear to me what the advantages are of
having two places to discuss bugs or even when to use which place.

> There is no problem with you reporting bugs, and also no problem with
> you contributing fixes.  The only problem arises if you are adamant
> that the fixes should be implemented your way.  Even more so if your
> fixes fail my testing.

I am not adamant that they are fixed my way -- just that they are fixed
and that the code is clean (see my other posting). If you followed this
thread, you can see that multiple times, I threw out my code and started
again based on suggestions by Julian and you.

I think part of the problem here is that you are trying a bit too hard
to be the perfect maintainer by trying to do and fix everything
yourself. I say this in a complimentary and constructive way. You don't
need to fix everything yourself right away if you can't get my patches
to work -- this code has been bad for years and a few more days of
waiting won't kill anybody - particularly since I am probably the only
person even using the optional variables that break the function.

I will be happy to provide proper patches in the future, though I find
that a PITA since I usually just patch functions rather than
files (Perhaps we could come up with an emacs/vm command that would
generate patches based on functions rather than files -- that would be
very usefult to me). But please if I am taking the lead in fixing a bug,
don't go off and fork your own fixes unless I fail to deliver in a
reasonable enough amount of time (and for a fix like this 2 days is not
a reasonable amount of time). I believe that this is what caused the
confusion is that you started your own branch of fixing it indepedently
when you failed to get my changes to work the first time.

> I admit to having been irritated.  But this was because I got the
> impression that you weren't treating the issue with as much care as I
> was putting into it.

That is unfortunately a mistaken impression.

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
> This is not helpful.  I have tried the "two patches you submitted",
> found problems, and sent you back the corrections.  Did you apply them
> and test them?  If you didn't do so, the further exchange didn't make
> any contribution.  In fact, it wasn't clear if we were even talking
> about the same things any more.

I didn't see the corrections you sent me and didn't see you asking me
to test them. The only "patch" I saw was basically the same one that I
suggested except that:
- You added the additional line which I don't think is necessary
      -	(command (process-command process)))
      +	(process-command (process-command process)))
- You left out the other patched line that I have mentioned MULTIPLE
  TIMES -- adding a comma before ,wait
  IT WILL CRASH WITHOUT THAT CHANGE.

> (My apologies for having misremembered that I committed the changes to
> launchpad.  I posted the patches here and was waiting to hear from
> you.  I have now uploaded my version to launchpad so that you can test
> it properly.)

As I mentioned before, in addition to not knowing that you had committed
the patcheds launchpad, I was using the wrong launchpad site based on
your posting from November
     From: Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk>
     Subject: Towards VM 8.1
     Newsgroups: gnu.emacs.vm.info
     Date: Tue, 17 Nov 2009 17:46:38 +0000
     Organization: Janet Usenet Reading Service.

     Folks, bad news and good news.

     I haven't gotten a response from Rob F to my email messages.  He may yet
     come back, but we need to proceed on the assumption that he will not be
     available for some time.

     At the same time, I have been able to create a branch at Launchpad for
     VM 8.1.  The URL is

       http://bazaar.launchpad.net/~reddyuday/viewmail/8.1

As a newbie to launchpad, I had no idea that there was another VM
launchpad site at:
      http://bazaar.launchpad.net/~vm/vm/trunk/files/head%3A/lisp/

Personally, I find launchpad awkward and confusing but I will try to use
it if that is what the team is using.

Anyway let's stop this back-and-forth and get back to good coding. I
think there is enough blame to go around and we can each take away
lessons for the future from this interchange.

Let's start on a clean slate here and hopefully let's avoid any flame
wars or recriminations. I think we are both just trying to improve this
great program.

Here is a patch against your latest *668* revision that cleans things up a
bit more. In particular,

    - It removes the confusing/redundant use of the 2nd argument command
      (since it is available via process-command). 

    - Also, it removes the duplicate and potentially interfering
      functionality created by the unnecessary variable
      vm-message-includes-separators. Instead, this functionality can be
      accessed using 4 prefix arguments which is 100% consistent usage
      with the other flavors of addressing message parts in
      vm-pipe-message-part and vm-pipes-message-to-command.

      Note, I also added code in vm-pipe-messages-to-command that allows
      the 4 prefixes to override the separate start/end separators
      rather than conflicting with them (as is the case in your code)

If I should be submitting patches instead to launchpad please tell me
where and how to do that.

If the patch doesn't work or you have suggestions for improvement,
please let me know.

--- vm-save.el	2010-01-08 10:27:45.742590014 -0500
+++ vm-save.el.new	2010-01-08 11:47:32.797589324 -0500
@@ -556,7 +556,7 @@
 	 (list (vm-headers-of m) (vm-text-of m)))
 	((equal prefix-arg '(64))
 	 (list (vm-vheaders-of m) (vm-text-end-of m)))
-	(vm-message-includes-separators
+	((equal prefix-arg '(256))
 	 (list (vm-start-of m) (vm-end-of m)))
 	(t 
 	 (list (vm-headers-of m) (vm-text-end-of m)))))
@@ -571,6 +571,7 @@
 With two \\[universal-argument]'s the header portion of the message is used.
 With three \\[universal-argument]'s the visible header portion of the message
 plus the text portion is used.
+With four \\[universal-argument]'s the entire message plus separators is used
 
 When invoked on marked messages (via vm-next-command-uses-marks),
 each marked message is successively piped to the shell command,
@@ -642,33 +643,33 @@
 	   current-prefix-arg)))
   (vm-pipe-message-to-command command prefix-arg t))
 
-(defun vm-pipe-command-exit-handler (process command discard-output 
+(defun vm-pipe-command-exit-handler (process discard-output 
 					     &optional exit-handler)
-"Switch to output buffer of PROCESS that ran COMMAND, if
-DISCARD-OUTPUT non-nil.  
-If non-nil call EXIT-HANDLER with the two arguments COMMAND and OUTPUT-BUFFER." 
+  "Switch to output buffer of PROCESS if DISCARD-OUTPUT non-nil.
+If non-nil call EXIT-HANDLER with the command associated with the process
+ as the first argument and OUTPUT-BUFFER as the second argument." 
   (let ((exit-code (process-exit-status process))
 	(buffer (process-buffer process))
-	(process-command (process-command process)))
+	(command (process-command process)))
   (if (not (zerop exit-code))
       (message "Command '%s' exit code is %d." command exit-code))
   (vm-display nil nil '(vm-pipe-message-to-command)
 	      '(vm-pipe-message-to-command))
   (vm-switch-to-command-output-buffer command buffer discard-output)
   (if exit-handler
-      (funcall exit-handler process-command buffer))))
+      (funcall exit-handler command buffer))))
 
 (defvar vm-pipe-messages-to-command-start t
   "*The string to be used as the leading message separator by
 `vm-pipe-messages-to-command' at the beginning of each message.
 If set to 't', then use the leading message separator stored in the VM
-folder.")
+folder. If nil then skip insertion.")
 
 (defvar vm-pipe-messages-to-command-end t
   "*The string to be used as the trailing message separator by
 `vm-pipe-messages-to-command' at the end of each message.
 If set to 't', then use the trailing message separator stored in the VM
-folder.")
+folder. If nil then skip insertion")
 
 ;;;###autoload
 (defun vm-pipe-messages-to-command (command &optional prefix-arg 
@@ -680,8 +681,14 @@
 is much faster than calling the command on each message.  This is
 more like saving to a pipe.
 
-Before a message it will insert `vm-pipe-messages-to-command-start'
-and after a message `vm-pipe-messages-to-command-end'.
+Note that the interpretation of \\[universal-argument]'s is the same as for
+`vm-pipe-message-to-command'
+
+Before and after piping each message, it will add separators based on 
+the value of the variables `vm-pipe-messages-to-command-start'
+and `vm-pipe-messages-to-command-end', respectively. 
+\(These variables are ignored if four \\[universal-argument]'s are used so as
+to avoid duplication of separators.\)
 
 Output, if any, is displayed unless DISCARD-OUTPUT is t.
 
@@ -721,7 +728,7 @@
 	(if (eq 'exit status)
 	    (if ,no-wait
 		(vm-pipe-command-exit-handler 
-		 process ,command ,discard-output 
+		 process ,discard-output 
 		 (if (and ,no-wait (functionp ,no-wait))
 		     ,no-wait)))
 	  (message "Command '%s' changed state to %s."
@@ -731,17 +738,23 @@
       (set-buffer (vm-buffer-of m))
       (save-restriction
 	(widen)
-	(if (equal vm-pipe-messages-to-command-start t)
-	    (process-send-region process 
-				 (vm-start-of m) (vm-headers-of m))
-	  (process-send-string process vm-pipe-messages-to-command-start))
+	(cond
+	 ((or (null vm-pipe-messages-to-command-start) ; No start separator
+		  (equal current-prefix-arg '(256)))); Initial separator already inserted
+	 ((equal vm-pipe-messages-to-command-start t)
+	  (process-send-region process  ; Insert content before header
+				 (vm-start-of m) (vm-headers-of m)))
+	 (t (process-send-string process vm-pipe-messages-to-command-start)))
 	(let ((region (vm-pipe-message-part m prefix-arg)))
 	  (process-send-region process (nth 0 region) (nth 1 region)))
-	(if (equal vm-pipe-messages-to-command-end t)
-	    (process-send-region process 
-				 (vm-text-end-of m) (vm-end-of m))
-	  (process-send-string process vm-pipe-messages-to-command-end)))
-      (setq mlist (cdr mlist)))
+	(cond
+	 ((or (null vm-pipe-messages-to-command-end) ; No end separator
+		  (equal current-prefix-arg '(256)))) ; End separator already inserted
+	 ((equal vm-pipe-messages-to-command-end t)
+	  (process-send-region process  ; Insert content after end of body
+				 (vm-text-end-of m) (vm-end-of m)))
+	 ( t (process-send-string process vm-pipe-messages-to-command-end)))
+      (setq mlist (cdr mlist))))
 
     (process-send-eof process)
 
@@ -749,7 +762,7 @@
       (while (and (eq 'run (process-status process)))
 	(accept-process-output process)
 	(sit-for 0))
-      (vm-pipe-command-exit-handler process command discard-output))
+      (vm-pipe-command-exit-handler process discard-output))
     buffer))
 
 (defun vm-pipe-messages-to-command-to-string (command &optional prefix-arg)



--- vm-vars.el	2010-01-08 11:38:10.332588869 -0500
+++ vm-vars.el.new	2010-01-08 11:39:05.017589566 -0500
@@ -775,13 +775,6 @@
 
 (make-variable-buffer-local 'vm-sync-thunderbird-status)
 
-(defcustom vm-message-includes-separators
-  nil
-"*Non-nil value means that VM should include the leading and
-separator lines when it passes a message to external programs."
-  :group 'vm
-  :type 'boolean)
-
 (defcustom vm-visible-headers
   '("Resent-"
     "From:" "Sender:"

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:

> blueman wrote:
>>
>> OK - I found your revisions.
>
> Actually, you didn't.  These were the old revisions that I had made
> before you even told us that you were interested in the
> vm-pipe-messages-to-command (the plural version of the operation).
>
> I uploaded the current revision this morning.  I had intended to wait
> for your feedback before I uploaded the changes.  But...
>
>> The way you implemented it is really just a kludge which adds an
>> additional unnecessary variable vm-message-include-separators. 
>
> You seem to love to argue about code.  I don't.  The code I like is
> simple, direct and clean.  It has a manual that is short, clear and
> understandable.  It has as few features as possible to serve the
> function it is designed to do, but these features would be as powerful
> as possible without being complex.  Unfortunately, VM is already well
> past all of these things.  I am only trying not to make it any worse
> than it is.

Pot meet kettle. Let's stop making this personal.

I don't like to argue about code, but your code has several shortcomings
that go against being "short, clear, and understandable". And I don't
want any more kludgey and buggy code added to vm. In particular, your
code: 
     - Adds an unnecessary new variable and vm already has way too
       many variables.
     - Breaks the consistency of how special cases are treated -- the
       other special cases of body only, header only, visible headers only
       are treated with prefix args but you add a special case that is
       treated with a new variable for no apparant reason.
     - Results in redundant separator insertion when using
       vm-pipe-messages-to-command since if your varialbe is set and
       Robert's variables are set then the separators get inserted
       twice. This potential (and unnecessary) conflict is not even
       documented in your code.

> If I had my own way, I would never had added the plural version of the
> command (which has 3 versions) or its associated variables (2 of
> them).  I would have made the changes that you find in revision 664,
> which includes just this one variable vm-message-includes-separators
> and its effect on vm-pipe-message-to-command.
>

Well, the plural version is *much* faster than the
vm-pipe-message-to-command multiple times.

Also how else would you enable an external command to act on multiple
messages?

The whole 'culture' of vm is to allow functions to act on marked
messages as well as individual ones wherever it makes sense.

Also, vm-pipe-messages-to-command is *much* more powerful in that it
allows asynchronous calling of commands. Do you always want to wait for
a command to complete before using emacs again?

If anything, the vm-pipe-message-to-command should be eliminated or
replaced with the current version of vm-pipe-messages-to-command since
vm-pipe-messages-to-command is a true superset of the original function.

> My reasoning would be as follows.  The official definition of a
> "message" includes headers and body, but not the separators.  But,
> apparently, some external applications want the separators as well. To
> allow the users to use those applications, I would provide a variable
> that includes separators along with the messages.  The users that need
> to use those applications would set that variable, either in their .vm
> file or in their own user-defined functions, and the job is done.  End
> of story.

You said you like clean code -- well one of vm's biggest problems in my
opionion is that it has way too many variables and settings. We need to
get away from the habit of just adding more... Plus in the context of a
single message, adding the separators is just really another view like
the one for piping visible headers vs. all the headers (and we don't
have a variable to toggle between piping all vs. visible headers)

Clean code means consistent usage...

>
> If you still want to maintain that this is a "kludge", explain your
> reasoning.

Explained as above.... And again, I am not trying to attack you or the
code. I truly believe as explained above that your code is messy and
will lead to confusion and wrong results (e.g., duplicate separators).\
>
> ----
>
> But I am not proposing to delete your favourite functions and
> variables.  They were already there when I started and they will
> continue to be there.  I am only trying not to make it any worse than
> it is.  I don't have a plan to clean up VM.  It is too big a program
> for that.

Agreed. And I agree that there are too many redundant commands here.

I would *seriously* considering replacing vm-pipe-message-to-command
with the much more powerful, extensible and compatible
vm-pipe-messages-to-command. Such a replacement would be transparent to
users of the original vm-pipe-message-to-command code.

The ability to act on multiple messages and to pipe asynchronously is *awesome*

The other vesions are relatively consistent with standard emacs process
usage where one is given the option to send the output to a string
rather than a buffer
>
> ----
>
> One final remark.  You have mentioned that I was engaging in some kind
> of a public rebuke.  But it seems to me that you are one that is
> repeatedly choosing public fora to make your points.  People that are
> focused on solving problems are quite happy to report them on
> launchpad or send us a bug report, get the problems fixed and go on
> with their lives.  What is it that you are trying to do?
>
Please stop the ad-hominem. You know well that I am trying to get the
code fixed properly and that I have incorporated all of your inputs and
Julian's in the code -- going so far as to rewrite it multiple times to
accommodate your valid inputs. This thread would be a lot shorter if you
stopped trying to argue with me rather than looking at the code I keep
providing. If you look back on this thread, you will see that you have
provided perhaps one piece of code (which was buggy) while almost all of
my posts have been code-related.

Also, I have already said that in the future I am happy to use launchpad but I
was not aware of it when this thread started.

> Cheers,
> Uday
Cheers and *please* stop trying to argue with me and let's focus on
getting the code right!!!

blueman wrote:
> Let's start on a clean slate here and hopefully let's avoid any flame
> wars or recriminations. I think we are both just trying to improve this
> great program.

That is starting on a kind note, but I don't see much different in 
what you are saying.  You are not asking for a bug fix.  You are 
simply asking me to change the program to suit your taste.  If the 
taste is so important to you, then there is only one way to achieve 
it.  You have to join the VM development team and take 
*responsibility* for maintaining the code.  Then you are welcome to 
change it according to your taste.  Even then, we might debate issues 
but your taste will come with a burden of responsibility which will 
give it weight.  What you are doing now is basically back seat driving.

> Here is a patch against your latest *668* revision that cleans things up a
> bit more. In particular,
> 
>     - It removes the confusing/redundant use of the 2nd argument command
>       (since it is available via process-command). 

It is not redundant.  process-command returns a list of strings, and 
the exit handler wants a string. The types don't match.  You haven't 
yet said whether you are proposing a patch that has been tested!

>     - Also, it removes the duplicate and potentially interfering
>       functionality created by the unnecessary variable
>       vm-message-includes-separators. Instead, this functionality can be
>       accessed using 4 prefix arguments which is 100% consistent usage
>       with the other flavors of addressing message parts in
>       vm-pipe-message-part and vm-pipes-message-to-command.

So, to run a command on a bunch of marked messages with separators 
included, one must type

   M n C-u C-u C-u C-u | |

and, if one makes a mistake anywhere, one must go back and type the 
whole thing all over again!  I don't see that as a good design.  And, 
what if I am always interested in applications that want separators. 
Am I for ever doomed to the fate of typing C-u's?

In any case, this doesn't affect you.  You are interested in the 
plural version of the operation, and this affects the singular version 
of the operation.  So, why don't you let go?

>       Note, I also added code in vm-pipe-messages-to-command that allows
>       the 4 prefixes to override the separate start/end separators
>       rather than conflicting with them (as is the case in your code)

That is a good point.  I will change the plural version of the 
operation to ignore the vm-message-includes-separators variable.

> If I should be submitting patches instead to launchpad please tell me
> where and how to do that.

The way you have formulated the patch this time is perfectly fine and 
usable.

> Well, the plural version is *much* faster than the
> vm-pipe-message-to-command multiple times.
> 
> Also how else would you enable an external command to act on multiple
> messages?

You can run 'cat | whatever' with vm-pipe-message-to-command.

> The whole 'culture' of vm is to allow functions to act on marked
> messages as well as individual ones wherever it makes sense.

Except that marked messages only work with keyboard commands, not M-x 
commands.  This a serious flaw in the design of VM.  Marked messages 
are a very powerful tool, but I don't use them much because they are a 
pain to use.  If I have time, I might implement some kind of a 
selection mechanism that is easier to use.

> Also, vm-pipe-messages-to-command is *much* more powerful in that it
> allows asynchronous calling of commands. Do you always want to wait for
> a command to complete before using emacs again?

I have told you multiple times that vm-pipe-messages-to-command is not 
working on Windows Cygwin.  It hangs, and the whole Emacs freezes. 
Unless this is caused by some bug in Windows or Cygwin, this would be 
quite unacceptable.  I will test it on Linux when I get a chance.

>> My reasoning would be as follows.  The official definition of a
>> > "message" includes headers and body, but not the separators.  But,
>> > apparently, some external applications want the separators as well. To
>> > allow the users to use those applications, I would provide a variable
>> > that includes separators along with the messages.  The users that need
>> > to use those applications would set that variable, either in their .vm
>> > file or in their own user-defined functions, and the job is done.  End
>> > of story.
> 
> You said you like clean code -- well one of vm's biggest problems in my
> opionion is that it has way too many variables and settings. We need to
> get away from the habit of just adding more... Plus in the context of a
> single message, adding the separators is just really another view like
> the one for piping visible headers vs. all the headers (and we don't
> have a variable to toggle between piping all vs. visible headers)
> 
> Clean code means consistent usage...

This is not what I asked for.  You labeled my solution as a "kludge" 
which means that it is clumsy and inelegant.  I have given you what is 
in my opinion a clean explanation for it.  You haven't pointed out 
what is clumsy or inelegant about it.

Minimizing the number of variable settings is a separate issue.  In 
this particular case, I think the variable is a reasonable one because 
it describes what is meant by a "message" in the user's environment. 
Further, I don't want to force the user to type 4 prefix arguments in 
the middle of a compulsory keyboard command.  Usability takes priority 
over all other considerations.

In the interest of minimizing the number of variable, why don't you 
give up the clumsily named vm-pipe-messages-to-command-start/end 
variables and use the new variable in their place?  I have tried to 
talk you into it a few days ago and you refused.

> And I agree that there are too many redundant commands here.
> 
> I would *seriously* considering replacing vm-pipe-message-to-command
> with the much more powerful, extensible and compatible
> vm-pipe-messages-to-command. Such a replacement would be transparent to
> users of the original vm-pipe-message-to-command code.
> 
> The ability to act on multiple messages and to pipe asynchronously is *awesome*

I don't mind replacing the old operations by the new ones if you can 
convince me that the new ones actually work.  Give me a version of the 
operations that will work on Windows Cygwin running cat.  That would 
be a good start.

> Also, I have already said that in the future I am happy to use launchpad but I
> was not aware of it when this thread started.

Please do.  That will work a lot better.

Cheers,
Uday

Uday S Reddy wrote:

>> Well, the plural version is *much* faster than the
>> vm-pipe-message-to-command multiple times.
>>
>> Also how else would you enable an external command to act on multiple
>> messages?
> 
> You can run 'cat | whatever' with vm-pipe-message-to-command.

Sorry, that doesn't make sense.  What I had in mind is that one can 
concatenate the messages inside Emacs and send that to the command 
process.  This doesn't necessarily require asynchrony.

Cheers,
Uday

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:

> blueman wrote:
> It is not redundant.  process-command returns a list of strings, and
> the exit handler wants a string. The types don't match.  You haven't
> yet said whether you are proposing a patch that has been tested!

My patch has been tested and works on multiple different types of
commands. If you have an example that doesn't work, please post it so I
can replicate it...

>
>>     - Also, it removes the duplicate and potentially interfering
>>       functionality created by the unnecessary variable
>>       vm-message-includes-separators. Instead, this functionality can be
>>       accessed using 4 prefix arguments which is 100% consistent usage
>>       with the other flavors of addressing message parts in
>>       vm-pipe-message-part and vm-pipes-message-to-command.
>
> So, to run a command on a bunch of marked messages with separators
> included, one must type
>
>   M n C-u C-u C-u C-u | |
>
> and, if one makes a mistake anywhere, one must go back and type the
> whole thing all over again!  I don't see that as a good design.  And,
> what if I am always interested in applications that want
> separators. Am I for ever doomed to the fate of typing C-u's?

Why is 3 C-u good for showing virtual headers?
And adding a 4th C-u is not very hard since your fingers are presumably
already on the key.

Plus using the separator is really as Julian suggested (and you
confirmed) a deviant case since when piping single messages, the
separator really has no use or meaning. To quote Julian:

      The initial From line is NOT part of the RFC standard, nor is it
      part of the message. It's an obsolete Unix mbox hack that should
      have been killed off decades ago - the mbox format is patently
      unsuitable, leading to abominations like From-stuffing.  I've had
      immense trouble with very broken mail systems (procmail) that
      insist on putting the From in where it has no business - it caused
      standards-compliant mail filters to break, because they saw a From
      instead of a proper message.

Even I (who is the one who requested the From_ header) don't have a use
case for it in the case of individual messages. I only need it when
piping multiple messages.

And if you truly need a separator on a single message, then use
vm-pipe-messages to command on a single message with the
vm-pipe-messages-to-command-start/end variables set to their default
't'. Or if you insist on non-standard message definition then just use 4
C-u's if you prefer.

>
> In any case, this doesn't affect you.  You are interested in the
> plural version of the operation, and this affects the singular version
> of the operation.  So, why don't you let go?

But it *does* effect the plural version since the plural version calls
the same routine. So if you set your variable for use in the single
message routine, then it will *mess* up the multi-message routine.  So
you will need to continually toggle on-and-off this variable. This is
ridiculous and will inevitably lead to errors.

Thus your "fix" for the single message case, *breaks* the multi-message
case!!!!


I don't know how many times I have to say this!!!!!


>
>>       Note, I also added code in vm-pipe-messages-to-command that allows
>>       the 4 prefixes to override the separate start/end separators
>>       rather than conflicting with them (as is the case in your code)
>
> That is a good point.  I will change the plural version of the
> operation to ignore the vm-message-includes-separators variable.

I already did that in a clean way in my patch. Please look at it.

>> Also how else would you enable an external command to act on multiple
>> messages?
>
> You can run 'cat | whatever' with vm-pipe-message-to-command.

I don't understand. You are just catting a single message.

>> The whole 'culture' of vm is to allow functions to act on marked
>> messages as well as individual ones wherever it makes sense.
>
> Except that marked messages only work with keyboard commands, not M-x 
> commands.  This a serious flaw in the design of VM.  Marked messages
> are a very powerful tool, but I don't use them much because they are a
> pain to use.  If I have time, I might implement some kind of a
> selection mechanism that is easier to use.

True - but vm-pipe-message-to-command is bound to a keyboard
command. And marked messages are *very powerful* even if you don't use
them. I don't find them to be a pain at all but if you do, then don't
use them, but I can assure you that many users at least historically
have found marks extremely useful if not indispensible.

>
>> Also, vm-pipe-messages-to-command is *much* more powerful in that it
>> allows asynchronous calling of commands. Do you always want to wait for
>> a command to complete before using emacs again?
>
> I have told you multiple times that vm-pipe-messages-to-command is not
> working on Windows Cygwin.  It hangs, and the whole Emacs
> freezes. Unless this is caused by some bug in Windows or Cygwin, this
> would be quite unacceptable.  I will test it on Linux when I get a
> chance.

I just tested it on Windows Cygwin and it works just fine. So perhaps
there is something wrong with your setup. 

How about providing more info than "doesn't work"? You are always
asking others to submit detailed reports, perhaps you could do the
same. It would be much easier than me trying to convince you that it works.

>>> My reasoning would be as follows.  The official definition of a
>>> > "message" includes headers and body, but not the separators.  But,
>>> > apparently, some external applications want the separators as well. To
>>> > allow the users to use those applications, I would provide a variable
>>> > that includes separators along with the messages.  The users that need
>>> > to use those applications would set that variable, either in their .vm
>>> > file or in their own user-defined functions, and the job is done.  End
>>> > of story.
>>
>> You said you like clean code -- well one of vm's biggest problems in my
>> opionion is that it has way too many variables and settings. We need to
>> get away from the habit of just adding more... Plus in the context of a
>> single message, adding the separators is just really another view like
>> the one for piping visible headers vs. all the headers (and we don't
>> have a variable to toggle between piping all vs. visible headers)
>>
>> Clean code means consistent usage...
>
> This is not what I asked for.  You labeled my solution as a "kludge"
> which means that it is clumsy and inelegant.  I have given you what is
> in my opinion a clean explanation for it.  You haven't pointed out
> what is clumsy or inelegant about it.

1. It adds an unnecessary variable
2. It is inconsistent with the rest of usage of vm-pipe-message-part
3. As per Julian, including the separator when using a single message is
   a deviant case. At least as deviant as defining visible headers as a
   type of headers. Since visible headers uses a C-u relative to headers,
   why shouldn't your special case also use a C-u. Why should we be
   adding a variable to support a rare (and wrong) special case when the
   functionality is available and more appropriate to
   vm-pipe-messages-to-command where having separators makes sense.
4. It requires another change to vm-pipe-messages-to-command to prevent
   your change from breaking that command
>
> Minimizing the number of variable settings is a separate issue.  In
> this particular case, I think the variable is a reasonable one because
> it describes what is meant by a "message" in the user's
> environment. Further, I don't want to force the user to type 4 prefix
> arguments in the middle of a compulsory keyboard command.  Usability
> takes priority over all other considerations.

So do the same for visible headers instead of C-u C-u C-u since a
variable would then describe the meaning of headers.
My point is that adding an arbitrary variable for one modification and
not for another exactly analogous one is precisely the definition of a
kludge.
>
>
>> Also, I have already said that in the future I am happy to use launchpad but I
>> was not aware of it when this thread started.
>
> Please do.  That will work a lot better.
>

Just so you know, I twice completely reworked my code in response first
to Julian's objection that my definition of message is non-standard and
second in response to your objection that my code was "too clever". Yet
you have claimed that I am too fixated on code. However, you seem to be
the one who is unwilling to change your approach despite what I believe
to be reasonable objections, alternatives, and new code!!!

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:

> Uday S Reddy wrote:
>
>>> Well, the plural version is *much* faster than the
>>> vm-pipe-message-to-command multiple times.
>>>
>>> Also how else would you enable an external command to act on multiple
>>> messages?
>>
>> You can run 'cat | whatever' with vm-pipe-message-to-command.
>
> Sorry, that doesn't make sense.  What I had in mind is that one can
> concatenate the messages inside Emacs and send that to the command
> process.  This doesn't necessarily require asynchrony.
>
> Cheers,
> Uday

What single command does that for headers? for bodies? for whole
messages?

- Presumably I would have to first find or write a command that would
  concatenate messages, bodies, or headers to a new buffer based on the
  prefix (which could take up a lot of added buffer room if you run this
  on your whole Inbox)

- Then I would have to make sure that functionality is there to take
  care of separators.

- Then I would have to do something like call process on it and write
  handlers to do this asynchronously properly....

Or I could just use the existing command vm-pipe-messages-to-command
that does this ;)

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
> blueman wrote:
>> Let's start on a clean slate here and hopefully let's avoid any flame
>> wars or recriminations. I think we are both just trying to improve this
>> great program.
>
> That is starting on a kind note, but I don't see much different in
> what you are saying.  You are not asking for a bug fix.  You are
> simply asking me to change the program to suit your taste.  If the
> taste is so important to you, then there is only one way to achieve
> it.  You have to join the VM development team and take
> *responsibility* for maintaining the code.  Then you are welcome to
> change it according to your taste.  Even then, we might debate issues
> but your taste will come with a burden of responsibility which will
> give it weight.  What you are doing now is basically back seat
> driving.
>
I didn't ask you to change it to fit my taste.
- I found bugs
- I corrected them
- I made suggestions for extensions that make the function work better
- I coded the changes
- I spend a lot of time re-coding the changes multiple times to take
  into account other inputs 
- You complain that my code doesn't work for you but you don't supply
  any details
- You go off on your own and code your own change that has multiple
  deficiencies that I have pointed out
- Then you dig in your heels and are unwilling to accept any criticisms
  of your code -- even though you weren't even interested in these
  extensions in the first place

Such behavior doesn't exactly encourage one to join a development team.

blueman wrote:

> 
> My patch has been tested and works on multiple different types of
> commands. If you have an example that doesn't work, please post it so I
> can replicate it...

Ok, thanks for sharing that.  If you are going to act as the developer 
for now, then here it goes.

The behaviour I am getting is basically that whenever I mark multiple 
messages and run `|s' with cat as the command, Emacs freezes.  The 
only way to recover is to forcibly kill the cat process in the task 
manager.  Then I see messages like:

     Next command uses marks...
     vm-pipe-messages-to-command: writing to process:
     CCL: Quited., cat

     Next command uses marks...
     vm-pipe-messages-to-command: writing to process: invalid 
argument, cat

in the *Messages* buffer.  It doesn't seem to matter what messages I 
try it on.

When I try it on a single message, it often works.

This is the behaviour I see with my patched version which is on 
launchpad, as well as the patches you posted today.

If you want me to post something that will help diagnose the problem, 
then you need to tell me what you want to send and where to send it to.

The other problem that I saw with your original patch, to do with the 
'command' variable, didn't arise today.  But I haven't checked closely 
to see whether the problem code was getting exercised or not.

---

There is some merit in the rest of the arguments in your message that 
I will think about.

I am not 'fixated' on my code.  If it is faulty, I am quite happy to 
retract it.  It happens all the time!  But I think this message was 
the first instance where you have talked about anything going wrong. 
So, thanks for getting to the point!

Cheers,
Uday

blueman wrote:

> I didn't ask you to change it to fit my taste.
> - I found bugs
> - I corrected them
> - I made suggestions for extensions that make the function work better
> - I coded the changes
> - I spend a lot of time re-coding the changes multiple times to take
>   into account other inputs 
> - You complain that my code doesn't work for you but you don't supply
>   any details
> - You go off on your own and code your own change that has multiple
>   deficiencies that I have pointed out
> - Then you dig in your heels and are unwilling to accept any criticisms
>   of your code -- even though you weren't even interested in these
>   extensions in the first place

I don't think that is an accurate characterization of what happened. 
You sent in a patch.  When I tested it, it didn't work.  I found the 
problems and fixed them.  I posted the fixes.  You didn't pay 
attention to the fixes.  Then we had a discussion about procedural 
issues which didn't get anywhere.

If you sent me correct patches to start with, or if you paid attention 
to the fixes that I needed to incorporate, we could have probably 
avoided the procedural discussion.

But your version and my version are doing the same thing for the 
function that you are interested in.  So, I don't understand what you 
feel aggrieved about!

> Such behavior doesn't exactly encourage one to join a development team.

I am not "encouraging" you to join the development team.  It is your 
choice.  If you want your decisions to prevail, then you have to take 
responsibility.  If you want to be the driver, you have to sit in 
front of the steering wheel.  There are no two ways about it!

Cheers,
Uday

blueman wrote:

>     - It removes the confusing/redundant use of the 2nd argument command
>       (since it is available via process-command). 

I have looked into this now.  process-command returns a list of 
strings, in my examples something like '("sh.exe" "-cf" "cat").  But 
in messages, you just want to print "cat".  The 'command' argument is 
the string that is getting printed in messages.  I don't see any other 
place where the command argument is getting used.

So, it is not a big deal, but it is certainly not redundant.

Cheers,
Uday

blueman wrote:

> 
> Plus using the separator is really as Julian suggested (and you
> confirmed) a deviant case since when piping single messages, the
> separator really has no use or meaning. To quote Julian:
> 
>       The initial From line is NOT part of the RFC standard, nor is it
>       part of the message. It's an obsolete Unix mbox hack that should
>       have been killed off decades ago - the mbox format is patently
>       unsuitable, leading to abominations like From-stuffing.  I've had
>       immense trouble with very broken mail systems (procmail) that
>       insist on putting the From in where it has no business - it caused
>       standards-compliant mail filters to break, because they saw a From
>       instead of a proper message.
> 
> Even I (who is the one who requested the From_ header) don't have a use
> case for it in the case of individual messages. I only need it when
> piping multiple messages.

Ok, I am removing the original addition to the singular version of the 
command since it doesn't seem to be useful.  There will be no 4-prefix 
version of the command for the same reason.

It is my understanding that the plural version of the command in 
launchpad satisfies all your needs.  If not, please let me know.

Cheers,
Uday

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
> blueman wrote:
>
>>
>> My patch has been tested and works on multiple different types of
>> commands. If you have an example that doesn't work, please post it so I
>> can replicate it...
>
> Ok, thanks for sharing that.  If you are going to act as the developer
> for now, then here it goes.

Thanks - I really do want to help

>
> The behaviour I am getting is basically that whenever I mark multiple
> messages and run `|s' with cat as the command, Emacs freezes.  The
> only way to recover is to forcibly kill the cat process in the task
> manager.  Then I see messages like:
>
>     Next command uses marks...
>     vm-pipe-messages-to-command: writing to process:
>     CCL: Quited., cat
>
>     Next command uses marks...
>     vm-pipe-messages-to-command: writing to process: invalid argument,
> cat
>
> in the *Messages* buffer.  It doesn't seem to matter what messages I
> try it on.
>
> When I try it on a single message, it often works.

This is interesting. Because for me "|s" with cat worked even before my
patches. It only failed when I tried either to set the optional variable
DISCARD-OUTPUT or when I tried to use it asynchronously by setting the
NO-WAIT exit handler. These problems were due to a combination of the
"missing" command argument and the missing comma in front of no-wait --
all of which only surfaced when the optional  variables were set to
non-nil.

Now it seems like you are having problems with the command itself. This
is actually good since it elminates the complexity of the problem being
with some funky exit handler you were writing ;)

> This is the behaviour I see with my patched version which is on
> launchpad, as well as the patches you posted today.
>
> If you want me to post something that will help diagnose the problem,
> then you need to tell me what you want to send and where to send it
> to.

If you have access to Linux, I would suggest trying there to see whether
this can be attributed to a cygwin problem. I tested mine on the 1.7
version of cygwin which may still be beta and also I was using ntemacs
rather than cygwin emacs so are Windows platforms may be different.

That being said, check what the value of the variable shell-file-name
is. If you are using ntemacs then that gets set by default to 
"C:/My Programs/Emacs/bin/cmdproxy.exe" which results in a Windows
shell. I set the shell to "C:/cygwin/bin/bash.exe" and it all works
fine.

The difference between vm-pipe-message-to-command and
vm-pipe-messages-to-command is that the first one uses
"call-process-region" while the 2nd one first starts the process using
"start-process" and then sends it input.

As a test try running:
(tart-process "test" "tempbuff" shell-file-name shell-command-switch "cat > testfile"))
(list-processes)

When shell-file-name = "C:/cygwin/bin/bash.exe" the process starts
When shell-file-name = "C:/cygwin/bin/bash.exe" the process fails to start.

> The other problem that I saw with your original patch, to do with the
> command' variable, didn't arise today.  But I haven't checked closely
> to see whether the problem code was getting exercised or not.
>
> ---
>
> There is some merit in the rest of the arguments in your message that
> I will think about.
>
> I am not 'fixated' on my code.  If it is faulty, I am quite happy to
> retract it.  It happens all the time!  But I think this message was
> the first instance where you have talked about anything going
> wrong. So, thanks for getting to the point!
>
> Cheers,
> Uday

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:
> blueman wrote:
>
>>
>> Plus using the separator is really as Julian suggested (and you
>> confirmed) a deviant case since when piping single messages, the
>> separator really has no use or meaning. To quote Julian:
>>
>>       The initial From line is NOT part of the RFC standard, nor is it
>>       part of the message. It's an obsolete Unix mbox hack that should
>>       have been killed off decades ago - the mbox format is patently
>>       unsuitable, leading to abominations like From-stuffing.  I've had
>>       immense trouble with very broken mail systems (procmail) that
>>       insist on putting the From in where it has no business - it caused
>>       standards-compliant mail filters to break, because they saw a From
>>       instead of a proper message.
>>
>> Even I (who is the one who requested the From_ header) don't have a use
>> case for it in the case of individual messages. I only need it when
>> piping multiple messages.
>
> Ok, I am removing the original addition to the singular version of the
> command since it doesn't seem to be useful.  There will be no 4-prefix
> version of the command for the same reason.

Although I always like more options and wouldn't have minded the
4-prefix version, I think your decision is wise in that it avoids
confusion between multiple ways of adding separators. And since as
Julian pointed out, the separators have more to do with multiple
messages and don't really have meaning on single messages, the best way
to get separators for a single message is to use vm-pipe-messages-to-command.
Perhaps we should add to the def strings that vm-pipe-message-to-command
does not include separators and that vm-pipe-messages does.

> It is my understanding that the plural version of the command in
> launchpad satisfies all your needs.  If not, please let me know.
>
That is correct. I will send you a new patch when we figure out why it
isn't working on your setup.

blueman wrote:

> As a test try running:
> (tart-process "test" "tempbuff" shell-file-name shell-command-switch "cat > testfile"))
> (list-processes)

This worked fine.  My normal shell-file-name is sh.exe and the 
shell-command-switch is "-cf".

I don't know how to tell the version of cygwin. I often run cygwin 
setup and it updates whatever it wants to.  I seem to have run it last 
on Dec 25.  My setup.exe is dated Jul 13, 2009 and my cygwin1.dll has 
the date of Jun 12, 2008.

--

In any case, I might have made some progress in diagnosing the 
problem.  I got an inkling that the size of the message being piped 
was a factor.  So, I sorted by mbox by size and did a binary search. 
For a message of size 61682 bytes, it worked.  For size 63940, it 
hung.  So, it looks like 64K minus some delta hangs.  It could be some 
buffering issue.

I also tried piping multiple small messages.

- Three message sized 32, 33, 45: worked.
- Four messages sized 32, 33, 45, 53: hung.
- Three messages sized 33, 45, 53: hung
- Two messages sized 45, 53: worked.
- Three messages sized 53, 67, 68: worked.
- Four messages sized 53, 67, 68, 72: hung.

I don't see a pattern here.

Good luck finding the problem.  Asynchronous problems are really hard 
to debug!

Cheers,
Uday

Uday S Reddy <uDOTsDOTreddy@cs.bham.ac.uk> writes:

> blueman wrote:
> I don't know how to tell the version of cygwin. I often run cygwin
> setup and it updates whatever it wants to.  I seem to have run it last
> on Dec 25.  My setup.exe is dated Jul 13, 2009 and my cygwin1.dll has
> the date of Jun 12, 2008.

Sounds like Cygwin 1.5
You can see the version by running "uname -r"

>
> --
>
> In any case, I might have made some progress in diagnosing the
> problem.  I got an inkling that the size of the message being piped
> was a factor.  So, I sorted by mbox by size and did a binary
> search. For a message of size 61682 bytes, it worked.  For size 63940,
> it hung.  So, it looks like 64K minus some delta hangs.  It could be
> some buffering issue.
>
> I also tried piping multiple small messages.
>
> - Three message sized 32, 33, 45: worked.
> - Four messages sized 32, 33, 45, 53: hung.
> - Three messages sized 33, 45, 53: hung
> - Two messages sized 45, 53: worked.
> - Three messages sized 53, 67, 68: worked.
> - Four messages sized 53, 67, 68, 72: hung.
>
> I don't see a pattern here.
>
> Good luck finding the problem.  Asynchronous problems are really hard
> to debug!

Especially hard if I can't reproduce the error (I was wrong before about
having an error under my cygwin version -- I was giving it a
non-existant target directory). I even tried it on an older version of
Emacs with Cygwin 1.5. 

(Note I am assuming you are just running the command |s on multiple
marked messages without any exit handler etc. for now)

If you are interested in debugging (which I would understand if you are
not...), I would offer the following since I can't reproduce it yet...

1. First if you can try it on Linux so hopefully we can narrow down that
   this is just a problem for you on cygwin.

2. Why don't you run ededbud-eval-top-level-form on the function
   vm-pipe-messages-to-command to see where it crashes?


3. Alternatively, you could try to do something like the following manually
   (by evaling each of the following sexp one at a time) First fill a
   buffer with >64K of stuff. Then run sequentially:

(start-process "test" "tempbuff" shell-file-name shell-command-switch "cat > testfile"))
(process-send-region process (point-min) (point-max))
(process-send-region process (point-min) (point-max))
(process-send-region process (point-min) (point-max))
(process-send-region process (point-min) (point-max))
(process-send-region process (point-min) (point-max))
; etc. repeat and seen when/whether crashes
; also check in another window whether the file testfile is getting its data
; when/if you are convinced this is working fine, continue with the
; following code:
(process-send-eof process)
(while (eq 'run (process-status process))
   	(accept-process-output process)
       	(sit-for 0))


4. Try running in emacs -q mode with vm.el loaded manually to make sure
   none of your other settings are getting in the way. Similarly, if
   running emacs from cygwin-bash, then use a default .bashrc/.bash-profile.

5. Be as explicit as possible about *everything* you are doing. Emacs
   version, cygwin version. Exact command you are running

6. Make up a non-private folder of emails that it crashes on and send it
   to me to make sure I am not running on the exact same content.

Again, if you think this is just an issue that is particular to your
setup and you are not interested in debugging, that is OK too -- since I
have it working on multiple machines and setups here so I am reasonably
confident that the vm code itself is ok.

blueman wrote:

> Sounds like Cygwin 1.5
> You can see the version by running "uname -r"

Good guess.  It says: 1.5.25(0.156/4/2).


> 3. Alternatively, you could try to do something like the following manually
>    (by evaling each of the following sexp one at a time) First fill a
>    buffer with >64K of stuff. Then run sequentially:
> 
> (start-process "test" "tempbuff" shell-file-name shell-command-switch "cat > testfile"))
> (process-send-region process (point-min) (point-max))
> (process-send-region process (point-min) (point-max))
> (process-send-region process (point-min) (point-max))
> (process-send-region process (point-min) (point-max))
> (process-send-region process (point-min) (point-max))
> ; etc. repeat and seen when/whether crashes
> ; also check in another window whether the file testfile is getting its data
> ; when/if you are convinced this is working fine, continue with the
> ; following code:
> (process-send-eof process)
> (while (eq 'run (process-status process))
>    	(accept-process-output process)
>        	(sit-for 0))

Nothing goes wrong when I do this.  The testfile got created normally. 
  It took a while for all the stuff to go into the testfile though.

I played with some variations of it, and finally realized that the 
problem must be the handling of the output.  In your test process, the 
output is going to the testfile.  If I just use 'cat' as the command 
instead of 'cat > testfile', then sure enough the problem reappears. 
It starts hanging somewhere above 64K.

If you look up the ELisp manual section on "Receiving output from 
processes", you should be able to code this more safely.

Cheers,
Uday