bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug


From: Philipp Stephani
Subject: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
Date: Sat, 22 Dec 2018 22:05:41 +0100

Am Di., 18. Dez. 2018 um 02:57 Uhr schrieb Alan Mackenzie <acm@muc.de>:
>
> Hello, Philipp,
>
> I hope I'm not late to the party, here, but....
>
> What is this patch about, and what is it for?

It fixes the bug referenced in the commit message (bug#28691).

>
> In article <mailman.5866.1545079749.1284.bug-gnu-emacs@gnu.org> you wrote:
> > * src/process.c (Fmake_process): Add new keyword argument
> > ':file-handler'.
> > (syms_of_process) <make-process, :file-handler>: Define new symbols.
>
> > * lisp/files.el (file-name-non-special): Add support for
> > 'make-process'.
>
> > * test/src/process-tests.el (make-process/file-handler/found)
> > (make-process/file-handler/not-found)
> > (make-process/file-handler/disable): New unit tests.
> > (process-tests--file-handler): New helper function.
>
> > * test/lisp/files-tests.el
> > (files-tests-file-name-non-special-make-process): New unit test.
>
> > * doc/lispref/files.texi (Magic File Names): Document that
> > 'make-process' can invoke file name handlers.
>
> > * doc/lispref/processes.texi (Asynchronous Processes): Document
> > ':file-handlers' argument to 'make-process'.
> > ---
> >  doc/lispref/files.texi     |  2 ++
> >  doc/lispref/processes.texi | 10 ++++++--
> >  etc/NEWS                   |  5 ++++
> >  lisp/files.el              | 11 +++++++--
> >  src/process.c              | 17 +++++++++++++
> >  test/lisp/files-tests.el   | 10 ++++++++
> >  test/src/process-tests.el  | 49 ++++++++++++++++++++++++++++++++++++++
> >  7 files changed, 100 insertions(+), 4 deletions(-)
>
> > diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
> > index b795864815..00e7fc1841 100644
> > --- a/doc/lispref/files.texi
> > +++ b/doc/lispref/files.texi
> > @@ -3171,6 +3171,7 @@ Magic File Names
> >  @code{make-directory},
> >  @code{make-directory-internal},
> >  @code{make-nearby-temp-file},
> > +@code{make-process},
> >  @code{make-symbolic-link},@*
> >  @code{process-file},
> >  @code{rename-file}, @code{set-file-acl}, @code{set-file-modes},
> > @@ -3227,6 +3228,7 @@ Magic File Names
> >  @code{make-auto-save-file-name},
> >  @code{make-direc@discretionary{}{}{}tory},
> >  @code{make-direc@discretionary{}{}{}tory-internal},
> > +@code{make-process},
> >  @code{make-symbolic-link},
> >  @code{process-file},
> >  @code{rename-file}, @code{set-file-acl}, @code{set-file-modes},
> > diff --git a/doc/lispref/processes.texi b/doc/lispref/processes.texi
> > index d88c7fbe62..92dc9df22e 100644
> > --- a/doc/lispref/processes.texi
> > +++ b/doc/lispref/processes.texi
> > @@ -696,6 +696,12 @@ Asynchronous Processes
> >  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}.
> > +
> > +@item :file-handler @var{file-handler}
> > +If @var{file-handler} is non-@code{nil}, then look for a file name
> > +handler for the current buffer's @code{default-directory}, and invoke
> > +that file handler to make the process.  If there is no such handler,
> > +proceed as if @var{file-handler} were @code{nil}.
> >  @end table
>
> Are we talking about a "file name handler" or a "file handler", here?

Those terms are synonyms.
https://www.gnu.org/software/emacs/manual/html_node/elisp/Magic-File-Names.html
uses the terms interchangeably.

> Any chance of consistency?  What is one of these?  In what way does it
> "handle" the file, or the file name?
>
> What is the domain of the search which takes place?  How is such a
> handler recognised?
>
> How is this handler "invoked"?  What arguments does it take?

It behaves exactly as described in
https://www.gnu.org/software/emacs/manual/html_node/elisp/Magic-File-Names.html.
I don't think there's a need to repeat that information everywhere
where a file handlers gets invoked. See e.g. the docstring of
start-file-process.

