[PATCH 1/3] Document that 'make-process' mixes the output streams

[PATCH 1/3] Document that 'make-process' mixes the output streams

[Philipp Stephani]

* doc/lispref/processes.texi (Asynchronous Processes):
* src/process.c (Fmake_process): Document that standard error is mixed
with standard output if STDERR is nil.

* test/src/process-tests.el (make-process/mix-stderr): New unit test.
---
 doc/lispref/processes.texi |  4 +++-
 src/process.c              |  3 ++-
 test/src/process-tests.el  | 18 ++++++++++++++++++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/doc/lispref/processes.texi b/doc/lispref/processes.texi
index af177e053c..3e26f57798 100644
--- a/doc/lispref/processes.texi
+++ b/doc/lispref/processes.texi
@@ -681,7 +681,9 @@ Asynchronous Processes
 @item :stderr @var{stderr}
 Associate @var{stderr} with the standard error of the process.  A
 non-@code{nil} value should be either a buffer or a pipe process
-created with @code{make-pipe-process}, described below.
+created with @code{make-pipe-process}, described below.  If
+@var{stderr} is @code{nil}, standard error is mixed with standard
+output, and both are sent to @var{buffer} or @var{filter}.
 @end table
 
 The original argument list, modified with the actual connection
diff --git a/src/process.c b/src/process.c
index ed2cab7b51..c357a8bdc3 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1657,7 +1657,8 @@ to use a pty, or nil to use the default specified through
 
 :stderr STDERR -- STDERR is either a buffer or a pipe process attached
 to the standard error of subprocess.  Specifying this implies
-`:connection-type' is set to `pipe'.
+`:connection-type' is set to `pipe'.  If STDERR is nil, standard error
+is mixed with standard output and sent to BUFFER or FILTER.
 
 usage: (make-process &rest ARGS)  */)
   (ptrdiff_t nargs, Lisp_Object *args)
diff --git a/test/src/process-tests.el b/test/src/process-tests.el
index 7d35560229..b1b4665c3c 100644
--- a/test/src/process-tests.el
+++ b/test/src/process-tests.el
@@ -181,5 +181,23 @@ process-test-sentinel-wait-function-working-p
               (should-not (process-query-on-exit-flag process))))
         (kill-process process)))))
 
+(ert-deftest make-process/mix-stderr ()
+  "Check that ‘make-process’ mixes the output streams if STDERR is nil."
+  (skip-unless (executable-find shell-file-name))
+  (with-temp-buffer
+    (let ((process (make-process
+                    :name "mix-stderr"
+                    :command (list shell-file-name shell-command-switch
+                                   "echo stdout; echo stderr >&2")
+                    :buffer (current-buffer)
+                    :sentinel #'ignore
+                    :noquery t
+                    :connection-type 'pipe)))
+      (while (process-live-p process)
+        (accept-process-output process))
+      (should (eq (process-status process) 'exit))
+      (should (eq (process-exit-status process) 0))
+      (should (equal (buffer-string) "stdout\nstderr\n")))))
+
 (provide 'process-tests)
 ;; process-tests.el ends here.
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Philipp Stephani]

* src/process.c (syms_of_process): Add 'ignore' symbol.
(Fmake_process): Use it as sentinel for the standard error pipe
process.

* test/src/process-tests.el (make-process/stderr-sentinel): New unit
test.
---
 src/process.c             |  5 ++++-
 test/src/process-tests.el | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/process.c b/src/process.c
index c357a8bdc3..dcc9dcb31e 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1714,7 +1714,9 @@ usage: (make-process &rest ARGS)  */)
 			  QCbuffer,
 			  Fget_buffer_create (xstderr),
 			  QCnoquery,
-			  query_on_exit ? Qnil : Qt);
+			  query_on_exit ? Qnil : Qt,
+			  QCsentinel,
+			  Qignore);
     }
 
   proc = make_process (name);
@@ -8184,6 +8186,7 @@ syms_of_process (void)
 	  "internal-default-process-sentinel");
   DEFSYM (Qinternal_default_process_filter,
 	  "internal-default-process-filter");
