Re: [PATCH] Generalize start-process with keyword args

[Daiki Ueno]

Eli Zaretskii <address@hidden> writes:

>> No, just that it will treat ":coding nil" in the same way as the absence
>> of a :coding arg (i.e. fallback on some default mechanism).
>
> IMO, it will be terribly confusing to have incompatible treatment of
> nil in this one API.

I tend to agree with Stefan, as I find no documentation about the
implication of no-conversion for ":coding nil" except in the C source
code.  Maybe we could add the exception back if it turns out to be too
confusing.

If there are no other problems, I'm going to use the attaching patch
which addressed the suggestions (thanks for that!).

Regards,
-- 
Daiki Ueno

>From 206196c18652601920017b3a30316ac4205b42dd Mon Sep 17 00:00:00 2001
From: Daiki Ueno <address@hidden>
Date: Mon, 16 Mar 2015 11:38:05 +0900
Subject: [PATCH] Generalize start-process with keyword args

* src/process.c (Fmake_process): New function.
(create_process, create_pty): Check p->pty_flag instead of
Vprocess_connection_type.
(syms_of_process): Register QCcommand, QCconnection_type, Qpty,
Qpipe, and Smake_process.

* lisp/subr.el (start-process): New function, ported from the C
implementation.

* doc/lispref/processes.texi (Asynchronous Processes): Mention
`make-process'.

* etc/NEWS: Mention `make-process'.
---
 doc/lispref/ChangeLog      |   4 +
 doc/lispref/processes.texi |  57 ++++++++++++++
 etc/ChangeLog              |   4 +
 etc/NEWS                   |   3 +
 lisp/ChangeLog             |   5 ++
 lisp/subr.el               |  22 ++++++
 src/ChangeLog              |   8 ++
 src/process.c              | 187 +++++++++++++++++++++++++++++++++------------
 8 files changed, 241 insertions(+), 49 deletions(-)

diff --git a/doc/lispref/ChangeLog b/doc/lispref/ChangeLog
index 260656c..06d4630 100644
--- a/doc/lispref/ChangeLog
+++ b/doc/lispref/ChangeLog
@@ -1,3 +1,7 @@
+2015-03-16  Daiki Ueno  <address@hidden>
+
+       * processes.texi (Asynchronous Processes): Mention `make-process'.
+
 2015-03-09  Nicolas Petton <address@hidden>
 
        * sequences.texi (seq-into): Add documentation for the new
diff --git a/doc/lispref/processes.texi b/doc/lispref/processes.texi
index 177cd68..337669d 100644
--- a/doc/lispref/processes.texi
+++ b/doc/lispref/processes.texi
@@ -692,6 +692,63 @@ use the function @code{process-tty-name} (@pxref{Process
 Information}).
 @end defvar
 
address@hidden make-process &rest args
+This function is like @code{start-process}, but takes keyword arguments.
+
+The arguments @var{args} are a list of keyword/argument pairs.
+Omitting a keyword is always equivalent to specifying it with value
address@hidden  Here are the meaningful keywords:
+
address@hidden @asis
address@hidden :name @var{name}
+Use the string @var{name} as the process name.  It is modified if
+necessary to make it unique.
+
address@hidden :buffer @var{buffer}
+Use @var{buffer} as the process buffer.
+
address@hidden :command @var{command}
+Use @var{command} as the command line of the process.  @var{command}
+is a list starting with the program's executable file name, followed
+by strings to give to program as arguments.
+
address@hidden :coding @var{coding}
+If @var{coding} is a symbol, it specifies the coding system to be
+used for both reading and writing of data from and to the
+connection.  If @var{coding} is a cons cell
address@hidden@code{(@var{decoding} . @var{encoding})}}, then @var{decoding}
+will be used for reading and @var{encoding} for writing.
+
+If @var{coding} is @code{nil}, the coding system chosen for decoding
+output is @code{undecided}, meaning deduce the encoding from the
+actual data.
address@hidden from Processes}.
+
address@hidden :connection-type @var{TYPE}
+Initialize the type of device used to communicate with the subprocess.
+Possible values are @code{pty} to use a pty, @code{pipe} to use a
+pipe, or @code{nil} to use the default derived from the value of
+the @code{process-connection-type} variable.
+
address@hidden :noquery @var{query-flag}
+Initialize the process query flag to @var{query-flag}.
address@hidden Before Exit}.
+
address@hidden :stop @var{stopped}
+If @var{stopped} is address@hidden, start the process in the
+``stopped'' state.
+
address@hidden :filter @var{filter}
+Initialize the process filter to @var{filter}.
+
address@hidden :sentinel @var{sentinel}
+Initialize the process sentinel to @var{sentinel}.
address@hidden table
+
+The original argument list, modified with the actual connection
+information, is available via the @code{process-contact} function.
address@hidden defun
+
 @node Deleting Processes
 @section Deleting Processes
 @cindex deleting processes
diff --git a/etc/ChangeLog b/etc/ChangeLog
index cd5c540..5d5a47f 100644
--- a/etc/ChangeLog
+++ b/etc/ChangeLog
@@ -1,3 +1,7 @@
+2015-03-18  Daiki Ueno  <address@hidden>
+
+       * NEWS: Mention `make-process'.
+
 2015-03-03  Kelvin White  <address@hidden>
 
        * NEWS.24: Add section to include ERC changes.