>
> Apologies if these things are already explained in nearby text.  I
> haven't read any of the context beyond what's in the patch.
>
> >  The original argument list, modified with the actual connection
> > @@ -704,8 +710,8 @@ Asynchronous Processes
> >  The current working directory of the subprocess is set to the current
> >  buffer's value of @code{default-directory} if that is local (as
> >  determined by `unhandled-file-name-directory'), or "~" otherwise.  If
> > -you want to run a process in a remote directory use
> > -@code{start-file-process}.
> > +you want to run a process in a remote directory, pass
> > +@code{:file-handler t} to @code{make-process}.
> >  @end defun
> >
> >  @defun make-pipe-process &rest args
> > diff --git a/etc/NEWS b/etc/NEWS
> > index c88f6ef5ca..2987937064 100644
> > --- a/etc/NEWS
> > +++ b/etc/NEWS
> > @@ -1407,6 +1407,11 @@ un-obsoleting it.
> >  +++
> >  ** New function 'group-name' returns a group name corresponding to GID.
> >
> > +** 'make-process' now takes a keyword argument ':file-handler'; if
> > +that is non-nil, it will look for a file name handler for the current
> > +buffer's 'default-directory' and invoke that file handler to make the
> > +process.  That way 'make-process' can start remote processes.
> > +
> >  ^L
> >  * Changes in Emacs 27.1 on Non-Free Operating Systems
> >
> > diff --git a/lisp/files.el b/lisp/files.el
> > index fb6cf0193a..448df62710 100644
> > --- a/lisp/files.el
> > +++ b/lisp/files.el
> > @@ -7103,7 +7103,8 @@ file-name-non-special
> >          (default-directory
> >           (if (memq operation
> >                      '(insert-directory process-file start-file-process
> > -                                       shell-command 
> > temporary-file-directory))
> > +                                       make-process shell-command
> > +                                       temporary-file-directory))
> >               (directory-file-name
> >                (expand-file-name
> >                 (unhandled-file-name-directory default-directory)))
> > @@ -7151,7 +7152,13 @@ file-name-non-special
> >                            ;; These file-notify-* operations take a
> >                            ;; descriptor.
> >                            (file-notify-rm-watch)
> > -                          (file-notify-valid-p)))
> > +                          (file-notify-valid-p)
> > +                          ;; `make-process' uses keyword arguments and
> > +                          ;; doesn't mangle its filenames in any way.
> > +                          ;; It already strips /: from the binary
> > +                          ;; filename, so we don't have to do this
> > +                          ;; here.
> > +                          (make-process)))
> >                   ;; For all other operations, treat the first
> >                   ;; argument only as the file name.
> >                   '(nil 0))))
> > diff --git a/src/process.c b/src/process.c
> > index 8e0b2349f9..5895f77446 100644
> > --- a/src/process.c
> > +++ b/src/process.c
> > @@ -1661,6 +1661,11 @@ to the standard error of subprocess.  Specifying 
> > this implies
> >  `:connection-type' is set to `pipe'.  If STDERR is nil, standard error
> >  is mixed with standard output and sent to BUFFER or FILTER.
> >
> > +:file-handler FILE-HANDLER -- If FILE-HANDLER is non-nil, then look
> > +for a file name handler for the current buffer's `default-directory'
> > +and invoke that file handler to make the process.  If there is no
> > +such handler, proceed as if FILE-HANDLER were nil.
> > +
>
> Ditto.
>
> >  usage: (make-process &rest ARGS)  */)
> >    (ptrdiff_t nargs, Lisp_Object *args)
> >  {
> > @@ -1674,6 +1679,15 @@ usage: (make-process &rest ARGS)  */)
> >    /* Save arguments for process-contact and clone-process.  */
> >    contact = Flist (nargs, args);
> >
> > +  if (!NILP (Fplist_get (contact, QCfile_handler)))
> > +    {
> > +      Lisp_Object file_handler
> > +        = Ffind_file_name_handler (BVAR (current_buffer, directory),
> > +                                   Qmake_process);
> > +      if (!NILP (file_handler))
> > +        return CALLN (Fapply, file_handler, Qmake_process, contact);
> > +    }
> > +
> >    buffer = Fplist_get (contact, QCbuffer);
> >    if (!NILP (buffer))
> >      buffer = Fget_buffer_create (buffer);
> > @@ -8098,6 +8112,8 @@ init_process_emacs (int sockfd)
> >  void
> >  syms_of_process (void)
> >  {
> > +  DEFSYM (Qmake_process, "make-process");
> > +
> >  #ifdef subprocesses
> >
> >    DEFSYM (Qprocessp, "processp");
> > @@ -8138,6 +8154,7 @@ syms_of_process (void)
> >    DEFSYM (Qreal, "real");
> >    DEFSYM (Qnetwork, "network");
> >    DEFSYM (Qserial, "serial");
> > +  DEFSYM (QCfile_handler, ":file-handler");
> >    DEFSYM (QCbuffer, ":buffer");
> >    DEFSYM (QChost, ":host");
> >    DEFSYM (QCservice, ":service");
> > diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
> > index 3b192ee872..9d827e865d 100644
> > --- a/test/lisp/files-tests.el
> > +++ b/test/lisp/files-tests.el
> > @@ -1109,6 +1109,16 @@ files-tests-file-attributes-equal
> >      (with-temp-buffer
> >        (write-region nil nil nospecial nil :visit))))
> >
> > +(ert-deftest files-tests-file-name-non-special-make-process ()
> > +  "Check that the ‘:file-handler’ argument of ‘make-process’
> > +works as expected if the default directory is quoted."
> > +  (let ((default-directory (file-name-quote invocation-directory))
> > +        (program (file-name-quote
> > +                  (expand-file-name invocation-name 
> > invocation-directory))))
> > +    (should (processp (make-process :name "name"
> > +                                    :command (list program "--version")
> > +                                    :file-handler t)))))
> > +
> >  (ert-deftest files-tests--insert-directory-wildcard-in-dir-p ()
> >    (let ((alist (list (cons "/home/user/*/.txt" (cons "/home/user/" 
> > "*/.txt"))
> >                       (cons "/home/user/.txt" nil)
> > diff --git a/test/src/process-tests.el b/test/src/process-tests.el
> > index 551b34ff37..2e4be53185 100644
> > --- a/test/src/process-tests.el
> > +++ b/test/src/process-tests.el
> > @@ -215,5 +215,54 @@ process-tests--mixable
> >                                        (string-to-list "stdout\n")
> >                                        (string-to-list "stderr\n"))))))
> >
> > +(ert-deftest make-process/file-handler/found ()
> > +  "Check that the ‘:file-handler’ argument of ‘make-process’
> > +works as expected if a file handler is found."
> > +  (let ((file-handler-calls 0))
> > +    (cl-flet ((file-handler
> > +               (&rest args)
> > +               (should (equal default-directory "test-handler:/dir/"))
> > +               (should (equal args '(make-process :name "name"
> > +                                                  :command ("/bin/true")
> > +                                                  :file-handler t)))
> > +               (cl-incf file-handler-calls)
> > +               'fake-process))
> > +      (let ((file-name-handler-alist (list (cons (rx bos "test-handler:")
> > +                                                 #'file-handler)))
> > +            (default-directory "test-handler:/dir/"))
> > +        (should (eq (make-process :name "name"
> > +                                  :command '("/bin/true")
> > +                                  :file-handler t)
> > +                    'fake-process))
> > +        (should (= file-handler-calls 1))))))
> > +
> > +(ert-deftest make-process/file-handler/not-found ()
> > +  "Check that the ‘:file-handler’ argument of ‘make-process’
> > +works as expected if no file handler is found."
> > +  (let ((file-name-handler-alist ())
> > +        (default-directory invocation-directory)
> > +        (program (expand-file-name invocation-name invocation-directory)))
> > +    (should (processp (make-process :name "name"
> > +                                    :command (list program "--version")
> > +                                    :file-handler t)))))
> > +
> > +(ert-deftest make-process/file-handler/disable ()
> > +  "Check ‘make-process’ works as expected if it shouldn’t use the
> > +file handler."
> > +  (let ((file-name-handler-alist (list (cons (rx bos "test-handler:")
> > +                                             
> > #'process-tests--file-handler)))
> > +        (default-directory "test-handler:/dir/")
> > +        (program (expand-file-name invocation-name invocation-directory)))
> > +    (should (processp (make-process :name "name"
> > +                                    :command (list program 
> > "--version"))))))
> > +
> > +(defun process-tests--file-handler (operation &rest _args)
> > +  (cl-ecase operation
> > +    (unhandled-file-name-directory "/")
> > +    (make-process (ert-fail "file handler called unexpectedly"))))
> > +
> > +(put #'process-tests--file-handler 'operations
> > +     '(unhandled-file-name-directory make-process))
> > +
> >  (provide 'process-tests)
> >  ;; process-tests.el ends here.
> > --
> > 2.20.0.405.gbc1bbc6f85-goog
>
> --
> Alan Mackenzie (Nuremberg, Germany).
>





reply via email to

[Prev in Thread] Current Thread [Next in Thread]