+  DEFSYM (Qignore, "ignore");
 #endif
   DEFSYM (Qpri, "pri");
   DEFSYM (Qnice, "nice");
diff --git a/test/src/process-tests.el b/test/src/process-tests.el
index b1b4665c3c..838ba78acb 100644
--- a/test/src/process-tests.el
+++ b/test/src/process-tests.el
@@ -199,5 +199,23 @@ process-test-sentinel-wait-function-working-p
       (should (eq (process-exit-status process) 0))
       (should (equal (buffer-string) "stdout\nstderr\n")))))
 
+(ert-deftest make-process/stderr-sentinel ()
+  "Check that `make-process' doesn't install the default sentinel for stderr."
+  (skip-unless (executable-find shell-file-name))
+  (with-temp-buffer
+    (let ((process (make-process
+                    :name "stderr-sentinel"
+                    :command (list shell-file-name shell-command-switch
+                                   "echo stderr >&2")
+                    :buffer nil
+                    :stderr (current-buffer)
+                    :noquery t
+                    :connection-type 'pipe)))
+      (while (process-live-p process)
+        (accept-process-output process))
+      (should (eq (process-status process) 'exit))
+      (should (eq (process-exit-status process) 0))
+      (should (equal (buffer-string) "stderr\n")))))
+
 (provide 'process-tests)
 ;; process-tests.el ends here.
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Wed,  4 Apr 2018 14:02:17 +0200
> Cc: Philipp Stephani <phst@google.com>
> 
> * src/process.c (syms_of_process): Add 'ignore' symbol.
> (Fmake_process): Use it as sentinel for the standard error pipe
> process.

Why would we want that, and by default on top of that?  Please give at
least some rationale behind this change.

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Philipp Stephani]

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

Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 4. Apr. 2018 um 15:14 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Wed,  4 Apr 2018 14:02:17 +0200
> > Cc: Philipp Stephani <phst@google.com>
> >
> > * src/process.c (syms_of_process): Add 'ignore' symbol.
> > (Fmake_process): Use it as sentinel for the standard error pipe
> > process.
>
> Why would we want that, and by default on top of that?  Please give at
> least some rationale behind this change.
>

Neither the manual not the docstring for `make-process' specify that Emacs
prints "Process foo stderr finished" at the end of the standard error
buffer, so that message shouldn't be printed.

[-- Attachment #2: Type: text/html, Size: 1133 bytes --]

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 07 Apr 2018 20:21:40 +0000
> Cc: emacs-devel@gnu.org, phst@google.com
> 
>  > * src/process.c (syms_of_process): Add 'ignore' symbol.
>  > (Fmake_process): Use it as sentinel for the standard error pipe
>  > process.
> 
>  Why would we want that, and by default on top of that?  Please give at
>  least some rationale behind this change.
> 
> Neither the manual not the docstring for `make-process' specify that Emacs prints "Process foo stderr
> finished" at the end of the standard error buffer, so that message shouldn't be printed. 

That assumes that the documentation is correct and the code isn't; it
could be the other way around.

But I'm guessing that the current behavior was unexpected for some
reason, and that's why you looked in the documentation.  If the guess
is correct, could you describe why it was unexpected/unwanted?

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Stephen Leake]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philipp Stephani <p.stephani2@gmail.com>
>> Date: Sat, 07 Apr 2018 20:21:40 +0000
>> Cc: emacs-devel@gnu.org, phst@google.com
>> 
>>  > * src/process.c (syms_of_process): Add 'ignore' symbol.
>>  > (Fmake_process): Use it as sentinel for the standard error pipe
>>  > process.
>> 
>>  Why would we want that, and by default on top of that?  Please give at
>>  least some rationale behind this change.
>> 
>> Neither the manual not the docstring for `make-process' specify that
>> Emacs prints "Process foo stderr
>> finished" at the end of the standard error buffer, so that message
>> shouldn't be printed.
>
> That assumes that the documentation is correct and the code isn't; it
> could be the other way around.
>
> But I'm guessing that the current behavior was unexpected for some
> reason, and that's why you looked in the documentation.  If the guess
> is correct, could you describe why it was unexpected/unwanted?

I had to work around that behavior as well, in DVC (Emacs front end for
some CM tools). When you are parsing the output of a process, that
string is unexpected.

-- 
-- Stephe

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Eli Zaretskii]

On April 10, 2018 7:14:39 AM GMT+03:00, Stephen Leake <stephen_leake@stephe-leake.org> wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Philipp Stephani <p.stephani2@gmail.com>
> >> Date: Sat, 07 Apr 2018 20:21:40 +0000
> >> Cc: emacs-devel@gnu.org, phst@google.com
> >> 
> >>  > * src/process.c (syms_of_process): Add 'ignore' symbol.
> >>  > (Fmake_process): Use it as sentinel for the standard error pipe
> >>  > process.
> >> 
> >>  Why would we want that, and by default on top of that?  Please
> give at
> >>  least some rationale behind this change.
> >> 
> >> Neither the manual not the docstring for `make-process' specify
> that
> >> Emacs prints "Process foo stderr
> >> finished" at the end of the standard error buffer, so that message
> >> shouldn't be printed.
> >
> > That assumes that the documentation is correct and the code isn't;
> it
> > could be the other way around.
> >
> > But I'm guessing that the current behavior was unexpected for some
> > reason, and that's why you looked in the documentation.  If the
> guess
> > is correct, could you describe why it was unexpected/unwanted?
> 
> I had to work around that behavior as well, in DVC (Emacs front end
> for
> some CM tools). When you are parsing the output of a process, that
> string is unexpected.

Are we talking about the same thing?  Inserting the exit status into the process
buffer by the default sentinel is veteran Emacs behavior; if you don't want that,
you are supposed to define your own sentinel.

Philipp suggested to avoid this *only* for buffers collecting stderr output,
and *only* when make-process defines :stderr.  That's a much more specialized
situation, which is also quite new.  By contrast, what you are saying seems to
suggest changes in general behavior of make-process that IMO are a no-starter
at this time.

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Philipp Stephani]

Am So., 8. Apr. 2018 um 15:13 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 07 Apr 2018 20:21:40 +0000
> > Cc: emacs-devel@gnu.org, phst@google.com
> >
> >  > * src/process.c (syms_of_process): Add 'ignore' symbol.
> >  > (Fmake_process): Use it as sentinel for the standard error pipe
> >  > process.
> >
> >  Why would we want that, and by default on top of that?  Please give at
> >  least some rationale behind this change.
> >
> > Neither the manual not the docstring for `make-process' specify that Emacs prints "Process foo stderr
> > finished" at the end of the standard error buffer, so that message shouldn't be printed.
>
> That assumes that the documentation is correct and the code isn't; it
> could be the other way around.

Sure. My opinion is that the documentation is correct (albeit
incomplete), and the code is slightly buggy.

>
> But I'm guessing that the current behavior was unexpected for some
> reason, and that's why you looked in the documentation.  If the guess
> is correct, could you describe why it was unexpected/unwanted?

It's unexpected that Emacs writes something. This is an instance of
the general case that by default you expect nothing to happen instead
of something, unless you request the "something". For example, after
$ echo foo > bar
would you expect that the file "bar" contains content other than
"foo\n" because the shell decides to write additional text before
closing redirections? This is the same thing.
The documentation for make-process doesn't state that if :stderr is a
buffer, make-process creates a pipe process with the default sentinel;
rather it treats that pipe process as implementation detail (which is
OK-ish), so users don't expect the default sentinel to run for the
stderr buffer.

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Fri, 19 Apr 2019 21:28:32 +0200
> Cc: Emacs developers <emacs-devel@gnu.org>, Philipp Stephani <phst@google.com>
> 
> > But I'm guessing that the current behavior was unexpected for some
> > reason, and that's why you looked in the documentation.  If the guess
> > is correct, could you describe why it was unexpected/unwanted?
> 
> It's unexpected that Emacs writes something. This is an instance of
> the general case that by default you expect nothing to happen instead
> of something, unless you request the "something". For example, after
> $ echo foo > bar
> would you expect that the file "bar" contains content other than
> "foo\n" because the shell decides to write additional text before
> closing redirections? This is the same thing.

But Emacs does that for ages, so in Emacs this is veteran behavior of
the default sentinel function.

> The documentation for make-process doesn't state that if :stderr is a
> buffer, make-process creates a pipe process with the default sentinel;

Actually, it does, albeit in the parent section:

     Alternatively, you could use the ‘:stderr’ parameter with a non-‘nil’
  value in a call to ‘make-process’ (*note make-process: Asynchronous
  Processes.) to make the destination of the error output separate from
  the standard output; in that case, Emacs will use pipes for
  communicating with the subprocess.

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Philipp Stephani]

Am Sa., 20. Apr. 2019 um 09:31 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Fri, 19 Apr 2019 21:28:32 +0200
> > Cc: Emacs developers <emacs-devel@gnu.org>, Philipp Stephani <phst@google.com>
> >
> > > But I'm guessing that the current behavior was unexpected for some
> > > reason, and that's why you looked in the documentation.  If the guess
> > > is correct, could you describe why it was unexpected/unwanted?
> >
> > It's unexpected that Emacs writes something. This is an instance of
> > the general case that by default you expect nothing to happen instead
> > of something, unless you request the "something". For example, after
> > $ echo foo > bar
> > would you expect that the file "bar" contains content other than
> > "foo\n" because the shell decides to write additional text before
> > closing redirections? This is the same thing.
>
> But Emacs does that for ages, so in Emacs this is veteran behavior of
> the default sentinel function.

Sure, but the documentation doesn't state clearly that Emacs creates a
pipe process with the default sentinel if the caller passes a buffer,
so it's unexpected that the default sentinel runs.

>
> > The documentation for make-process doesn't state that if :stderr is a
> > buffer, make-process creates a pipe process with the default sentinel;
>
> Actually, it does, albeit in the parent section:
>
>      Alternatively, you could use the ‘:stderr’ parameter with a non-‘nil’
>   value in a call to ‘make-process’ (*note make-process: Asynchronous
>   Processes.) to make the destination of the error output separate from
>   the standard output; in that case, Emacs will use pipes for
>   communicating with the subprocess.

That seems rather subtle and even unrelated. The "pipe" here refers to
Unix pipes, not to Emacs pipe processes.
If you don't want to change the behavior, how about at least
clarifying the documentation? I can install a patch for that.

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sun, 21 Apr 2019 15:55:54 +0200
> Cc: Emacs developers <emacs-devel@gnu.org>, Philipp Stephani <phst@google.com>
> 
> >      Alternatively, you could use the ‘:stderr’ parameter with a non-‘nil’
> >   value in a call to ‘make-process’ (*note make-process: Asynchronous
> >   Processes.) to make the destination of the error output separate from
> >   the standard output; in that case, Emacs will use pipes for
> >   communicating with the subprocess.
> 
> That seems rather subtle and even unrelated.

I agree, to seome extent.

> If you don't want to change the behavior, how about at least
> clarifying the documentation? I can install a patch for that.

I didn't yet make up my mind, and would love to hear opinions from
others.

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Stefan Monnier]

>> > But I'm guessing that the current behavior was unexpected for some
>> > reason, and that's why you looked in the documentation.  If the guess
>> > is correct, could you describe why it was unexpected/unwanted?
>> 
>> It's unexpected that Emacs writes something. This is an instance of
>> the general case that by default you expect nothing to happen instead
>> of something, unless you request the "something". For example, after
>> $ echo foo > bar
>> would you expect that the file "bar" contains content other than
>> "foo\n" because the shell decides to write additional text before
>> closing redirections? This is the same thing.
>
> But Emacs does that for ages, so in Emacs this is veteran behavior of
> the default sentinel function.

FWIW, I don't much like this behavior and since the stderr case is new
I think we don't have to inherit this part of the behavior.


        Stefan

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sun, 21 Apr 2019 16:57:12 -0400
> 
> > But Emacs does that for ages, so in Emacs this is veteran behavior of
> > the default sentinel function.
> 
> FWIW, I don't much like this behavior and since the stderr case is new
> I think we don't have to inherit this part of the behavior.

By "this behavior" do you mean only the "normal" case, or also the
abnormal termination case, in which case we show the reason of the
abnormal termination?

Philipp's proposal, AFAIU, would remove the notification for stderr
even in the abnormal case, so we will have to rely on the stdout
sentinel to announce the reason.  Are we sure no information will be
lost through this assumption, i.e. are we sure both sentinels will
always make the same announcements?

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Stefan Monnier]

> By "this behavior" do you mean only the "normal" case, or also the
> abnormal termination case, in which case we show the reason of the
> abnormal termination?

The latter: there's no point showing the abnormal termination on both
outputs, I think.


        Stefan

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Mon, 22 Apr 2019 10:43:32 -0400
> Cc: emacs-devel@gnu.org
> 
> > By "this behavior" do you mean only the "normal" case, or also the
> > abnormal termination case, in which case we show the reason of the
> > abnormal termination?
> 
> The latter: there's no point showing the abnormal termination on both
> outputs, I think.

If it will show on both.  Are we sure it will?

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Philipp Stephani]

Am Mo., 22. Apr. 2019 um 17:27 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> > Date: Mon, 22 Apr 2019 10:43:32 -0400
> > Cc: emacs-devel@gnu.org
> >
> > > By "this behavior" do you mean only the "normal" case, or also the
> > > abnormal termination case, in which case we show the reason of the
> > > abnormal termination?
> >
> > The latter: there's no point showing the abnormal termination on both
> > outputs, I think.
>
> If it will show on both.  Are we sure it will?
>

A pipe can't terminate abnormally like a process, so the output will
always be "finished":

$ emacs -Q -batch -eval '(progn (make-process :name "test" :command
(list "/usr/bin/false") :stderr "*stderr*" :sentinel (function
ignore)) (sit-for 1) (with-current-buffer "*stderr*" (message "stderr
= %S" (buffer-string))))'
stderr = "
Process test stderr finished
"

Re: [PATCH 2/3] Don't print "process finished" into the stderr buffer.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 22 Apr 2019 17:45:15 +0200
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Emacs developers <emacs-devel@gnu.org>
> 
> A pipe can't terminate abnormally like a process

Are you sure?  What about conditions like EPIPE?

[PATCH 3/3] Inherit process output coding system to stderr process.

[Philipp Stephani]

* src/process.c (Fmake_process): Inherit output coding system to
newly-created pipe process.

* test/src/process-tests.el (make-process/stderr-coding)
(make-process/mixed-output-coding): New unit tests.
---
 src/process.c             |  7 ++++
 test/src/process-tests.el | 76 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/src/process.c b/src/process.c
index dcc9dcb31e..2b3e00d423 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1698,12 +1698,15 @@ usage: (make-process &rest ARGS)  */)
   bool query_on_exit = NILP (Fplist_get (contact, QCnoquery));
 
   stderrproc = Qnil;
+  bool stderr_inherit_coding_system;
   xstderr = Fplist_get (contact, QCstderr);
   if (PROCESSP (xstderr))
     {
       if (!PIPECONN_P (xstderr))
 	error ("Process is not a pipe process");
       stderrproc = xstderr;
+      /* Use the coding system passed to `make-pipe-process'.  */
+      stderr_inherit_coding_system = false;
     }
   else if (!NILP (xstderr))
     {
@@ -1717,6 +1720,8 @@ usage: (make-process &rest ARGS)  */)
 			  query_on_exit ? Qnil : Qt,
 			  QCsentinel,
 			  Qignore);
+      /* Inherit the output coding system from the main process.  */
+      stderr_inherit_coding_system = true;
     }
 
   proc = make_process (name);