diff --git a/etc/NEWS b/etc/NEWS
index 75d55de..8cfc238 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -623,6 +623,9 @@ word syntax, use `\sw' instead.
 
 * Lisp Changes in Emacs 25.1
 
+** New function make-process provides a generalized Lisp interface to
+   subprocess creation.
+
 ** Emacs Lisp now supports generators.
 
 ** New finalizer facility for running code when objects
diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 747a1d6..835712c 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,8 @@
+2015-03-17  Daiki Ueno  <address@hidden>
+
+       * subr.el (start-process): New function, ported from the C
+       implementation.
+
 2015-03-15  Michael Albinus  <address@hidden>
 
        * net/tramp-adb.el:
diff --git a/lisp/subr.el b/lisp/subr.el
index deadca6..5d763f9 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1901,6 +1901,28 @@ and the file name is displayed in the echo area."
 
 ;;;; Process stuff.
 
+(defun start-process (name buffer program &rest program-args)
+  "Start a program in a subprocess.  Return the process object for it.
+NAME is name for process.  It is modified if necessary to make it unique.
+BUFFER is the buffer (or buffer name) to associate with the process.
+
+Process output (both standard output and standard error streams) goes
+at end of BUFFER, unless you specify an output stream or filter
+function to handle the output.  BUFFER may also be nil, meaning that
+this process is not associated with any buffer.
+
+PROGRAM is the program file name.  It is searched for in `exec-path'
+\(which see).  If nil, just associate a pty with the buffer.  Remaining
+arguments are strings to give program as arguments.
+
+If you want to separate standard output from standard error, invoke
+the command through a shell and redirect one of them using the shell
+syntax."
+  (unless (fboundp 'make-process)
+    (error "Emacs was compiled without subprocess support"))
+  (make-process :name name :buffer buffer
+               :command (cons program program-args)))
+
 (defun process-lines (program &rest args)
   "Execute PROGRAM with ARGS, returning its output as a list of lines.
 Signal an error if the program returns with a non-zero exit status."
diff --git a/src/ChangeLog b/src/ChangeLog
index e328afc..8f9a5ee 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,11 @@
+2015-03-17  Daiki Ueno  <address@hidden>
+
+       * process.c (Fmake_process): New function.
+       (create_process, create_pty): Check p->pty_flag instead of
+       Vprocess_connection_type.
+       (syms_of_process): Register QCcommand, QCconnection_type, Qpty,
+       Qpipe, and Smake_process.  Unregister Sstart_process.
+
 2015-03-15  Eli Zaretskii  <address@hidden>
 
        * xdisp.c (handle_invisible_prop): Fix up it->position even when
diff --git a/src/process.c b/src/process.c
index 1d935ba..6068540 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1355,34 +1355,65 @@ DEFUN ("process-list", Fprocess_list, Sprocess_list, 0, 
0, 0,
 
 static void start_process_unwind (Lisp_Object proc);
 
-DEFUN ("start-process", Fstart_process, Sstart_process, 3, MANY, 0,
+DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0,
        doc: /* Start a program in a subprocess.  Return the process object for 
it.
-NAME is name for process.  It is modified if necessary to make it unique.
-BUFFER is the buffer (or buffer name) to associate with the process.
 
-Process output (both standard output and standard error streams) goes
-at end of BUFFER, unless you specify an output stream or filter
-function to handle the output.  BUFFER may also be nil, meaning that
-this process is not associated with any buffer.
+This is similar to `start-process', but arguments are specified as
+keyword/argument pairs.  The following arguments are defined:
 
-PROGRAM is the program file name.  It is searched for in `exec-path'
-(which see).  If nil, just associate a pty with the buffer.  Remaining
-arguments are strings to give program as arguments.
+:name NAME -- NAME is name for process.  It is modified if necessary
+to make it unique.
+
+:buffer BUFFER -- BUFFER is the buffer (or buffer-name) to associate
+with the process.  Process output goes at end of that buffer, unless
+you specify an output stream or filter function to handle the output.
+BUFFER may be also nil, meaning that this process is not associated
+with any buffer.
+
+:command COMMAND -- COMMAND is a list starting with the program file
+name, followed by strings to give to the program as arguments.
 
-If you want to separate standard output from standard error, invoke
-the command through a shell and redirect one of them using the shell
-syntax.
+:coding CODING -- If CODING is a symbol, it specifies the coding
+system used for both reading and writing for this process.  If CODING
+is a cons (DECODING . ENCODING), DECODING is used for reading, and
+ENCODING is used for writing.
 
-usage: (start-process NAME BUFFER PROGRAM &rest PROGRAM-ARGS)  */)
+:noquery BOOL -- When exiting Emacs, query the user if BOOL is nil and
+the process is running.  If BOOL is not given, query before exiting.
+
+:stop BOOL -- Start process in the `stopped' state if BOOL non-nil.
+In the stopped state, a process does not accept incoming data, but you
+can send outgoing data.  The stopped state is cleared by
+`continue-process' and set by `stop-process'.
+
+:connection-type TYPE -- TYPE is control type of device used to
+communicate with subprocesses.  Values are `pipe' to use a pipe, `pty'
+to use a pty, or nil to use the default specified through
+`process-connection-type'.
+
+:filter FILTER -- Install FILTER as the process filter.
+
+:sentinel SENTINEL -- Install SENTINEL as the process sentinel.
+
+usage: (make-process &rest ARGS)  */)
   (ptrdiff_t nargs, Lisp_Object *args)
 {
-  Lisp_Object buffer, name, program, proc, current_dir, tem;
+  Lisp_Object buffer, name, command, program, proc, contact, current_dir, tem;
   unsigned char **new_argv;
+  ptrdiff_t new_argc;
   ptrdiff_t i;
   ptrdiff_t count = SPECPDL_INDEX ();
+  struct gcpro gcpro1;
   USE_SAFE_ALLOCA;
 
-  buffer = args[1];
+  if (nargs == 0)
+    return Qnil;
+
+  /* Save arguments for process-contact and clone-process.  */
+  contact = Flist (nargs, args);
+  GCPRO1 (contact);
+
+  buffer = Fplist_get (contact, QCbuffer);
   if (!NILP (buffer))
     buffer = Fget_buffer_create (buffer);
 
@@ -1402,10 +1433,11 @@ usage: (start-process NAME BUFFER PROGRAM &rest 
PROGRAM-ARGS)  */)
     UNGCPRO;
   }
 
-  name = args[0];
+  name = Fplist_get (contact, QCname);
   CHECK_STRING (name);
 
-  program = args[2];
+  command = Fplist_get (contact, QCcommand);
+  program = XCAR (command);
 
   if (!NILP (program))
     CHECK_STRING (program);
@@ -1423,7 +1455,25 @@ usage: (start-process NAME BUFFER PROGRAM &rest 
PROGRAM-ARGS)  */)
   pset_buffer (XPROCESS (proc), buffer);
   pset_sentinel (XPROCESS (proc), Qinternal_default_process_sentinel);
   pset_filter (XPROCESS (proc), Qinternal_default_process_filter);
-  pset_command (XPROCESS (proc), Flist (nargs - 2, args + 2));
+  pset_command (XPROCESS (proc), Fcopy_sequence (command));
+
+  if (tem = Fplist_get (contact, QCnoquery), !NILP (tem))
+    XPROCESS (proc)->kill_without_query = 1;
+  if (tem = Fplist_get (contact, QCstop), !NILP (tem))
+    pset_command (XPROCESS (proc), Qt);
+
+  tem = Fplist_get (contact, QCconnection_type);
+  if (!NILP (tem))
+    {
+      if (EQ (tem, Qpty))
+       XPROCESS (proc)->pty_flag = 1;
+      else if (EQ (tem, Qpipe))
+       XPROCESS (proc)->pty_flag = 0;
+      else
+       report_file_error ("Unknown connection type", tem);
+    }
+  else
+    XPROCESS (proc)->pty_flag = !NILP (Vprocess_connection_type);
 
 #ifdef HAVE_GNUTLS
   /* AKA GNUTLS_INITSTAGE(proc).  */
@@ -1453,15 +1503,30 @@ usage: (start-process NAME BUFFER PROGRAM &rest 
PROGRAM-ARGS)  */)
     Lisp_Object val, *args2;
     struct gcpro gcpro1, gcpro2;
 
-    val = Vcoding_system_for_read;
+    tem = Fplist_get (contact, QCcoding);
+    val = Qnil;
+    if (!NILP (tem))
+      {
+       val = tem;
+       if (CONSP (val))
+         val = XCAR (val);
+      }
+    else
+      val = Vcoding_system_for_read;
     if (NILP (val))
       {
-       SAFE_ALLOCA_LISP (args2, nargs + 1);
-       args2[0] = Qstart_process;
-       for (i = 0; i < nargs; i++) args2[i + 1] = args[i];
+       ptrdiff_t nargs2 = 3 + XINT (Flength (command));
+       Lisp_Object tem2;
+       SAFE_ALLOCA_LISP (args2, nargs2);
+       i = 0;
+       args2[i++] = Qstart_process;
+       args2[i++] = name;
+       args2[i++] = buffer;
+       for (tem2 = command; CONSP (tem2); tem2 = XCDR (tem2))
+         args2[i++] = XCAR (tem2);
        GCPRO2 (proc, current_dir);
        if (!NILP (program))
-         coding_systems = Ffind_operation_coding_system (nargs + 1, args2);
+         coding_systems = Ffind_operation_coding_system (nargs2, args2);
        UNGCPRO;
        if (CONSP (coding_systems))
          val = XCAR (coding_systems);
@@ -1470,17 +1535,31 @@ usage: (start-process NAME BUFFER PROGRAM &rest 
PROGRAM-ARGS)  */)
       }
     pset_decode_coding_system (XPROCESS (proc), val);
 
-    val = Vcoding_system_for_write;
+    val = Qnil;
+    if (!NILP (tem))
+      {
+       val = tem;
+       if (CONSP (val))
+         val = XCDR (val);
+      }
+    else
+      val = Vcoding_system_for_write;
     if (NILP (val))
       {
        if (EQ (coding_systems, Qt))
          {
-           SAFE_ALLOCA_LISP (args2, nargs + 1);
-           args2[0] = Qstart_process;
-           for (i = 0; i < nargs; i++) args2[i + 1] = args[i];
+           ptrdiff_t nargs2 = 3 + XINT (Flength (command));
+           Lisp_Object tem2;
+           SAFE_ALLOCA_LISP (args2, nargs2);
+           i = 0;
+           args2[i++] = Qstart_process;
+           args2[i++] = name;
+           args2[i++] = buffer;
+           for (tem2 = command; CONSP (tem2); tem2 = XCDR (tem2))
+             args2[i++] = XCAR (tem2);
            GCPRO2 (proc, current_dir);
            if (!NILP (program))
-             coding_systems = Ffind_operation_coding_system (nargs + 1, args2);
+             coding_systems = Ffind_operation_coding_system (nargs2, args2);
            UNGCPRO;
          }
        if (CONSP (coding_systems))
@@ -1512,10 +1591,10 @@ usage: (start-process NAME BUFFER PROGRAM &rest 
PROGRAM-ARGS)  */)
          && !(SCHARS (program) > 1
               && IS_DEVICE_SEP (SREF (program, 1))))
        {
-         struct gcpro gcpro1, gcpro2, gcpro3, gcpro4;
+         struct gcpro gcpro1, gcpro2;
 
          tem = Qnil;
-         GCPRO4 (name, program, buffer, current_dir);
+         GCPRO2 (buffer, current_dir);
          openp (Vexec_path, program, Vexec_suffixes, &tem,
                 make_number (X_OK), false);
          UNGCPRO;
@@ -1534,31 +1613,32 @@ usage: (start-process NAME BUFFER PROGRAM &rest 
PROGRAM-ARGS)  */)
       tem = remove_slash_colon (tem);
 
       {
-       Lisp_Object arg_encoding = Qnil;
+       Lisp_Object arg_encoding = Qnil, tem2;
        struct gcpro gcpro1;
+
+       tem = Fcons (tem, Fcopy_sequence (XCDR (command)));
        GCPRO1 (tem);
 
        /* Encode the file name and put it in NEW_ARGV.
           That's where the child will use it to execute the program.  */
-       tem = list1 (ENCODE_FILE (tem));
+       XSETCAR (tem, ENCODE_FILE (XCAR (tem)));
 
        /* Here we encode arguments by the coding system used for sending
           data to the process.  We don't support using different coding
           systems for encoding arguments and for encoding data sent to the
           process.  */
 
-       for (i = 3; i < nargs; i++)
+       for (tem2 = XCDR (tem); CONSP (tem2); tem2 = XCDR (tem2))
          {
-           tem = Fcons (args[i], tem);
-           CHECK_STRING (XCAR (tem));
-           if (STRING_MULTIBYTE (XCAR (tem)))
+           CHECK_STRING (XCAR (tem2));
+           if (STRING_MULTIBYTE (XCAR (tem2)))
              {
                if (NILP (arg_encoding))
                  arg_encoding = (complement_process_encoding_system
                                  (XPROCESS (proc)->encode_coding_system));
-               XSETCAR (tem,
+               XSETCAR (tem2,
                         code_convert_string_norecord
-                        (XCAR (tem), arg_encoding, 1));
+                        (XCAR (tem2), arg_encoding, 1));
              }
          }
 
@@ -1567,10 +1647,11 @@ usage: (start-process NAME BUFFER PROGRAM &rest 
PROGRAM-ARGS)  */)
 
       /* Now that everything is encoded we can collect the strings into
         NEW_ARGV.  */
-      SAFE_NALLOCA (new_argv, 1, nargs - 1);
-      new_argv[nargs - 2] = 0;
+      new_argc = XINT (Flength (tem));
+      SAFE_NALLOCA (new_argv, 1, new_argc + 1);
+      new_argv[new_argc] = 0;
 
-      for (i = nargs - 2; i-- != 0; )
+      for (i = 0; i < new_argc; i++)
        {
          new_argv[i] = SDATA (XCAR (tem));
          tem = XCDR (tem);
@@ -1581,6 +1662,7 @@ usage: (start-process NAME BUFFER PROGRAM &rest 
PROGRAM-ARGS)  */)
   else
     create_pty (proc);
 
+  UNGCPRO;
   SAFE_FREE ();
   return unbind_to (count, proc);
 }
@@ -1648,7 +1730,7 @@ create_process (Lisp_Object process, char **new_argv, 
Lisp_Object current_dir)
 
   inchannel = outchannel = -1;
 
-  if (!NILP (Vprocess_connection_type))
+  if (p->pty_flag)
     outchannel = inchannel = allocate_pty (pty_name);
 
   if (inchannel >= 0)
@@ -1692,6 +1774,8 @@ create_process (Lisp_Object process, char **new_argv, 
Lisp_Object current_dir)
   chan_process[inchannel] = process;
   p->infd = inchannel;
   p->outfd = outchannel;
+  if (inchannel > max_process_desc)
+    max_process_desc = inchannel;
 
   /* Previously we recorded the tty descriptor used in the subprocess.
      It was only used for getting the foreground tty process, so now
@@ -1701,10 +1785,11 @@ create_process (Lisp_Object process, char **new_argv, 
Lisp_Object current_dir)
   p->pty_flag = pty_flag;
   pset_status (p, Qrun);
 
-  FD_SET (inchannel, &input_wait_mask);
-  FD_SET (inchannel, &non_keyboard_wait_mask);
-  if (inchannel > max_process_desc)
-    max_process_desc = inchannel;
+  if (!EQ (p->command, Qt))
+    {
+      FD_SET (inchannel, &input_wait_mask);
+      FD_SET (inchannel, &non_keyboard_wait_mask);
+    }
 
   /* This may signal an error.  */
   setup_process_coding_systems (process);
@@ -1894,7 +1979,7 @@ create_pty (Lisp_Object process)
 {
   struct Lisp_Process *p = XPROCESS (process);
   char pty_name[PTY_NAME_SIZE];
-  int pty_fd = NILP (Vprocess_connection_type) ? -1 : allocate_pty (pty_name);
+  int pty_fd = !p->pty_flag ? -1 : allocate_pty (pty_name);
 
   if (pty_fd >= 0)
     {
@@ -7269,6 +7354,10 @@ syms_of_process (void)
   DEFSYM (QCstop, ":stop");
   DEFSYM (QCoptions, ":options");
   DEFSYM (QCplist, ":plist");
+  DEFSYM (QCcommand, ":command");
+  DEFSYM (QCconnection_type, ":connection-type");
+  DEFSYM (Qpty, "pty");
+  DEFSYM (Qpipe, "pipe");
 
   DEFSYM (Qlast_nonmenu_event, "last-nonmenu-event");
 
@@ -7371,7 +7460,7 @@ The variable takes effect when `start-process' is called. 
 */);
   defsubr (&Sprocess_plist);
   defsubr (&Sset_process_plist);
   defsubr (&Sprocess_list);
-  defsubr (&Sstart_process);
+  defsubr (&Smake_process);
   defsubr (&Sserial_process_configure);
   defsubr (&Smake_serial_process);
   defsubr (&Sset_network_process_option);
-- 
2.1.0