unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Redirecting standard output
@ 2011-04-20 21:19 Lars Magne Ingebrigtsen
  2011-04-20 22:54 ` Stefan Monnier
  2011-04-21  5:57 ` Eli Zaretskii
  0 siblings, 2 replies; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-20 21:19 UTC (permalink / raw)
  To: emacs-devel

I was trying to play around with the pbmplus programs from Emacs, and
that turns out to be somewhat awkward.  The programs all send output to
STDOUT, and there seems to be no way to easily redirect STDOUT with
`call-process'.

You can usually work around this by using `shell-command' or the like,
but when dealing with directories that contain arbitrary characters,
getting the quoting right can be somewhat icky.  (Although I found
`shell-quote-argument' just now while writing this.  :-)

Anyway, `call-process' allows redirecting STDERR to a file, but
apparently not STDOUT:

----
Insert output in BUFFER before point; t means current buffer;
 nil for BUFFER means discard it; 0 means discard and don't wait.
BUFFER can also have the form (REAL-BUFFER STDERR-FILE); in that case,
REAL-BUFFER says what to do with standard output, as above,
while STDERR-FILE says what to do with standard error in the child.
STDERR-FILE may be nil (discard standard error output),
t (mix it with ordinary output), or a file name string.
----

(On the other side, you can't redirect STDERR to a buffer, it seems?)

Unless I've missed something here, would anybody object to me extending
`call-process' to allow redirecting STDOUT to a file, too?

I'm not sure what the syntax would be, though, since a string parameter
to REAL-BUFFER already means "use the buffer with this name"....
eurmh...  well, off the top of my head, we could use keywords for the
complex case.  That is BUFFER could be, er, something like

(:stdout (:file "/tmp/foo") :stderr (:buffer "bar"))

Backwards compatibility would be easy to maintain.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-20 21:19 Redirecting standard output Lars Magne Ingebrigtsen
@ 2011-04-20 22:54 ` Stefan Monnier
  2011-04-21  1:54   ` Lars Magne Ingebrigtsen
  2011-04-21  8:27   ` Michael Albinus
  2011-04-21  5:57 ` Eli Zaretskii
  1 sibling, 2 replies; 37+ messages in thread
From: Stefan Monnier @ 2011-04-20 22:54 UTC (permalink / raw)
  To: emacs-devel

> I'm not sure what the syntax would be, though, since a string parameter
> to REAL-BUFFER already means "use the buffer with this name"....
> eurmh...  well, off the top of my head, we could use keywords for the
> complex case.  That is BUFFER could be, er, something like

> (:stdout (:file "/tmp/foo") :stderr (:buffer "bar"))

> Backwards compatibility would be easy to maintain.

Sounds fine to me, as long as the patch is simple enough,


        Stefan



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-20 22:54 ` Stefan Monnier
@ 2011-04-21  1:54   ` Lars Magne Ingebrigtsen
  2011-04-21  6:10     ` Eli Zaretskii
  2011-04-21  8:27   ` Michael Albinus
  1 sibling, 1 reply; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-21  1:54 UTC (permalink / raw)
  To: emacs-devel

I've started reading the code, and it doesn't look too harrowing to
do -- there's a possibility it might make the code simpler, because we'd
basically be able to factor out stdout/stderr handling into one helper
function instead of special-casing the stdout/stderr handling.  I think.

But looking at the code, I remembered `start-process', which doesn't
allow separating stderr at all, so I took a look at that function, too.
It's much simpler and cleaner than `call-process'.  `call-process' has a
fair bit of #ifdef for DOS and NT and Windows, while `start-process'
doesn't.  And even stranger is that if you give a 0 as the value for
`call-process', you get asynchronous behaviour, which is very much like
the behaviour that `start-process' provides.  (But without the
possibility of adding filters.)

So here's my question: What's the reason that `call-process' isn't just
a shim around `start-process'?

Hm...  oh!  process.c isn't compiled at all on MS-DOS?  Hm.  Shame.  It
would have been nice if those two (very similar functionality-wise)
functions could have been merged somehow...

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-20 21:19 Redirecting standard output Lars Magne Ingebrigtsen
  2011-04-20 22:54 ` Stefan Monnier
@ 2011-04-21  5:57 ` Eli Zaretskii
  2011-04-21  6:28   ` Thierry Volpiatto
  2011-04-21 11:40   ` Lars Magne Ingebrigtsen
  1 sibling, 2 replies; 37+ messages in thread
From: Eli Zaretskii @ 2011-04-21  5:57 UTC (permalink / raw)
  To: Lars Magne Ingebrigtsen; +Cc: emacs-devel

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Wed, 20 Apr 2011 23:19:57 +0200
> 
> I was trying to play around with the pbmplus programs from Emacs, and
> that turns out to be somewhat awkward.  The programs all send output to
> STDOUT, and there seems to be no way to easily redirect STDOUT with
> `call-process'.

You mean, redirect to a file?  Why do you need that?

Anyway, redirecting to a temp buffer and then writing that buffer to a
file should be good enough, right?

> You can usually work around this by using `shell-command' or the like,
> but when dealing with directories that contain arbitrary characters,
> getting the quoting right can be somewhat icky.  (Although I found
> `shell-quote-argument' just now while writing this.  :-)

So, if you use shell-quote-argument, is there still a problem?



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21  1:54   ` Lars Magne Ingebrigtsen
@ 2011-04-21  6:10     ` Eli Zaretskii
  2011-04-21 11:45       ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 37+ messages in thread
From: Eli Zaretskii @ 2011-04-21  6:10 UTC (permalink / raw)
  To: Lars Magne Ingebrigtsen; +Cc: emacs-devel

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 21 Apr 2011 03:54:10 +0200
> 
> Hm...  oh!  process.c isn't compiled at all on MS-DOS?

It is, but only its last part, the one outside the "#ifdef subprocesses"
condition.  Which doesn't include start-process and its subroutines,
of course.

And I'm not sure this is the only reason why we have 2 APIs instead of
one.

> Hm.  Shame.  It would have been nice if those two (very similar
> functionality-wise) functions could have been merged somehow...

If you do it, I promise to review the patches and test them.  I myself
don't plan investing such a serious effort in such a central part of
Emacs any time soon.  I think you greatly underestimate the effort
needed here; the amount of #ifdef's in call-process should tell you
something.  (There were much more of these #ifdef's originally, but
Dan's efforts brought them to the current state, which is probably the
absolute minimum.)  Making start-process compile on MS-DOS will need
to introduce an even larger amount of #ifdef's into it, which is
hardly a step into the right direction.

And anyway, I'm not sure your conclusion about these 2 APIs being very
similar is indeed true.  Nor do I see how start-process solves your
problem: it doesn't let you separate stdout and stderr at all, unless
you go through a shell.  What am I missing?



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21  5:57 ` Eli Zaretskii
@ 2011-04-21  6:28   ` Thierry Volpiatto
  2011-04-21  6:41     ` Eli Zaretskii
  2011-04-21 11:40   ` Lars Magne Ingebrigtsen
  1 sibling, 1 reply; 37+ messages in thread
From: Thierry Volpiatto @ 2011-04-21  6:28 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
>> Date: Wed, 20 Apr 2011 23:19:57 +0200
>> 
>> I was trying to play around with the pbmplus programs from Emacs, and
>> that turns out to be somewhat awkward.  The programs all send output to
>> STDOUT, and there seems to be no way to easily redirect STDOUT with
>> `call-process'.
>
> You mean, redirect to a file?  Why do you need that?
>
> Anyway, redirecting to a temp buffer and then writing that buffer to a
> file should be good enough, right?
>
>> You can usually work around this by using `shell-command' or the like,
>> but when dealing with directories that contain arbitrary characters,
>> getting the quoting right can be somewhat icky.  (Although I found
>> `shell-quote-argument' just now while writing this.  :-)
>
> So, if you use shell-quote-argument, is there still a problem?
shell-quote-argument is sometime unusable with special characters:
(shell-quote-argument "Vidéos")
==> "Vid\\éos"

eshell:
ls (shell-quote-argument "Vidéos")
==>Vid\éos: No such file or directory



-- 
A+ Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21  6:28   ` Thierry Volpiatto
@ 2011-04-21  6:41     ` Eli Zaretskii
  2011-04-21  7:33       ` Thierry Volpiatto
  0 siblings, 1 reply; 37+ messages in thread
From: Eli Zaretskii @ 2011-04-21  6:41 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: emacs-devel

> From: Thierry Volpiatto <thierry.volpiatto@gmail.com>
> Date: Thu, 21 Apr 2011 08:28:21 +0200
> 
> > So, if you use shell-quote-argument, is there still a problem?
> shell-quote-argument is sometime unusable with special characters:
> (shell-quote-argument "Vidéos")
> ==> "Vid\\éos"
> 
> eshell:
> ls (shell-quote-argument "Vidéos")
> ==>Vid\éos: No such file or directory

Please report this as a bug.




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21  6:41     ` Eli Zaretskii
@ 2011-04-21  7:33       ` Thierry Volpiatto
  0 siblings, 0 replies; 37+ messages in thread
From: Thierry Volpiatto @ 2011-04-21  7:33 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Thierry Volpiatto <thierry.volpiatto@gmail.com>
>> Date: Thu, 21 Apr 2011 08:28:21 +0200
>> 
>> > So, if you use shell-quote-argument, is there still a problem?
>> shell-quote-argument is sometime unusable with special characters:
>> (shell-quote-argument "Vidéos")
>> ==> "Vid\\éos"
>> 
>> eshell:
>> ls (shell-quote-argument "Vidéos")
>> ==>Vid\éos: No such file or directory
>
> Please report this as a bug.
Done.

-- 
A+ Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-20 22:54 ` Stefan Monnier
  2011-04-21  1:54   ` Lars Magne Ingebrigtsen
@ 2011-04-21  8:27   ` Michael Albinus
  1 sibling, 0 replies; 37+ messages in thread
From: Michael Albinus @ 2011-04-21  8:27 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I'm not sure what the syntax would be, though, since a string parameter
>> to REAL-BUFFER already means "use the buffer with this name"....
>> eurmh...  well, off the top of my head, we could use keywords for the
>> complex case.  That is BUFFER could be, er, something like
>
>> (:stdout (:file "/tmp/foo") :stderr (:buffer "bar"))
>
>> Backwards compatibility would be easy to maintain.
>
> Sounds fine to me, as long as the patch is simple enough,

This would also require changes in Tramp, for process-file (and
start-file-process, if start-process is changed as well). Not a big
deal, just to mention it ...

>         Stefan

Best regards, Michael.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21  5:57 ` Eli Zaretskii
  2011-04-21  6:28   ` Thierry Volpiatto
@ 2011-04-21 11:40   ` Lars Magne Ingebrigtsen
  2011-04-21 11:58     ` Eli Zaretskii
  1 sibling, 1 reply; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-21 11:40 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> You mean, redirect to a file?  Why do you need that?

The pbmplus commands read from stdin and write to stdout, mostly.  So if
you want to rescale a picture, you have to say "pnmscale < foo > bar".

> Anyway, redirecting to a temp buffer and then writing that buffer to a
> file should be good enough, right?

These are huge files.  It would be kinda slow.

> So, if you use shell-quote-argument, is there still a problem?

I think putting stuff into a string and then reparsing the string is bad
programming practise.  If it can be (easily) avoided, it should be.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21  6:10     ` Eli Zaretskii
@ 2011-04-21 11:45       ` Lars Magne Ingebrigtsen
  2011-04-21 13:25         ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-21 11:45 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> Hm...  oh!  process.c isn't compiled at all on MS-DOS?
>
> It is, but only its last part, the one outside the "#ifdef subprocesses"
> condition.  Which doesn't include start-process and its subroutines,
> of course.

Right.

> And I'm not sure this is the only reason why we have 2 APIs instead of
> one.

Does anybody know what the (historical) reason for these two functions
was?  The differing function parameters has always puzzled me:

(start-process NAME BUFFER PROGRAM &rest PROGRAM-ARGS)

(call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS)

> And anyway, I'm not sure your conclusion about these 2 APIs being very
> similar is indeed true.  Nor do I see how start-process solves your
> problem: it doesn't let you separate stdout and stderr at all, unless
> you go through a shell.  What am I missing?

No, `start-process' doesn't help me at all.  I was just thinking that if
I made the change in `call-process', I should probably look at extending
the same parameters in `start-process', and then I started wondering
about why the two functions existed separately, which is when I wrote
that email.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 11:40   ` Lars Magne Ingebrigtsen
@ 2011-04-21 11:58     ` Eli Zaretskii
  2011-04-21 12:24       ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 37+ messages in thread
From: Eli Zaretskii @ 2011-04-21 11:58 UTC (permalink / raw)
  To: Lars Magne Ingebrigtsen; +Cc: emacs-devel

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 21 Apr 2011 13:40:45 +0200
> 
> The pbmplus commands read from stdin and write to stdout, mostly.  So if
> you want to rescale a picture, you have to say "pnmscale < foo > bar".
> [...]
> > So, if you use shell-quote-argument, is there still a problem?
> 
> I think putting stuff into a string and then reparsing the string is bad
> programming practise.  If it can be (easily) avoided, it should be.

Sorry, I don't follow: what string? what reparsing?

I meant to use something like this:

  (call-process shell-file-name nil nil nil
  		shell-command-switch 
		(concat "pnmscale < " 
			(shell-quote-argument "foo") " > "
			(shell-quote-argument "bar")))

Does this do what you want?



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 11:58     ` Eli Zaretskii
@ 2011-04-21 12:24       ` Lars Magne Ingebrigtsen
  2011-04-21 14:25         ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-21 12:24 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> Sorry, I don't follow: what string? what reparsing?
>
> I meant to use something like this:
>
>   (call-process shell-file-name nil nil nil
>   		shell-command-switch 
> 		(concat "pnmscale < " 
> 			(shell-quote-argument "foo") " > "
> 			(shell-quote-argument "bar")))

Here you put stuff in a string and let the shell reparse it.  I think
that, in general, if you can avoid doing that, you should, just like you
should avoid saying `(eval (read-from-string zot))', if practical.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 11:45       ` Lars Magne Ingebrigtsen
@ 2011-04-21 13:25         ` Lars Magne Ingebrigtsen
  2011-04-21 14:10           ` Eli Zaretskii
  2011-04-21 16:29           ` Glenn Morris
  0 siblings, 2 replies; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-21 13:25 UTC (permalink / raw)
  To: emacs-devel

I've spent a couple of hours reading the code in `call-process' in more
detail.  Man, that's a complicated function.  Thanks for the warning,
Eli.  :-)

Anyway, I think that redirecting STDOUT to a file would be very easy to
implement, and would be quite non-invasive.  (The code already has a
redirect-to-fd case, the /dev/null output, so there's very little code
to add.  Just open something else than /dev/null, and you're there,
almost.)

However, redirecting STDERR to a buffer seems like it would be quite
complicated, unfortunately.  I thought that having STDOUT and STDERR be
handled symmetrically would be a nice feature (for redirecting STDOUT to
one buffer and STDERR to another, for instance), but unless I'm reading
the code wrong (and I very well could be), I don't see an obvious way to
do that.

Does this assessment of the complexities involved seem accurate?

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 13:25         ` Lars Magne Ingebrigtsen
@ 2011-04-21 14:10           ` Eli Zaretskii
  2011-04-21 15:15             ` Lars Magne Ingebrigtsen
  2011-04-21 17:05             ` Jan Djärv
  2011-04-21 16:29           ` Glenn Morris
  1 sibling, 2 replies; 37+ messages in thread
From: Eli Zaretskii @ 2011-04-21 14:10 UTC (permalink / raw)
  To: Lars Magne Ingebrigtsen; +Cc: emacs-devel

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 21 Apr 2011 15:25:29 +0200
> 
> However, redirecting STDERR to a buffer seems like it would be quite
> complicated, unfortunately.  I thought that having STDOUT and STDERR be
> handled symmetrically would be a nice feature (for redirecting STDOUT to
> one buffer and STDERR to another, for instance), but unless I'm reading
> the code wrong (and I very well could be), I don't see an obvious way to
> do that.

On Posix systems, we use a pipe to read from subprocess's STDOUT.  You
cannot do the same with STDERR, but you can redirect STDERR to a
temporary file, and then read it when the subprocess exits.  You can
refactor the part of call-process that reads STDOUT and decodes it,
and reuse it for STDERR (only reading from a file).

Btw, I think the reason why no one implemented that till now is that
STDERR normally isn't interesting, unless the subprocess exits with an
error code, in which case reading the text from the temporary file to
which STDERR was redirected is easy.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 12:24       ` Lars Magne Ingebrigtsen
@ 2011-04-21 14:25         ` Lars Magne Ingebrigtsen
  0 siblings, 0 replies; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-21 14:25 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 636 bytes --]

Here's a quick stab at it.  There's some cargo-cult programming in
there, and I have to go over the error cases again to make sure there's
no FD leaks, but it'll probably look something like the patch included.

Usage is:

(call-process "echo" nil '(:file "/tmp/hello") nil "thing")

or

(call-process "echo" nil '((:file "/tmp/hello") "/tmp/error") nil "thing")

So with this, `call-process' can do all the combinations of
STDERR/STDOUT to file/buffers, except that you can't put STDERR in one
buffer and STDOUT in a different buffer.

But you can get STDERR in a buffer by itself, and STDOUT in a file, so
it's progress of a kind...


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: callproc.patch --]
[-- Type: text/x-diff, Size: 5445 bytes --]