@@ -1808,6 +1813,8 @@ usage: (make-process &rest ARGS)  */)
 	  val = XCAR (Vdefault_process_coding_system);
       }
     pset_decode_coding_system (XPROCESS (proc), val);
+    if (stderr_inherit_coding_system)
+      Fset_process_coding_system (stderrproc, val, Qno_conversion);
 
     if (!NILP (tem))
       {
diff --git a/test/src/process-tests.el b/test/src/process-tests.el
index 838ba78acb..828f4207e1 100644
--- a/test/src/process-tests.el
+++ b/test/src/process-tests.el
@@ -217,5 +217,81 @@ process-test-sentinel-wait-function-working-p
       (should (eq (process-exit-status process) 0))
       (should (equal (buffer-string) "stderr\n")))))
 
+(ert-deftest make-process/stderr-coding ()
+  "Check that `make-process' correctly encodes standard error."
+  (skip-unless (executable-find shell-file-name))
+  ;; The byte sequences printed to standard output and error are valid
+  ;; UTF-8 code unit sequences for "ä" and "ö", respectively.
+  ;; However, `make-process' should decode them both using
+  ;; `iso-latin-1-dos', as specified below.
+  (with-temp-buffer
+    (let ((stdout (current-buffer))
+          ;; `coding-system-for-read' should be ignored.
+          (coding-system-for-read 'utf-8-unix))
+      (with-temp-buffer
+        (let ((process (make-process
+                        :name "stderr-coding"
+                        :command (list shell-file-name shell-command-switch
+                                       (concat "echo -e '\\xC3\\xA4\\r'; "
+                                               "echo -e '\\xC3\\xB6\\r' >&2"))
+                        :buffer stdout
+                        :stderr (current-buffer)
+                        :sentinel #'ignore
+                        :coding 'iso-latin-1-dos
+                        :noquery t
+                        :connection-type 'pipe)))
+          (while (process-live-p process)
+            (accept-process-output process))
+          (let ((stderr-process (get-buffer-process (current-buffer))))
+            ;; FIXME: The following form should not be required.
+            (while (process-live-p stderr-process)
+              (accept-process-output stderr-process)))
+          (should (eq (process-status process) 'exit))
+          (should (eq (process-exit-status process) 0))
+          (should (equal (buffer-string) "\u00C3\u00B6\n"))))
+      (should (equal (buffer-string) "\u00C3\u00A4\n")))))
+
+(ert-deftest make-process/mixed-output-coding ()
+  "Check that `make-process' allows different coding systems for
+the output streams."
+  (skip-unless (executable-find shell-file-name))
+  ;; The byte sequences printed to standard output and error are valid
+  ;; UTF-8 code unit sequences for "ä" and "ö", respectively.
+  ;; However, `make-process' should decode them using the coding
+  ;; systems passed to `make-process' and `make-pipe-process',
+  ;; respectively.
+  (with-temp-buffer
+    (let ((stdout (current-buffer))
+          ;; `coding-system-for-read' should be ignored.
+          (coding-system-for-read 'utf-8-unix))
+      (with-temp-buffer
+        (let* ((stderr-process (make-pipe-process
+                                :name "stderr-coding stderr"
+                                :buffer (current-buffer)
+                                :sentinel #'ignore
+                                :coding 'iso-latin-1-unix
+                                :noquery t))
+               (process (make-process
+                         :name "stderr-coding"
+                         :command (list shell-file-name shell-command-switch
+                                        (concat "echo -e '\\xC3\\xA4\\r'; "
+                                                "echo -e '\\xC3\\xB6\\r' >&2"))
+                         :buffer stdout
+                         :stderr stderr-process
+                         :sentinel #'ignore
+                         :coding 'iso-latin-1-dos
+                         :noquery t
+                         :connection-type 'pipe)))
+          (while (process-live-p process)
+            (accept-process-output process))
+          ;; FIXME: Should the following be required?
+          (while (process-live-p stderr-process)
+            (accept-process-output stderr-process))
+          (should (eq (process-status process) 'exit))
+          (should (eq (process-exit-status process) 0))
+          (should (eq (process-status stderr-process) 'closed))
+          (should (equal (buffer-string) "\u00C3\u00B6\r\n"))))
+      (should (equal (buffer-string) "\u00C3\u00A4\n")))))
+
 (provide 'process-tests)
 ;; process-tests.el ends here.
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 3/3] Inherit process output coding system to stderr process.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Wed,  4 Apr 2018 14:02:18 +0200
> Cc: Philipp Stephani <phst@google.com>
> 
> * src/process.c (Fmake_process): Inherit output coding system to
> newly-created pipe process.

I'm sorry, I don't understand the need for this "inheriting".  If the
problem is that make-process and make-pipe-process use different logic
to decide on the default coding-systems, then I think we should make
them use the same logic, and then there will be no need for
"inheriting".  Or is there something else I'm missing?

> +        (let ((process (make-process
> +                        :name "stderr-coding"
> +                        :command (list shell-file-name shell-command-switch
> +                                       (concat "echo -e '\\xC3\\xA4\\r'; "
> +                                               "echo -e '\\xC3\\xB6\\r' >&2"))

This shell command is non-portable.  I think even "echo -e" is not
portable enough, let alone with hex escapes and the trailing \r.
Can't we use Emacs instead?  There's also the ";" issue again.

> +          (should (equal (buffer-string) "\u00C3\u00B6\n"))))
> +      (should (equal (buffer-string) "\u00C3\u00A4\n")))))

Why not use literal characters here?  It will make the source more
readable, IMO.

Re: [PATCH 3/3] Inherit process output coding system to stderr process.

[Philipp Stephani]

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

Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 4. Apr. 2018 um 15:39 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Wed,  4 Apr 2018 14:02:18 +0200
> > Cc: Philipp Stephani <phst@google.com>
> >
> > * src/process.c (Fmake_process): Inherit output coding system to
> > newly-created pipe process.
>
> I'm sorry, I don't understand the need for this "inheriting".  If the
> problem is that make-process and make-pipe-process use different logic
> to decide on the default coding-systems, then I think we should make
> them use the same logic, and then there will be no need for
> "inheriting".  Or is there something else I'm missing?
>

At least I would expect Emacs to use the same output encoding for both
standard output and error streams if an explicit output encoding is
provided, given that most external programs will also use the same encoding
for both streams. But currently Emacs uses the default encoding for the
standard error stream if it creates the pipe itself.


>
> > +        (let ((process (make-process
> > +                        :name "stderr-coding"
> > +                        :command (list shell-file-name
> shell-command-switch
> > +                                       (concat "echo -e
> '\\xC3\\xA4\\r'; "
> > +                                               "echo -e '\\xC3\\xB6\\r'
> >&2"))
>
> This shell command is non-portable.  I think even "echo -e" is not
> portable enough, let alone with hex escapes and the trailing \r.
> Can't we use Emacs instead?  There's also the ";" issue again.
>

Is it possible to write raw bytes to the standard output/error streams
using Emacs?


>
> > +          (should (equal (buffer-string) "\u00C3\u00B6\n"))))
> > +      (should (equal (buffer-string) "\u00C3\u00A4\n")))))
>
> Why not use literal characters here?  It will make the source more
> readable, IMO.
>

I don't care much, but using the character escapes here makes it obvious
that the output is decoded as Latin-1.

[-- Attachment #2: Type: text/html, Size: 2998 bytes --]

Re: [PATCH 3/3] Inherit process output coding system to stderr process.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 07 Apr 2018 21:12:10 +0000
> Cc: emacs-devel@gnu.org, phst@google.com
> 
>  > * src/process.c (Fmake_process): Inherit output coding system to
>  > newly-created pipe process.
> 
>  I'm sorry, I don't understand the need for this "inheriting".  If the
>  problem is that make-process and make-pipe-process use different logic
>  to decide on the default coding-systems, then I think we should make
>  them use the same logic, and then there will be no need for
>  "inheriting".  Or is there something else I'm missing?
> 
> At least I would expect Emacs to use the same output encoding for both standard output and error streams if
> an explicit output encoding is provided, given that most external programs will also use the same encoding for
> both streams. But currently Emacs uses the default encoding for the standard error stream if it creates the
> pipe itself.

But that's easy to fix: just pass the same :coding argument with which
make-process was invoked to Fmake_pipe_process.  (And make the code
which determines the encoding in both functions be identical while at
that.)

>  > +        (let ((process (make-process
>  > +                        :name "stderr-coding"
>  > +                        :command (list shell-file-name shell-command-switch
>  > +                                       (concat "echo -e '\\xC3\\xA4\\r'; "
>  > +                                               "echo -e '\\xC3\\xB6\\r' >&2"))
> 
>  This shell command is non-portable.  I think even "echo -e" is not
>  portable enough, let alone with hex escapes and the trailing \r.
>  Can't we use Emacs instead?  There's also the ";" issue again.
> 
> Is it possible to write raw bytes to the standard output/error streams using Emacs?

Yes, use raw-text or no-conversion as the encoding (the former allows
to produce DOS and Mac EOLs, the latter doesn't).

>  > +          (should (equal (buffer-string) "\u00C3\u00B6\n"))))
>  > +      (should (equal (buffer-string) "\u00C3\u00A4\n")))))
> 
>  Why not use literal characters here?  It will make the source more
>  readable, IMO.
> 
> I don't care much, but using the character escapes here makes it obvious that the output is decoded as
> Latin-1. 

On the contrary, it might miss that if the output is decoded as raw
bytes.  Using the actual characters, OTOH, will make sure that, I
think.

Re: [PATCH 3/3] Inherit process output coding system to stderr process.

[Eli Zaretskii]

> Date: Sun, 08 Apr 2018 16:18:10 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: phst@google.com, emacs-devel@gnu.org
> 
> > Is it possible to write raw bytes to the standard output/error streams using Emacs?
> 
> Yes, use raw-text or no-conversion as the encoding (the former allows
> to produce DOS and Mac EOLs, the latter doesn't).

Just in case it is relevant and helpful: the function insert-byte
could help you insert arbitrary raw bytes into a buffer.

Re: [PATCH 1/3] Document that 'make-process' mixes the output streams

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Wed,  4 Apr 2018 14:02:16 +0200
> Cc: Philipp Stephani <phst@google.com>
> 
> * doc/lispref/processes.texi (Asynchronous Processes):
> * src/process.c (Fmake_process): Document that standard error is mixed
> with standard output if STDERR is nil.

Thanks.

> +(ert-deftest make-process/mix-stderr ()
> +  "Check that ‘make-process’ mixes the output streams if STDERR is nil."
> +  (skip-unless (executable-find shell-file-name))
> +  (with-temp-buffer
> +    (let ((process (make-process
> +                    :name "mix-stderr"
> +                    :command (list shell-file-name shell-command-switch
> +                                   "echo stdout; echo stderr >&2")
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This shell command is non-portable: on Windows you have to use "&"
instead of ";" to chain commands.  You could use "&&", which in this
case will do the same on both Posix and Windows systems.

Re: [PATCH 1/3] Document that 'make-process' mixes the output streams

[Philipp Stephani]

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

Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 4. Apr. 2018 um 15:12 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Wed,  4 Apr 2018 14:02:16 +0200
> > Cc: Philipp Stephani <phst@google.com>
> >
> > * doc/lispref/processes.texi (Asynchronous Processes):
> > * src/process.c (Fmake_process): Document that standard error is mixed
> > with standard output if STDERR is nil.
>
> Thanks.
>
> > +(ert-deftest make-process/mix-stderr ()
> > +  "Check that ‘make-process’ mixes the output streams if STDERR is nil."
> > +  (skip-unless (executable-find shell-file-name))
> > +  (with-temp-buffer
> > +    (let ((process (make-process
> > +                    :name "mix-stderr"
> > +                    :command (list shell-file-name shell-command-switch
> > +                                   "echo stdout; echo stderr >&2")
>                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This shell command is non-portable: on Windows you have to use "&"
> instead of ";" to chain commands.  You could use "&&", which in this
> case will do the same on both Posix and Windows systems.
>

Done and pushed to master.

[-- Attachment #2: Type: text/html, Size: 1785 bytes --]