=== modified file 'src/callproc.c'
*** src/callproc.c	2011-04-14 19:34:42 +0000
--- src/callproc.c	2011-04-21 14:17:44 +0000
***************
*** 96,101 ****
--- 96,103 ----
  /* Nonzero if this is termination due to exit.  */
  static int call_process_exited;
  
+ static Lisp_Object Qcallproc_file_symbol;
+ 
  static Lisp_Object Fgetenv_internal (Lisp_Object, Lisp_Object);
  
  static Lisp_Object
***************
*** 196,205 ****
--- 198,209 ----
    /* File to use for stderr in the child.
       t means use same as standard output.  */
    Lisp_Object error_file;
+   Lisp_Object output_file = Qnil;
  #ifdef MSDOS	/* Demacs 1.1.1 91/10/16 HIRANO Satoshi */
    char *outf, *tempfile;
    int outfilefd;
  #endif
+   int fd_output = 0;
    struct coding_system process_coding; /* coding-system of process output */
    struct coding_system argument_coding;	/* coding-system of arguments */
    /* Set to the return value of Ffind_operation_coding_system.  */
***************
*** 275,281 ****
  
        /* If BUFFER is a list, its meaning is
  	 (BUFFER-FOR-STDOUT FILE-FOR-STDERR).  */
!       if (CONSP (buffer))
  	{
  	  if (CONSP (XCDR (buffer)))
  	    {
--- 279,286 ----
  
        /* If BUFFER is a list, its meaning is
  	 (BUFFER-FOR-STDOUT FILE-FOR-STDERR).  */
!       if (CONSP (buffer) &&
! 	  ! EQ (Qcallproc_file_symbol, XCAR (buffer)))
  	{
  	  if (CONSP (XCDR (buffer)))
  	    {
***************
*** 291,296 ****
--- 296,312 ----
  	  buffer = XCAR (buffer);
  	}
  
+       /* If the buffer is (still) a list, it might be a (:file "file") spec. */
+       if (CONSP (buffer) &&
+ 	  CONSP (XCDR (buffer)) &&
+ 	  EQ (Qcallproc_file_symbol, XCAR (buffer)))
+ 	{
+ 	  output_file = Fexpand_file_name (XCAR (XCDR (buffer)),
+ 					   BVAR (current_buffer, directory));
+ 	  CHECK_STRING (output_file);
+ 	  buffer = Qnil;
+ 	}
+ 
        if (!(EQ (buffer, Qnil)
  	    || EQ (buffer, Qt)
  	    || INTEGERP (buffer)))
***************
*** 318,328 ****
       protected by the caller, so all we really have to worry about is
       buffer.  */
    {
!     struct gcpro gcpro1, gcpro2, gcpro3, gcpro4;
  
      current_dir = BVAR (current_buffer, directory);
  
!     GCPRO4 (infile, buffer, current_dir, error_file);
  
      current_dir = Funhandled_file_name_directory (current_dir);
      if (NILP (current_dir))
--- 334,344 ----
       protected by the caller, so all we really have to worry about is
       buffer.  */
    {
!     struct gcpro gcpro1, gcpro2, gcpro3, gcpro4, gcpro5;
  
      current_dir = BVAR (current_buffer, directory);
  
!     GCPRO5 (infile, buffer, current_dir, error_file, output_file);
  
      current_dir = Funhandled_file_name_directory (current_dir);
      if (NILP (current_dir))
***************
*** 342,347 ****
--- 358,365 ----
        current_dir = ENCODE_FILE (current_dir);
      if (STRINGP (error_file) && STRING_MULTIBYTE (error_file))
        error_file = ENCODE_FILE (error_file);
+     if (STRINGP (output_file) && STRING_MULTIBYTE (output_file))
+       output_file = ENCODE_FILE (output_file);
      UNGCPRO;
    }
  
***************
*** 353,358 ****
--- 371,394 ----
        infile = DECODE_FILE (infile);
        report_file_error ("Opening process input file", Fcons (infile, Qnil));
      }
+ 
+   if (STRINGP (output_file))
+     {
+ #ifdef DOS_NT
+       fd_output = emacs_open (SSDATA (output_file),
+ 			      O_WRONLY | O_TRUNC | O_CREAT | O_TEXT,
+ 			      S_IREAD | S_IWRITE);
+ #else  /* not DOS_NT */
+       fd_output = creat (SSDATA (output_file), 0666);
+ #endif /* not DOS_NT */
+       if (fd_output < 0)
+ 	{
+            output_file = DECODE_FILE (output_file);
+            report_file_error ("Opening process output file",
+ 	   Fcons (output_file, Qnil));
+         }
+     }
+ 
    /* Search for program; barf if not found.  */
    {
      struct gcpro gcpro1, gcpro2, gcpro3, gcpro4;
***************
*** 413,425 ****
    strcat (tempfile, "detmp.XXX");
    mktemp (tempfile);
  
!   outfilefd = creat (tempfile, S_IREAD | S_IWRITE);
!   if (outfilefd < 0)
      {
        emacs_close (filefd);
        report_file_error ("Opening process output file",
! 			 Fcons (build_string (tempfile), Qnil));
      }
    fd[0] = filefd;
    fd[1] = outfilefd;
  #endif /* MSDOS */
--- 449,467 ----
    strcat (tempfile, "detmp.XXX");
    mktemp (tempfile);
  
!   /* If we're redirecting STDOUT to a file, this is already opened. */
!   if (fd_output == 0)
      {
+       outfilefd = creat (tempfile, S_IREAD | S_IWRITE);
+       if (outfilefd < 0)
+ 	{
        emacs_close (filefd);
        report_file_error ("Opening process output file",
! 	Fcons (build_string (tempfile), Qnil));
!         }
      }
+   else
+       outfilefd = fd_output;
    fd[0] = filefd;
    fd[1] = outfilefd;
  #endif /* MSDOS */
***************
*** 450,455 ****
--- 492,499 ----
      struct sigaction sigpipe_action;
  #endif
  
+     if (fd_output > 0)
+       fd1 = fd_output;
  #if 0  /* Some systems don't have sigblock.  */
      mask = sigblock (sigmask (SIGCHLD));
  #endif
***************
*** 1554,1559 ****
--- 1598,1606 ----
  #endif
    staticpro (&Vtemp_file_name_pattern);
  
+   Qcallproc_file_symbol = intern_c_string (":file");
+   staticpro (&Qcallproc_file_symbol);
+ 
    DEFVAR_LISP ("shell-file-name", Vshell_file_name,
  	       doc: /* *File name to load inferior shells from.
  Initialized from the SHELL environment variable, or to a system-dependent


[-- Attachment #3: Type: text/plain, Size: 182 bytes --]


(Note!  Not thoroughly tested.  I'll go through all the possibilities
now...)

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 14:10           ` Eli Zaretskii
@ 2011-04-21 15:15             ` Lars Magne Ingebrigtsen
  2011-04-21 15:46               ` Lars Magne Ingebrigtsen
  2011-04-21 17:05             ` Jan Djärv
  1 sibling, 1 reply; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-21 15:15 UTC (permalink / raw)
  To: emacs-devel

I've squashed a few more bugs, and all combinations of OUTPUT-BUFFER
ERROR-FILE now seem to work.

However, since if we're redirecting both STDOUT and STDERR to files, the
way that `call-process' waits for the process to exit won't work.  The
normal way is to loop over a read from the process, and since we now no
longer have any fds that we can read from, it falls through to the

wait_for_termination (pid);

So if you do this:

(call-process "sleep" nil '((:file "/tmp/hello") "/tmp/error") nil "9")

you can't interrupt the sub-process.  That's obviously not acceptable.
Is there an interruptible (is that a word?) version of that function
that can be used?  I've grepped a bit, but didn't see anything
obvious... 

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 15:15             ` Lars Magne Ingebrigtsen
@ 2011-04-21 15:46               ` Lars Magne Ingebrigtsen
  2011-04-21 16:15                 ` Eli Zaretskii
  0 siblings, 1 reply; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-21 15:46 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]

I added a interruptible_wait_for_termination function, which will
probably not work on NT, since I have no idea how to do that.

But otherwise, I think it's usable.  Comments and style tips are
welcome.  :-)  I'm cargo-culting somewhat when it comes to Emacs C
internals... 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: callproc2.patch --]
[-- Type: text/x-diff, Size: 9703 bytes --]

=== modified file 'src/callproc.c'
*** src/callproc.c	2011-04-14 19:34:42 +0000
--- src/callproc.c	2011-04-21 15:41:49 +0000
***************
*** 96,101 ****
--- 96,103 ----
  /* Nonzero if this is termination due to exit.  */
  static int call_process_exited;
  
+ static Lisp_Object Qcallproc_file_symbol;
+ 
  static Lisp_Object Fgetenv_internal (Lisp_Object, Lisp_Object);
  
  static Lisp_Object
***************
*** 156,163 ****
         doc: /* Call PROGRAM synchronously in separate process.
  The remaining arguments are optional.
  The program's input comes from file INFILE (nil means `/dev/null').
! Insert output in BUFFER before point; t means current buffer;
!  nil for BUFFER means discard it; 0 means discard and don't wait.
  BUFFER can also have the form (REAL-BUFFER STDERR-FILE); in that case,
  REAL-BUFFER says what to do with standard output, as above,
  while STDERR-FILE says what to do with standard error in the child.
--- 158,166 ----
         doc: /* Call PROGRAM synchronously in separate process.
  The remaining arguments are optional.
  The program's input comes from file INFILE (nil means `/dev/null').
! Insert output in BUFFER before point; t means current buffer; nil for BUFFER
!  means discard it; 0 means discard and don't wait; and `(:file FILE)', where
!  FILE is a file name string, means that it should be written to that file.
  BUFFER can also have the form (REAL-BUFFER STDERR-FILE); in that case,
  REAL-BUFFER says what to do with standard output, as above,
  while STDERR-FILE says what to do with standard error in the child.
***************
*** 196,209 ****
--- 199,215 ----
    /* File to use for stderr in the child.
       t means use same as standard output.  */
    Lisp_Object error_file;
+   Lisp_Object output_file = Qnil;
  #ifdef MSDOS	/* Demacs 1.1.1 91/10/16 HIRANO Satoshi */
    char *outf, *tempfile;
    int outfilefd;
  #endif
+   int fd_output = -1;
    struct coding_system process_coding; /* coding-system of process output */
    struct coding_system argument_coding;	/* coding-system of arguments */
    /* Set to the return value of Ffind_operation_coding_system.  */
    Lisp_Object coding_systems;
+   int output_to_buffer = 1;
  
    /* Qt denotes that Ffind_operation_coding_system is not yet called.  */
    coding_systems = Qt;
***************
*** 273,281 ****
      {
        buffer = args[2];
  
!       /* If BUFFER is a list, its meaning is
! 	 (BUFFER-FOR-STDOUT FILE-FOR-STDERR).  */
!       if (CONSP (buffer))
  	{
  	  if (CONSP (XCDR (buffer)))
  	    {
--- 279,289 ----
      {
        buffer = args[2];
  
!       /* If BUFFER is a list, its meaning is (BUFFER-FOR-STDOUT
! 	 FILE-FOR-STDERR), unless the first element is :file, in which case see
! 	 the next paragraph. */
!       if (CONSP (buffer) &&
! 	  !EQ (Qcallproc_file_symbol, XCAR (buffer)))
  	{
  	  if (CONSP (XCDR (buffer)))
  	    {
***************
*** 291,296 ****
--- 299,315 ----
  	  buffer = XCAR (buffer);
  	}
  
+       /* If the buffer is (still) a list, it might be a (:file "file") spec. */
+       if (CONSP (buffer) &&
+ 	  CONSP (XCDR (buffer)) &&
+ 	  EQ (Qcallproc_file_symbol, XCAR (buffer)))
+ 	{
+ 	  output_file = Fexpand_file_name (XCAR (XCDR (buffer)),
+ 					   BVAR (current_buffer, directory));
+ 	  CHECK_STRING (output_file);
+ 	  buffer = Qnil;
+ 	}
+ 
        if (!(EQ (buffer, Qnil)
  	    || EQ (buffer, Qt)
  	    || INTEGERP (buffer)))
***************
*** 318,328 ****
       protected by the caller, so all we really have to worry about is
       buffer.  */
    {
!     struct gcpro gcpro1, gcpro2, gcpro3, gcpro4;
  
      current_dir = BVAR (current_buffer, directory);
  
!     GCPRO4 (infile, buffer, current_dir, error_file);
  
      current_dir = Funhandled_file_name_directory (current_dir);
      if (NILP (current_dir))
--- 337,347 ----
       protected by the caller, so all we really have to worry about is
       buffer.  */
    {
!     struct gcpro gcpro1, gcpro2, gcpro3, gcpro4, gcpro5;
  
      current_dir = BVAR (current_buffer, directory);
  
!     GCPRO5 (infile, buffer, current_dir, error_file, output_file);
  
      current_dir = Funhandled_file_name_directory (current_dir);
      if (NILP (current_dir))
***************
*** 342,347 ****
--- 361,368 ----
        current_dir = ENCODE_FILE (current_dir);
      if (STRINGP (error_file) && STRING_MULTIBYTE (error_file))
        error_file = ENCODE_FILE (error_file);
+     if (STRINGP (output_file) && STRING_MULTIBYTE (output_file))
+       output_file = ENCODE_FILE (output_file);
      UNGCPRO;
    }
  
***************
*** 353,358 ****
--- 374,399 ----
        infile = DECODE_FILE (infile);
        report_file_error ("Opening process input file", Fcons (infile, Qnil));
      }
+ 
+   if (STRINGP (output_file))
+     {
+ #ifdef DOS_NT
+       fd_output = emacs_open (SSDATA (output_file),
+ 			      O_WRONLY | O_TRUNC | O_CREAT | O_TEXT,
+ 			      S_IREAD | S_IWRITE);
+ #else  /* not DOS_NT */
+       fd_output = creat (SSDATA (output_file), 0666);
+ #endif /* not DOS_NT */
+       if (fd_output < 0)
+ 	{
+ 	  output_file = DECODE_FILE (output_file);
+ 	  report_file_error ("Opening process output file",
+ 			     Fcons (output_file, Qnil));
+         }
+       if (STRINGP (error_file) || NILP (error_file))
+         output_to_buffer = 0;
+     }
+ 
    /* Search for program; barf if not found.  */
    {
      struct gcpro gcpro1, gcpro2, gcpro3, gcpro4;
***************
*** 413,429 ****
    strcat (tempfile, "detmp.XXX");
    mktemp (tempfile);
  
!   outfilefd = creat (tempfile, S_IREAD | S_IWRITE);
!   if (outfilefd < 0)
      {
!       emacs_close (filefd);
!       report_file_error ("Opening process output file",
! 			 Fcons (build_string (tempfile), Qnil));
      }
    fd[0] = filefd;
    fd[1] = outfilefd;
  #endif /* MSDOS */
! 
    if (INTEGERP (buffer))
      fd[1] = emacs_open (NULL_DEVICE, O_WRONLY, 0), fd[0] = -1;
    else
--- 454,475 ----
    strcat (tempfile, "detmp.XXX");
    mktemp (tempfile);
  
!   /* If we're redirecting STDOUT to a file, this is already opened. */
!   if (fd_output < 0)
      {
!       outfilefd = creat (tempfile, S_IREAD | S_IWRITE);
!       if (outfilefd < 0) {
! 	emacs_close (filefd);
! 	report_file_error ("Opening process output file",
! 			   Fcons (build_string (tempfile), Qnil));
!       }
      }
+   else
+     outfilefd = fd_output;
    fd[0] = filefd;
    fd[1] = outfilefd;
  #endif /* MSDOS */
!   
    if (INTEGERP (buffer))
      fd[1] = emacs_open (NULL_DEVICE, O_WRONLY, 0), fd[0] = -1;
    else
***************
*** 450,455 ****
--- 496,503 ----
      struct sigaction sigpipe_action;
  #endif
  
+     if (fd_output >= 0)
+       fd1 = fd_output;
  #if 0  /* Some systems don't have sigblock.  */
      mask = sigblock (sigmask (SIGCHLD));
  #endif
***************
*** 572,578 ****
        }
  
      UNBLOCK_INPUT;
! 
  #ifdef HAVE_WORKING_VFORK
      /* Restore the signal state.  */
      sigaction (SIGPIPE, &sigpipe_action, 0);
--- 620,626 ----
        }
  
      UNBLOCK_INPUT;
!     
  #ifdef HAVE_WORKING_VFORK
      /* Restore the signal state.  */
      sigaction (SIGPIPE, &sigpipe_action, 0);
***************
*** 591,596 ****
--- 639,646 ----
      /* Close most of our fd's, but not fd[0]
         since we will use that to read input from.  */
      emacs_close (filefd);
+     if (fd_output >= 0)
+       emacs_close (fd_output);
      if (fd1 >= 0 && fd1 != fd_error)
        emacs_close (fd1);
    }
***************
*** 673,678 ****
--- 723,729 ----
    immediate_quit = 1;
    QUIT;
  
+   if (output_to_buffer)
    {
      register EMACS_INT nread;
      int first = 1;
***************
*** 802,808 ****
  
  #ifndef MSDOS
    /* Wait for it to terminate, unless it already has.  */
!   wait_for_termination (pid);
  #endif
  
    immediate_quit = 0;
--- 853,862 ----
  
  #ifndef MSDOS
    /* Wait for it to terminate, unless it already has.  */
!   if (output_to_buffer)
!     wait_for_termination (pid);
!   else
!     interruptible_wait_for_termination (pid);
  #endif
  
    immediate_quit = 0;
***************
*** 1554,1559 ****
--- 1608,1616 ----
  #endif
    staticpro (&Vtemp_file_name_pattern);
  
+   Qcallproc_file_symbol = intern_c_string (":file");
+   staticpro (&Qcallproc_file_symbol);
+ 
    DEFVAR_LISP ("shell-file-name", Vshell_file_name,
  	       doc: /* *File name to load inferior shells from.
  Initialized from the SHELL environment variable, or to a system-dependent

=== modified file 'src/lisp.h'
*** src/lisp.h	2011-04-15 08:22:34 +0000
--- src/lisp.h	2011-04-21 15:36:57 +0000
***************
*** 3309,3314 ****
--- 3309,3315 ----
  extern void init_all_sys_modes (void);
  extern void reset_all_sys_modes (void);
  extern void wait_for_termination (int);
+ extern void interruptible_wait_for_termination (int);
  extern void flush_pending_output (int);
  extern void child_setup_tty (int);
  extern void setup_pty (int);

=== modified file 'src/sysdep.c'
*** src/sysdep.c	2011-04-16 21:26:33 +0000
--- src/sysdep.c	2011-04-21 15:35:27 +0000
***************
*** 302,307 ****
--- 302,320 ----
  void
  wait_for_termination (int pid)
  {
+   wait_for_termination_1 (pid, 0);
+ }
+ 
+ /* Like the above, but allow keyboard interruption. */
+ void
+ interruptible_wait_for_termination (int pid)
+ {
+   wait_for_termination_1 (pid, 1);
+ }
+ 
+ void
+ wait_for_termination_1 (int pid, int interruptible)
+ {
    while (1)
      {
  #if defined (BSD_SYSTEM) || defined (HPUX)
***************
*** 339,344 ****
--- 352,359 ----
        sigsuspend (&empty_mask);
  #endif /* not WINDOWSNT */
  #endif /* not BSD_SYSTEM, and not HPUX version >= 6 */
+       if (interruptible)
+ 	QUIT;
      }
  }
  


[-- Attachment #3: Type: text/plain, Size: 103 bytes --]


-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 15:46               ` Lars Magne Ingebrigtsen
@ 2011-04-21 16:15                 ` Eli Zaretskii
  2011-04-21 16:22                   ` Lars Magne Ingebrigtsen
  2011-04-21 16:24                   ` Lars Magne Ingebrigtsen
  0 siblings, 2 replies; 37+ messages in thread
From: Eli Zaretskii @ 2011-04-21 16:15 UTC (permalink / raw)
  To: emacs-devel

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 21 Apr 2011 17:46:17 +0200
> 
> I added a interruptible_wait_for_termination function, which will
> probably not work on NT, since I have no idea how to do that.

It seems like the Windows version is already interruptible, see
w32proc.c:sys_wait (wait_for_termination calls `wait' on Windows, but
`wait' is a macro that resolves to sys_wait).  It calls a Windows API
that allows to specify a timeout, and we set that timeout to 1 sec.
So Emacs on Windows checks for QUIT roughly once a second.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 16:15                 ` Eli Zaretskii
@ 2011-04-21 16:22                   ` Lars Magne Ingebrigtsen
  2011-04-21 16:24                   ` Lars Magne Ingebrigtsen
  1 sibling, 0 replies; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-21 16:22 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> It seems like the Windows version is already interruptible, see
> w32proc.c:sys_wait (wait_for_termination calls `wait' on Windows, but
> `wait' is a macro that resolves to sys_wait).  It calls a Windows API
> that allows to specify a timeout, and we set that timeout to 1 sec.
> So Emacs on Windows checks for QUIT roughly once a second.

Ah, great!  That should be good enough, I think.  Thanks for
checking...

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 16:15                 ` Eli Zaretskii
  2011-04-21 16:22                   ` Lars Magne Ingebrigtsen
@ 2011-04-21 16:24                   ` Lars Magne Ingebrigtsen
  2011-04-21 16:55                     ` Eli Zaretskii
  1 sibling, 1 reply; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-21 16:24 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> It seems like the Windows version is already interruptible, see
> w32proc.c:sys_wait (wait_for_termination calls `wait' on Windows, but
> `wait' is a macro that resolves to sys_wait).  It calls a Windows API
> that allows to specify a timeout, and we set that timeout to 1 sec.
> So Emacs on Windows checks for QUIT roughly once a second.

Or to be more consistent -- should the non-Windows version also be
interruptible by default?  That might have unintended consequences, I
guess, but then we could drop the interruptible_wait_for function and
just put a QUIT in the normal version of the function...

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 13:25         ` Lars Magne Ingebrigtsen
  2011-04-21 14:10           ` Eli Zaretskii
@ 2011-04-21 16:29           ` Glenn Morris
  1 sibling, 0 replies; 37+ messages in thread
From: Glenn Morris @ 2011-04-21 16:29 UTC (permalink / raw)
  To: emacs-devel

Lars Magne Ingebrigtsen wrote:

> However, redirecting STDERR to a buffer seems like it would be quite
> complicated, unfortunately.

That's what the elisp manual says:

   Creating a Synchronous Process
   [...]
   You can't directly specify a buffer to put the error output in;
   that is too difficult to implement.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 16:24                   ` Lars Magne Ingebrigtsen
@ 2011-04-21 16:55                     ` Eli Zaretskii
  0 siblings, 0 replies; 37+ messages in thread
From: Eli Zaretskii @ 2011-04-21 16:55 UTC (permalink / raw)
  To: emacs-devel

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 21 Apr 2011 18:24:47 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > It seems like the Windows version is already interruptible, see
> > w32proc.c:sys_wait (wait_for_termination calls `wait' on Windows, but
> > `wait' is a macro that resolves to sys_wait).  It calls a Windows API
> > that allows to specify a timeout, and we set that timeout to 1 sec.
> > So Emacs on Windows checks for QUIT roughly once a second.
> 
> Or to be more consistent -- should the non-Windows version also be
> interruptible by default?

Maybe, I don't know.  There could be some dark corners wrt signals,
someone who knows more about that should tell.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 14:10           ` Eli Zaretskii
  2011-04-21 15:15             ` Lars Magne Ingebrigtsen
@ 2011-04-21 17:05             ` Jan Djärv
  2011-04-21 19:15               ` Eli Zaretskii
  1 sibling, 1 reply; 37+ messages in thread
From: Jan Djärv @ 2011-04-21 17:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Lars Magne Ingebrigtsen, emacs-devel@gnu.org



21 apr 2011 kl. 16:10 skrev Eli Zaretskii <eliz@gnu.org>:

>> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
>> Date: Thu, 21 Apr 2011 15:25:29 +0200
>> 
>> However, redirecting STDERR to a buffer seems like it would be quite
>> complicated, unfortunately.  I thought that having STDOUT and STDERR be
>> handled symmetrically would be a nice feature (for redirecting STDOUT to
>> one buffer and STDERR to another, for instance), but unless I'm reading
>> the code wrong (and I very well could be), I don't see an obvious way to
>> do that.
> 
> On Posix systems, we use a pipe to read from subprocess's STDOUT.  You
> cannot do the same with STDERR, but you can redirect STDERR to a
> temporary file, and then read it when the subprocess exits.

That is news to me. I've redirected STDERR to a pipe many times in other programs. Why can't it be done in Emacs?

    Jan D.




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 17:05             ` Jan Djärv
@ 2011-04-21 19:15               ` Eli Zaretskii
  2011-04-21 19:19                 ` Davis Herring
  0 siblings, 1 reply; 37+ messages in thread
From: Eli Zaretskii @ 2011-04-21 19:15 UTC (permalink / raw)
  To: Jan Djärv; +Cc: larsi, emacs-devel

> Cc: Lars Magne Ingebrigtsen <larsi@gnus.org>,
>  "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> From: Jan Djärv <jan.h.d@swipnet.se>
> Date: Thu, 21 Apr 2011 19:05:39 +0200
> 
> > On Posix systems, we use a pipe to read from subprocess's STDOUT.  You
> > cannot do the same with STDERR, but you can redirect STDERR to a
> > temporary file, and then read it when the subprocess exits.
> 
> That is news to me. I've redirected STDERR to a pipe many times in other programs. Why can't it be done in Emacs?

Maybe I'm missing something, but the call to `pipe' produces only 2
file descriptors on each end.  So you can have either stdout or stderr
of the subprocess, or both of them together, but not both of them
separately.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 19:15               ` Eli Zaretskii
@ 2011-04-21 19:19                 ` Davis Herring
  2011-04-21 19:31                   ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 37+ messages in thread
From: Davis Herring @ 2011-04-21 19:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, Jan Djärv, emacs-devel

>>> On Posix systems, we use a pipe to read from subprocess's STDOUT.  You
>>> cannot do the same with STDERR, but you can redirect STDERR to a
>>> temporary file, and then read it when the subprocess exits.
>>
>> That is news to me. I've redirected STDERR to a pipe many times in other programs. Why can't it be done in Emacs?
> 
> Maybe I'm missing something, but the call to `pipe' produces only 2
> file descriptors on each end.  So you can have either stdout or stderr
> of the subprocess, or both of them together, but not both of them
> separately.

You can call `pipe' more than once; the trouble comes when you have to
choose which one to read from to avoid blocking the process.  Probably
it's just that no one has bothered to implement it carefully enough with
`select' or so.

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 19:19                 ` Davis Herring
@ 2011-04-21 19:31                   ` Lars Magne Ingebrigtsen
  2011-04-22  5:50                     ` Eli Zaretskii
  0 siblings, 1 reply; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-21 19:31 UTC (permalink / raw)
  To: emacs-devel

Davis Herring <herring@lanl.gov> writes:

> You can call `pipe' more than once; the trouble comes when you have to
> choose which one to read from to avoid blocking the process.  Probably
> it's just that no one has bothered to implement it carefully enough with
> `select' or so.

Oh, I see...  I was a bit confused about how pipe worked.  Yes, that
makes sense.  Just hook up the STDERR endpoint to a pipe, too, and
you're in business.

For the MSDOS case (with STDERR-to-buffer), we'd just have to open
another temp file, just like we do with STDOUT, so that's a very minor
change.

For the non-MSDOS case, using two pipes and select shouldn't be too
difficult to implement, I think?  Or are there gotchas when using select
in Emacs?

But before embarking on that, I'd like to get some more feedback on the
existing patch.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-21 19:31                   ` Lars Magne Ingebrigtsen
@ 2011-04-22  5:50                     ` Eli Zaretskii
  2011-04-23 18:46                       ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 37+ messages in thread
From: Eli Zaretskii @ 2011-04-22  5:50 UTC (permalink / raw)
  To: emacs-devel

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 21 Apr 2011 21:31:58 +0200
> 
> Davis Herring <herring@lanl.gov> writes:
> 
> > You can call `pipe' more than once; the trouble comes when you have to
> > choose which one to read from to avoid blocking the process.  Probably
> > it's just that no one has bothered to implement it carefully enough with
> > `select' or so.
> 
> Oh, I see...  I was a bit confused about how pipe worked.  Yes, that
> makes sense.  Just hook up the STDERR endpoint to a pipe, too, and
> you're in business.
> 
> For the MSDOS case (with STDERR-to-buffer), we'd just have to open
> another temp file, just like we do with STDOUT, so that's a very minor
> change.
> 
> For the non-MSDOS case, using two pipes and select shouldn't be too
> difficult to implement, I think?

You are in danger of reinventing the wait_reading_process_output
wheel.  I'm not sure that kind of complexity is justified for such a
marginal need, just for the sake of theoretical completeness and/or
symmetry between stdout and stderr.  I think redirecting to a
temporary file and then reading from there is more than adequate, and
all but indistinguishable from the user POV (since we wait for the
process to exit anyway).

> Or are there gotchas when using select in Emacs?

wait_reading_process_output already uses select, so adding another one
is probably not a good idea.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-22  5:50                     ` Eli Zaretskii
@ 2011-04-23 18:46                       ` Lars Magne Ingebrigtsen
  2011-04-23 20:10                         ` Eli Zaretskii
  0 siblings, 1 reply; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-23 18:46 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> You are in danger of reinventing the wait_reading_process_output
> wheel.  I'm not sure that kind of complexity is justified for such a
> marginal need, just for the sake of theoretical completeness and/or
> symmetry between stdout and stderr.  I think redirecting to a
> temporary file and then reading from there is more than adequate, and
> all but indistinguishable from the user POV (since we wait for the
> process to exit anyway).

Symmetry is nice, and temp file handling is always a hassle, so if it
can be avoided (without adding too much complexity), it seems worthwhile
to me...

>> Or are there gotchas when using select in Emacs?
>
> wait_reading_process_output already uses select, so adding another one
> is probably not a good idea.

Would `select'-ing on two separate collections of sockets cause any
problems? 

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-23 18:46                       ` Lars Magne Ingebrigtsen
@ 2011-04-23 20:10                         ` Eli Zaretskii
  2011-04-24  8:30                           ` Jan Djärv
  0 siblings, 1 reply; 37+ messages in thread
From: Eli Zaretskii @ 2011-04-23 20:10 UTC (permalink / raw)
  To: Lars Magne Ingebrigtsen; +Cc: emacs-devel

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Sat, 23 Apr 2011 20:46:36 +0200
> 
> >> Or are there gotchas when using select in Emacs?
> >
> > wait_reading_process_output already uses select, so adding another one
> > is probably not a good idea.
> 
> Would `select'-ing on two separate collections of sockets cause any
> problems? 

I don't know.  I never tried, and I don't know enough about the
implementation of `select'.  But I envision difficulties in
multiplexing between the two `select' calls, if, e.g., there's an
async subprocess running (like a speller in some text buffer) and you
invoke a program via call-process.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-23 20:10                         ` Eli Zaretskii
@ 2011-04-24  8:30                           ` Jan Djärv
  2011-04-30 23:58                             ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 37+ messages in thread
From: Jan Djärv @ 2011-04-24  8:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Lars Magne Ingebrigtsen, emacs-devel



Eli Zaretskii skrev 2011-04-23 22.10:
>> From: Lars Magne Ingebrigtsen<larsi@gnus.org>
>> Date: Sat, 23 Apr 2011 20:46:36 +0200
>>
>>>> Or are there gotchas when using select in Emacs?
>>>
>>> wait_reading_process_output already uses select, so adding another one
>>> is probably not a good idea.
>>
>> Would `select'-ing on two separate collections of sockets cause any
>> problems?
>
> I don't know.  I never tried, and I don't know enough about the
> implementation of `select'.  But I envision difficulties in
> multiplexing between the two `select' calls, if, e.g., there's an
> async subprocess running (like a speller in some text buffer) and you
> invoke a program via call-process.

Since call-process normally doesn't return until the process is finished, that 
will not differ from what we have now.

As for selecting in two places, this is already done, for example when you 
open a new network socket, when a menu is opened on X, and when waiting for 
resize events on X.  You just put in the pipe end of stdin and stderr into the 
select and loop on that.

	Jan D.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-24  8:30                           ` Jan Djärv
@ 2011-04-30 23:58                             ` Lars Magne Ingebrigtsen
  2011-05-01  0:06                               ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-04-30 23:58 UTC (permalink / raw)
  To: emacs-devel

Jan Djärv <jan.h.d@swipnet.se> writes:

> Since call-process normally doesn't return until the process is
> finished, that will not differ from what we have now.

Yeah, so I think adding the buffer output for STDERR shouldn't be too
difficult to do, and I'll be thinking about adding it later.
Unfortunately, I've just had very little time lately, so I'll just apply
the STDOUT portion of the patch now, and we'll see what we do about
STDERR later.

These are all extensions, so adding this piece by piece shouldn't be a
disadvantage.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-04-30 23:58                             ` Lars Magne Ingebrigtsen
@ 2011-05-01  0:06                               ` Lars Magne Ingebrigtsen
  2011-05-01 17:56                                 ` Andy Moreton
  2011-05-07 11:34                                 ` Eli Zaretskii
  0 siblings, 2 replies; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-05-01  0:06 UTC (permalink / raw)
  To: emacs-devel

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Unfortunately, I've just had very little time lately, so I'll just apply
> the STDOUT portion of the patch now, and we'll see what we do about
> STDERR later.

I've now done this.  I have not verified that it works on Windows/DOS,
so if someone could do that (and fix up any bloopers :-), I'd be
grateful.

The test case is:

(call-process "echo" nil '(:file "/tmp/buffer") nil "hello")

which should leave you with a file called "/tmp/buffer" with "hello\n"
as the contents.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-05-01  0:06                               ` Lars Magne Ingebrigtsen
@ 2011-05-01 17:56                                 ` Andy Moreton
  2011-05-07 11:34                                 ` Eli Zaretskii
  1 sibling, 0 replies; 37+ messages in thread
From: Andy Moreton @ 2011-05-01 17:56 UTC (permalink / raw)
  To: emacs-devel

On Sun 01 May 2011, Lars Magne Ingebrigtsen wrote:

> Lars Magne Ingebrigtsen <larsi@gnus.org> writes:
>
>> Unfortunately, I've just had very little time lately, so I'll just apply
>> the STDOUT portion of the patch now, and we'll see what we do about
>> STDERR later.
>
> I've now done this.  I have not verified that it works on Windows/DOS,
> so if someone could do that (and fix up any bloopers :-), I'd be
> grateful.
>
> The test case is:
>
> (call-process "echo" nil '(:file "/tmp/buffer") nil "hello")
>
> which should leave you with a file called "/tmp/buffer" with "hello\n"
> as the contents.

This works fine for me on WinXP:

(call-process "echo" nil '(:file "c:/temp/buffer") nil "hello")

    AndyM




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-05-01  0:06                               ` Lars Magne Ingebrigtsen
  2011-05-01 17:56                                 ` Andy Moreton
@ 2011-05-07 11:34                                 ` Eli Zaretskii
  2011-05-07 12:10                                   ` Eli Zaretskii
  1 sibling, 1 reply; 37+ messages in thread
From: Eli Zaretskii @ 2011-05-07 11:34 UTC (permalink / raw)
  To: Lars Magne Ingebrigtsen; +Cc: emacs-devel

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Sun, 01 May 2011 02:06:46 +0200
> 
> I've now done this.  I have not verified that it works on Windows/DOS,
> so if someone could do that (and fix up any bloopers :-), I'd be
> grateful.
> 
> The test case is:
> 
> (call-process "echo" nil '(:file "/tmp/buffer") nil "hello")
> 
> which should leave you with a file called "/tmp/buffer" with "hello\n"
> as the contents.

Thanks.

It didn't work on DOS, because Emacs was trying to read from a
temporary file which we don't create when :file is used.

I fixed that; see the diffs for revno 104152.1.2 for the gory details
(if you are interested).



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-05-07 11:34                                 ` Eli Zaretskii
@ 2011-05-07 12:10                                   ` Eli Zaretskii
  2011-05-30 17:39                                     ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 37+ messages in thread
From: Eli Zaretskii @ 2011-05-07 12:10 UTC (permalink / raw)
  To: larsi, emacs-devel

> Date: Sat, 07 May 2011 14:34:05 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> I fixed that; see the diffs for revno 104152.1.2 for the gory details
> (if you are interested).

Btw, while working on this, I noticed that the code fragment shown
below is doing a lot of work that is just thrown away in the case that
`:file' is used to redirect stdout of the child.  That's because we
will never read from that file, so setting up the coding-system to
decode it seems futile.

Should we do all that only when output_to_buffer is non-zero?

  if (NILP (buffer))
    {
      /* If BUFFER is nil, we must read process output once and then
	 discard it, so setup coding system but with nil.  */
      setup_coding_system (Qnil, &process_coding);
    }
  else
    {
      Lisp_Object val, *args2;

      val = Qnil;
      if (!NILP (Vcoding_system_for_read))
	val = Vcoding_system_for_read;
      else
	{
	  if (EQ (coding_systems, Qt))
	    {
	      size_t i;

	      SAFE_ALLOCA (args2, Lisp_Object *, (nargs + 1) * sizeof *args2);
	      args2[0] = Qcall_process;
	      for (i = 0; i < nargs; i++) args2[i + 1] = args[i];
	      coding_systems
		= Ffind_operation_coding_system (nargs + 1, args2);
	    }
	  if (CONSP (coding_systems))
	    val = XCAR (coding_systems);
	  else if (CONSP (Vdefault_process_coding_system))
	    val = XCAR (Vdefault_process_coding_system);
	  else
	    val = Qnil;
	}
      Fcheck_coding_system (val);
      /* In unibyte mode, character code conversion should not take
	 place but EOL conversion should.  So, setup raw-text or one
	 of the subsidiary according to the information just setup.  */
      if (NILP (BVAR (current_buffer, enable_multibyte_characters))
	  && !NILP (val))
	val = raw_text_coding_system (val);
      setup_coding_system (val, &process_coding);
    }

  immediate_quit = 1;
  QUIT;



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Redirecting standard output
  2011-05-07 12:10                                   ` Eli Zaretskii
@ 2011-05-30 17:39                                     ` Lars Magne Ingebrigtsen
  0 siblings, 0 replies; 37+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-05-30 17:39 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> I fixed that; see the diffs for revno 104152.1.2 for the gory details
>> (if you are interested).

Thanks for doing this.  I had a feeling that the DOS bits weren't quite
right.  :-)

> Btw, while working on this, I noticed that the code fragment shown
> below is doing a lot of work that is just thrown away in the case that
> `:file' is used to redirect stdout of the child.  That's because we
> will never read from that file, so setting up the coding-system to
> decode it seems futile.
>
> Should we do all that only when output_to_buffer is non-zero?

Yes, I think that sounds correct.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/




^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2011-05-30 17:39 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-20 21:19 Redirecting standard output Lars Magne Ingebrigtsen
2011-04-20 22:54 ` Stefan Monnier
2011-04-21  1:54   ` Lars Magne Ingebrigtsen
2011-04-21  6:10     ` Eli Zaretskii
2011-04-21 11:45       ` Lars Magne Ingebrigtsen
2011-04-21 13:25         ` Lars Magne Ingebrigtsen
2011-04-21 14:10           ` Eli Zaretskii
2011-04-21 15:15             ` Lars Magne Ingebrigtsen
2011-04-21 15:46               ` Lars Magne Ingebrigtsen
2011-04-21 16:15                 ` Eli Zaretskii
2011-04-21 16:22                   ` Lars Magne Ingebrigtsen
2011-04-21 16:24                   ` Lars Magne Ingebrigtsen
2011-04-21 16:55                     ` Eli Zaretskii
2011-04-21 17:05             ` Jan Djärv
2011-04-21 19:15               ` Eli Zaretskii
2011-04-21 19:19                 ` Davis Herring
2011-04-21 19:31                   ` Lars Magne Ingebrigtsen
2011-04-22  5:50                     ` Eli Zaretskii
2011-04-23 18:46                       ` Lars Magne Ingebrigtsen
2011-04-23 20:10                         ` Eli Zaretskii
2011-04-24  8:30                           ` Jan Djärv
2011-04-30 23:58                             ` Lars Magne Ingebrigtsen
2011-05-01  0:06                               ` Lars Magne Ingebrigtsen
2011-05-01 17:56                                 ` Andy Moreton
2011-05-07 11:34                                 ` Eli Zaretskii
2011-05-07 12:10                                   ` Eli Zaretskii
2011-05-30 17:39                                     ` Lars Magne Ingebrigtsen
2011-04-21 16:29           ` Glenn Morris
2011-04-21  8:27   ` Michael Albinus
2011-04-21  5:57 ` Eli Zaretskii
2011-04-21  6:28   ` Thierry Volpiatto
2011-04-21  6:41     ` Eli Zaretskii
2011-04-21  7:33       ` Thierry Volpiatto
2011-04-21 11:40   ` Lars Magne Ingebrigtsen
2011-04-21 11:58     ` Eli Zaretskii
2011-04-21 12:24       ` Lars Magne Ingebrigtsen
2011-04-21 14:25         ` Lars Magne Ingebrigtsen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).