* bug#28691: 27.0.50; make-process has no file-name-handler @ 2017-10-03 17:13 Stefan Monnier 2017-10-03 17:50 ` Michael Albinus ` (3 more replies) 0 siblings, 4 replies; 27+ messages in thread From: Stefan Monnier @ 2017-10-03 17:13 UTC (permalink / raw) To: 28691 Package: Emacs Version: 27.0.50 `make-process` should aim to replace not just `start-process` but also `start-file-process`, so it needs to look up a file-name-handler. We could do that without breaking compatibility (contrary to what happened with `start-process`) by adding a :file keyword argument. Stefan In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars) of 2017-09-26 built on pastel Repository revision: 7796623e559825655cf966bd83a10179214a6fbe Windowing system distributor 'The X.Org Foundation', version 11.0.11902000 System Description: Debian GNU/Linux 9.1 (stretch) Recent messages: Type C-c C-c to toggle between editing or viewing the document. When done with a buffer, type C-x # New window #<window 119 on PDFDocument25.pdf> for buf PDFDocument25.pdf Note: file is write protected New window t for buf PDFDocument26.pdf Type C-c C-c to toggle between editing or viewing the document. When done with a buffer, type C-x # New window #<window 121 on PDFDocument26.pdf> for buf PDFDocument26.pdf Making completion list... Configured using: 'configure -C --enable-checking --with-modules --enable-check-lisp-object-type 'CFLAGS=-Wall -g3 -Og -Wno-pointer-sign' PKG_CONFIG_PATH=/home/monnier/lib/pkgconfig' Configured features: XAW3D XPM JPEG TIFF GIF PNG SOUND GPM DBUS NOTIFY GNUTLS LIBXML2 FREETYPE XFT ZLIB TOOLKIT_SCROLL_BARS LUCID X11 MODULES Important settings: value of $LANG: fr_CH.UTF-8 locale-coding-system: utf-8-unix Major mode: InactiveMinibuffer Minor modes in effect: shell-dirtrack-mode: t diff-auto-refine-mode: t electric-pair-mode: t global-reveal-mode: t reveal-mode: t auto-insert-mode: t savehist-mode: t minibuffer-electric-default-mode: t global-compact-docstrings-mode: t url-handler-mode: t global-eldoc-mode: t electric-indent-mode: t mouse-wheel-mode: t global-prettify-symbols-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t line-number-mode: t transient-mark-mode: t Load-path shadows: /home/monnier/src/emacs/elpa/packages/svg/svg hides /home/monnier/src/emacs/work/lisp/svg /home/monnier/src/emacs/elpa/packages/ada-mode/ada-prj hides /home/monnier/src/emacs/work/lisp/progmodes/ada-prj /home/monnier/src/emacs/elpa/packages/ada-mode/ada-stmt hides /home/monnier/src/emacs/work/lisp/progmodes/ada-stmt /home/monnier/src/emacs/elpa/packages/ada-mode/ada-mode hides /home/monnier/src/emacs/work/lisp/progmodes/ada-mode /home/monnier/src/emacs/elpa/packages/ada-mode/ada-xref hides /home/monnier/src/emacs/work/lisp/progmodes/ada-xref /home/monnier/src/emacs/elpa/packages/hyperbole/set hides /home/monnier/src/emacs/work/lisp/emacs-lisp/set /home/monnier/src/emacs/elpa/packages/crisp/crisp hides /home/monnier/src/emacs/work/lisp/obsolete/crisp /home/monnier/src/emacs/elpa/packages/landmark/landmark hides /home/monnier/src/emacs/work/lisp/obsolete/landmark Features: (mail-extr emacsbug org-table ox-odt ox-latex ox-icalendar ox-html ox-ascii ox-publish ox org-protocol org-mouse org-mobile org-agenda org-indent org-feed org-crypt org-capture org-attach org-id org-rmail org-mhe org-irc org-info org-gnus org-docview org-bibtex bibtex org-bbdb org-w3m org-element avl-tree generator org org-macro org-footnote org-pcomplete org-list org-faces org-entities org-version ob-emacs-lisp ob ob-tangle org-src ob-ref ob-lob ob-table ob-keys ob-exp ob-comint ob-core ob-eval org-compat org-macs org-loaddefs cal-menu calendar cal-loaddefs autorevert filenotify doc-view jka-compr image-mode eieio-opt log-edit message sendmail subr-x puny dired dired-loaddefs format-spec rfc822 mml mml-sec epa epg gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr mailabbrev mail-utils mailheader pcvs-util add-log shell pcomplete tuareg speedbar sb-image ezimage dframe caml imenu caml-help caml-types caml-emacs derived pcase quail typer-mode smie vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs vc-dir html5-schema rng-xsd xsd-regexp rng-cmpct rng-nxml nxml-mode nxml-outln nxml-rap sgml-mode dom misearch multi-isearch cl-extra help-fns radix-tree cl-print view tildify table rst compile rng-valid rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns nxml-util nxml-enc xmltok refer refer-to-bibtex refbib printing picture nroff-mode enriched ebnf2ps ps-print ps-print-loaddefs ps-def lpr delim-col rect bib-mode sort mpc executable copyright xscheme warnings unsafep trace testcover shadow scheme re-builder profiler inf-lisp ielm comint ansi-color ring gmm-utils ert pp find-func ewoc debug elp edebug cl-indent cus-edit cus-start cus-load wid-edit vc vc-dispatcher smerge-mode vc-git diff-mode filecache map server time-date noutline outline easy-mmode flyspell ispell checkdoc thingatpt help-mode load-dir elec-pair reveal autoinsert proof-site proof-autoloads cl pg-vars savehist minibuf-eldef disp-table compact-docstrings cl-seq inline kotl-loaddefs advice info realgud-recursive-autoloads finder-inf url-auth package easymenu epg-config url-handlers url-parse auth-source eieio eieio-core cl-macs eieio-loaddefs password-cache url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib bbdb-autoloads vm-autoloads mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese composite charscript charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs button faces cus-face macroexp files text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote dbusbind inotify dynamic-setting font-render-setting x-toolkit x multi-tty make-network-process emacs) Memory information: ((conses 16 522650 87942) (symbols 48 42366 0) (miscs 40 5507 549) (strings 32 138632 7037) (string-bytes 1 3917145) (vectors 16 72593) (vector-slots 8 2109504 123075) (floats 8 650 473) (intervals 56 28434 115) (buffers 992 70)) ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: 27.0.50; make-process has no file-name-handler 2017-10-03 17:13 bug#28691: 27.0.50; make-process has no file-name-handler Stefan Monnier @ 2017-10-03 17:50 ` Michael Albinus 2017-10-03 18:34 ` Eli Zaretskii 2018-12-16 23:39 ` bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) Philipp Stephani ` (2 subsequent siblings) 3 siblings, 1 reply; 27+ messages in thread From: Michael Albinus @ 2017-10-03 17:50 UTC (permalink / raw) To: Stefan Monnier; +Cc: 28691 Stefan Monnier <monnier@iro.umontreal.ca> writes: Hi Stefan, > `make-process` should aim to replace not just `start-process` but also > `start-file-process`, so it needs to look up a file-name-handler. > We could do that without breaking compatibility (contrary to what > happened with `start-process`) by adding a :file keyword argument. I'm open to that. If you add the call to the file name handler in process.c, I would start to write support in Tramp for this, beginning with tramp-sh.el. First, we need the OK from the maintainers, of course. > Stefan Best regards, Michael. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: 27.0.50; make-process has no file-name-handler 2017-10-03 17:50 ` Michael Albinus @ 2017-10-03 18:34 ` Eli Zaretskii 0 siblings, 0 replies; 27+ messages in thread From: Eli Zaretskii @ 2017-10-03 18:34 UTC (permalink / raw) To: Michael Albinus; +Cc: 28691, monnier > From: Michael Albinus <michael.albinus@gmx.de> > Date: Tue, 03 Oct 2017 19:50:31 +0200 > Cc: 28691@debbugs.gnu.org > > I'm open to that. If you add the call to the file name handler in > process.c, I would start to write support in Tramp for this, beginning > with tramp-sh.el. > > First, we need the OK from the maintainers, of course. No objections here. Thanks. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2017-10-03 17:13 bug#28691: 27.0.50; make-process has no file-name-handler Stefan Monnier 2017-10-03 17:50 ` Michael Albinus @ 2018-12-16 23:39 ` Philipp Stephani 2018-12-17 12:38 ` Michael Albinus [not found] ` <20181216233936.208568-1-phst@google.com> [not found] ` <mailman.5866.1545079749.1284.bug-gnu-emacs@gnu.org> 3 siblings, 1 reply; 27+ messages in thread From: Philipp Stephani @ 2018-12-16 23:39 UTC (permalink / raw) To: emacs-devel, 28691; +Cc: Philipp Stephani * src/process.c (Fmake_process): Add new keyword argument ':file-handler'. (syms_of_process) <make-process, :file-handler>: Define new symbols. * test/src/process-tests.el (make-process/file-handler): New unit test. --- etc/NEWS | 4 ++++ src/process.c | 15 +++++++++++++++ test/src/process-tests.el | 22 ++++++++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/etc/NEWS b/etc/NEWS index c88f6ef5ca..0a5f915b33 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1407,6 +1407,10 @@ 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 search for a file name handler for +'default-directory'. + \f * Changes in Emacs 27.1 on Non-Free Operating Systems diff --git a/src/process.c b/src/process.c index 8e0b2349f9..852431f421 100644 --- a/src/process.c +++ b/src/process.c @@ -1661,6 +1661,9 @@ 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 search +for a file name handler for `default-directory'. + usage: (make-process &rest ARGS) */) (ptrdiff_t nargs, Lisp_Object *args) { @@ -1674,6 +1677,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 +8110,8 @@ init_process_emacs (int sockfd) void syms_of_process (void) { + DEFSYM (Qmake_process, "make-process"); + #ifdef subprocesses DEFSYM (Qprocessp, "processp"); @@ -8138,6 +8152,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/src/process-tests.el b/test/src/process-tests.el index 551b34ff37..3e72e9210d 100644 --- a/test/src/process-tests.el +++ b/test/src/process-tests.el @@ -215,5 +215,27 @@ process-tests--mixable (string-to-list "stdout\n") (string-to-list "stderr\n")))))) +(ert-deftest make-process/file-handler () + "Check that the ‘:file-handler’ argument of ‘make-process’ +works as expected." + (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 + (cons (cons (rx bos "test-handler:") #'file-handler) + file-name-handler-alist)) + (default-directory "test-handler:/dir/")) + (should (eq (make-process :name "name" + :command '("/bin/true") + :file-handler t) + 'fake-process)) + (should (= file-handler-calls 1)))))) + (provide 'process-tests) ;; process-tests.el ends here. -- 2.20.0.405.gbc1bbc6f85-goog ^ permalink raw reply related [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-16 23:39 ` bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) Philipp Stephani @ 2018-12-17 12:38 ` Michael Albinus 2018-12-17 17:35 ` Eli Zaretskii ` (3 more replies) 0 siblings, 4 replies; 27+ messages in thread From: Michael Albinus @ 2018-12-17 12:38 UTC (permalink / raw) To: Philipp Stephani; +Cc: Philipp Stephani, 28691, emacs-devel Philipp Stephani <p.stephani2@gmail.com> writes: Hi Philipp, > +:file-handler FILE-HANDLER -- If FILE-HANDLER is non-nil, then search > +for a file name handler for `default-directory'. What happens, if no file name handler is found? Should there be a local process then, or should this be ignored (returning nil)? > +(ert-deftest make-process/file-handler () > + "Check that the ‘:file-handler’ argument of ‘make-process’ > +works as expected." > + (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 > + (cons (cons (rx bos "test-handler:") #'file-handler) > + file-name-handler-alist)) > + (default-directory "test-handler:/dir/")) > + (should (eq (make-process :name "name" > + :command '("/bin/true") > + :file-handler t) > + 'fake-process)) > + (should (= file-handler-calls 1)))))) I would make a second test, that calling `make-process' w/o the `:file-handler' argument returns the plain process #<process name>. The third test is for using non-nil `:file-handler', w/o finding one. This returns either a local process, or nil (see remark above). I also miss documentation in the Elisp manual, nodes "Magic File Names" and "Asynchronous Processes". And of course, the implementation of a file name handler is missing in tramp-adb.el, tramp-sh.el and tramp-smb.el. Best regards, Michael. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-17 12:38 ` Michael Albinus @ 2018-12-17 17:35 ` Eli Zaretskii 2018-12-17 19:07 ` Philipp Stephani ` (2 subsequent siblings) 3 siblings, 0 replies; 27+ messages in thread From: Eli Zaretskii @ 2018-12-17 17:35 UTC (permalink / raw) To: Michael Albinus; +Cc: phst, 28691, p.stephani2, emacs-devel > From: Michael Albinus <michael.albinus@gmx.de> > Date: Mon, 17 Dec 2018 13:38:33 +0100 > Cc: Philipp Stephani <phst@google.com>, 28691@debbugs.gnu.org, > emacs-devel@gnu.org > > Philipp Stephani <p.stephani2@gmail.com> writes: > > Hi Philipp, > > > +:file-handler FILE-HANDLER -- If FILE-HANDLER is non-nil, then search > > +for a file name handler for `default-directory'. > > What happens, if no file name handler is found? Should there be a local > process then, or should this be ignored (returning nil)? The proposed code runs the process locally, which I think is reasonable. > I also miss documentation in the Elisp manual, nodes "Magic File Names" > and "Asynchronous Processes". Right. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-17 12:38 ` Michael Albinus 2018-12-17 17:35 ` Eli Zaretskii @ 2018-12-17 19:07 ` Philipp Stephani [not found] ` <CAArVCkSzt=pcVG9ZRpdT6oSi30ExcqBGAQ5J8b3x0+gkOo+d2w@mail.gmail.com> [not found] ` <838t0odpk8.fsf@gnu.org> 3 siblings, 0 replies; 27+ messages in thread From: Philipp Stephani @ 2018-12-17 19:07 UTC (permalink / raw) To: Michael Albinus; +Cc: Philipp Stephani, 28691, Emacs developers Am Mo., 17. Dez. 2018 um 13:38 Uhr schrieb Michael Albinus <michael.albinus@gmx.de>: > > Philipp Stephani <p.stephani2@gmail.com> writes: > > Hi Philipp, > > > +:file-handler FILE-HANDLER -- If FILE-HANDLER is non-nil, then search > > +for a file name handler for `default-directory'. > > What happens, if no file name handler is found? Should there be a local > process then, or should this be ignored (returning nil)? I think it should be a new process, like start-file-process does. (make-process should always either return a process object or fail.) > > > +(ert-deftest make-process/file-handler () > > + "Check that the ‘:file-handler’ argument of ‘make-process’ > > +works as expected." > > + (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 > > + (cons (cons (rx bos "test-handler:") #'file-handler) > > + file-name-handler-alist)) > > + (default-directory "test-handler:/dir/")) > > + (should (eq (make-process :name "name" > > + :command '("/bin/true") > > + :file-handler t) > > + 'fake-process)) > > + (should (= file-handler-calls 1)))))) > > I would make a second test, that calling `make-process' w/o the > `:file-handler' argument returns the plain process #<process name>. > > The third test is for using non-nil `:file-handler', w/o finding one. > This returns either a local process, or nil (see remark above). > > I also miss documentation in the Elisp manual, nodes "Magic File Names" > and "Asynchronous Processes". Good points, I'll incorporate them in a follow-up patch. > > And of course, the implementation of a file name handler is missing in > tramp-adb.el, tramp-sh.el and tramp-smb.el. That should be part of a different bug. (Since make-process is more capable than start-file-process, implementing Tramp support will be a bit more difficult.) ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <CAArVCkSzt=pcVG9ZRpdT6oSi30ExcqBGAQ5J8b3x0+gkOo+d2w@mail.gmail.com>]
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) [not found] ` <CAArVCkSzt=pcVG9ZRpdT6oSi30ExcqBGAQ5J8b3x0+gkOo+d2w@mail.gmail.com> @ 2018-12-17 19:20 ` Philipp Stephani 2018-12-17 19:58 ` Eli Zaretskii 2018-12-17 19:33 ` Michael Albinus 2018-12-17 20:03 ` Drew Adams 2 siblings, 1 reply; 27+ messages in thread From: Philipp Stephani @ 2018-12-17 19:20 UTC (permalink / raw) To: emacs-devel, Michael Albinus, 28691; +Cc: Philipp Stephani * 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 | 9 ++++++-- etc/NEWS | 5 ++++ lisp/files.el | 11 +++++++-- src/process.c | 16 +++++++++++++ test/lisp/files-tests.el | 9 ++++++++ test/src/process-tests.el | 47 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 95 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..7ca0ac475f 100644 --- a/doc/lispref/processes.texi +++ b/doc/lispref/processes.texi @@ -696,6 +696,11 @@ 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}. If there +is no such handler, proceed as if @var{file-handler} were nil. @end table The original argument list, modified with the actual connection @@ -704,8 +709,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..942b6501a8 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'. That way 'make-process' can start +remote processes. + \f * 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..80c700a09b 100644 --- a/src/process.c +++ b/src/process.c @@ -1661,6 +1661,10 @@ 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'. +If there is no such handler, proceed as if FILE-HANDLER were nil. + usage: (make-process &rest ARGS) */) (ptrdiff_t nargs, Lisp_Object *args) { @@ -1674,6 +1678,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 +8111,8 @@ init_process_emacs (int sockfd) void syms_of_process (void) { + DEFSYM (Qmake_process, "make-process"); + #ifdef subprocesses DEFSYM (Qprocessp, "processp"); @@ -8138,6 +8153,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..708d6d4659 100644 --- a/test/lisp/files-tests.el +++ b/test/lisp/files-tests.el @@ -1109,6 +1109,15 @@ 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." + (skip-unless (file-accessible-directory-p "/:/bin/")) + (let ((default-directory "/:/bin/")) + (should (processp (make-process :name "name" + :command '("/:/bin/true") + :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..3587e3bfd8 100644 --- a/test/src/process-tests.el +++ b/test/src/process-tests.el @@ -215,5 +215,52 @@ 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." + (skip-unless (file-accessible-directory-p "/bin/")) + (let ((default-directory "/bin/")) + (should (processp (make-process :name "name" + :command '("/bin/true") + :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/")) + (should (processp (make-process :name "name" + :command '("/bin/true")))))) + +(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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-17 19:20 ` Philipp Stephani @ 2018-12-17 19:58 ` Eli Zaretskii 2018-12-17 20:47 ` Philipp Stephani 0 siblings, 1 reply; 27+ messages in thread From: Eli Zaretskii @ 2018-12-17 19:58 UTC (permalink / raw) To: Philipp Stephani; +Cc: phst, 28691, michael.albinus > From: Philipp Stephani <p.stephani2@gmail.com> > Date: Mon, 17 Dec 2018 20:20:11 +0100 > Cc: Philipp Stephani <phst@google.com> > > +@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 nil. ^^^ That "nil" should be in @code. > +** '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'. Likewise. > +:file-handler FILE-HANDLER -- If FILE-HANDLER is non-nil, then look > +for a file name handler for the current buffer's `default-directory'. Likewise. > + (skip-unless (file-accessible-directory-p "/:/bin/")) I don't see why we should skip important tests on platforms where there's no /bin. Isn't it possible to use the Emacs binary here (and elsewhere in the tests you add), as I suggested? Thanks. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-17 19:58 ` Eli Zaretskii @ 2018-12-17 20:47 ` Philipp Stephani 2018-12-22 9:07 ` Eli Zaretskii 0 siblings, 1 reply; 27+ messages in thread From: Philipp Stephani @ 2018-12-17 20:47 UTC (permalink / raw) To: eliz; +Cc: Philipp Stephani, 28691 * 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 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. + \f * 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. + 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-17 20:47 ` Philipp Stephani @ 2018-12-22 9:07 ` Eli Zaretskii 2018-12-22 9:31 ` Michael Albinus 2018-12-22 22:26 ` Philipp Stephani 0 siblings, 2 replies; 27+ messages in thread From: Eli Zaretskii @ 2018-12-22 9:07 UTC (permalink / raw) To: Philipp Stephani; +Cc: phst, 28691 > From: Philipp Stephani <p.stephani2@gmail.com> > Cc: 28691@debbugs.gnu.org, > Philipp Stephani <phst@google.com> > Date: Mon, 17 Dec 2018 21:47:46 +0100 > > * 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'. Thanks, this LGTM, but some of the tests use the Emacs binary, while others still use /bin/true for some reason. What am I missing? Please also include etc/NEWS change in the log message. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-22 9:07 ` Eli Zaretskii @ 2018-12-22 9:31 ` Michael Albinus 2018-12-22 21:08 ` Philipp Stephani 2018-12-22 22:26 ` Philipp Stephani 1 sibling, 1 reply; 27+ messages in thread From: Michael Albinus @ 2018-12-22 9:31 UTC (permalink / raw) To: Eli Zaretskii; +Cc: phst, 28691, Philipp Stephani Eli Zaretskii <eliz@gnu.org> writes: > this LGTM, Same here, but I still appreciate a clarification in the doc, that not all file name handlers will support this function, and they will return nil in that case. This is NOT the case that no file name handler is found, and a local process is created. Best regards, Michael. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-22 9:31 ` Michael Albinus @ 2018-12-22 21:08 ` Philipp Stephani 2018-12-23 7:41 ` Michael Albinus 0 siblings, 1 reply; 27+ messages in thread From: Philipp Stephani @ 2018-12-22 21:08 UTC (permalink / raw) To: Michael Albinus; +Cc: Philipp Stephani, 28691 Am Sa., 22. Dez. 2018 um 10:31 Uhr schrieb Michael Albinus <michael.albinus@gmx.de>: > > Eli Zaretskii <eliz@gnu.org> writes: > > > this LGTM, > > Same here, but I still appreciate a clarification in the doc, that not > all file name handlers will support this function, That isn't the case: https://www.gnu.org/software/emacs/manual/html_node/elisp/Magic-File-Names.html states clearly "The handler function must handle all of the above operations, and possibly others to be added in the future." > and they will return > nil in that case. That is also not the case. If there's no handler, setting :file-handler to t has no effect. That's the same as for start-file-process, which calls start-process if no handler is found. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-22 21:08 ` Philipp Stephani @ 2018-12-23 7:41 ` Michael Albinus 2018-12-23 10:08 ` Philipp Stephani 0 siblings, 1 reply; 27+ messages in thread From: Michael Albinus @ 2018-12-23 7:41 UTC (permalink / raw) To: Philipp Stephani; +Cc: Philipp Stephani, 28691 Philipp Stephani <p.stephani2@gmail.com> writes: Hi Philipp, >> > this LGTM, >> >> Same here, but I still appreciate a clarification in the doc, that not >> all file name handlers will support this function, > > That isn't the case: > https://www.gnu.org/software/emacs/manual/html_node/elisp/Magic-File-Names.html > states clearly "The handler function must handle all of the above > operations, and possibly others to be added in the future." Tramp does *handle* all operations. But it does not *support* all of them. In case it is not possible to offer an implementation for `start-file-process' (as example), Tramp uses `ignore' as implementation. >> and they will return >> nil in that case. > > That is also not the case. If there's no handler, setting > :file-handler to t has no effect. That's the same as for > start-file-process, which calls start-process if no handler is found. Of course. I do not speak about this case. Tramp offers a handler, which will also supports `make-network-process'. This handler is called `tramp-file-name-handler', and you see it in `file-name-handler-alist'. But this handler could decide, that for a given connection method there's nothing to do. Remember the example I have shown already: --8<---------------cut here---------------start------------->8--- (with-temp-buffer (let ((default-directory "/ssh::")) (start-file-process "foo" (current-buffer) "/bin/true"))) => #<process foo> (with-temp-buffer (let ((default-directory "/sftp::")) (start-file-process "foo" (current-buffer) "/bin/true"))) => nil --8<---------------cut here---------------end--------------->8--- "sftp" belongs to the connection methods implemented via GVFS, no external processes are possible. See `tramp-gvfs-file-name-handler-alist', many operations use `ignore' as their implementation. `make-network-process' will get a similar entry. Best regards, Michael. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-23 7:41 ` Michael Albinus @ 2018-12-23 10:08 ` Philipp Stephani 2018-12-23 10:26 ` Michael Albinus 0 siblings, 1 reply; 27+ messages in thread From: Philipp Stephani @ 2018-12-23 10:08 UTC (permalink / raw) To: Michael Albinus; +Cc: Philipp Stephani, 28691 Am So., 23. Dez. 2018 um 08:41 Uhr schrieb Michael Albinus <michael.albinus@gmx.de>: > > Philipp Stephani <p.stephani2@gmail.com> writes: > > Hi Philipp, > > >> > this LGTM, > >> > >> Same here, but I still appreciate a clarification in the doc, that not > >> all file name handlers will support this function, > > > > That isn't the case: > > https://www.gnu.org/software/emacs/manual/html_node/elisp/Magic-File-Names.html > > states clearly "The handler function must handle all of the above > > operations, and possibly others to be added in the future." > > Tramp does *handle* all operations. But it does not *support* all of > them. In case it is not possible to offer an implementation for > `start-file-process' (as example), Tramp uses `ignore' as implementation. Yes, I've seen this, but it's a characteristic of Tramp, not of Emacs itself, so I don't think it should be documented in the Elisp manual itself. I also think that Tramp should actually not return nil, but signal an error instead. make-process either returns a process object or signals an error (except for a weird corner case with zero arguments), and I think file name handlers handling make-process should behave the same. > > >> and they will return > >> nil in that case. > > > > That is also not the case. If there's no handler, setting > > :file-handler to t has no effect. That's the same as for > > start-file-process, which calls start-process if no handler is found. > > Of course. I do not speak about this case. Tramp offers a handler, which > will also supports `make-network-process'. This handler is called > `tramp-file-name-handler', and you see it in > `file-name-handler-alist'. But this handler could decide, that for a > given connection method there's nothing to do. > > Remember the example I have shown already: > > --8<---------------cut here---------------start------------->8--- > (with-temp-buffer > (let ((default-directory "/ssh::")) > (start-file-process "foo" (current-buffer) "/bin/true"))) > => #<process foo> > > (with-temp-buffer > (let ((default-directory "/sftp::")) > (start-file-process "foo" (current-buffer) "/bin/true"))) > => nil > --8<---------------cut here---------------end--------------->8--- > > "sftp" belongs to the connection methods implemented via GVFS, no > external processes are possible. See `tramp-gvfs-file-name-handler-alist', > many operations use `ignore' as their implementation. `make-network-process' > will get a similar entry. > > Best regards, Michael. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-23 10:08 ` Philipp Stephani @ 2018-12-23 10:26 ` Michael Albinus 2018-12-23 15:45 ` Eli Zaretskii 0 siblings, 1 reply; 27+ messages in thread From: Michael Albinus @ 2018-12-23 10:26 UTC (permalink / raw) To: Philipp Stephani; +Cc: Philipp Stephani, 28691 Philipp Stephani <p.stephani2@gmail.com> writes: Hi Philipp, >> Tramp does *handle* all operations. But it does not *support* all of >> them. In case it is not possible to offer an implementation for >> `start-file-process' (as example), Tramp uses `ignore' as implementation. > > Yes, I've seen this, but it's a characteristic of Tramp, not of Emacs > itself, so I don't think it should be documented in the Elisp manual > itself. Tramp (and partly ange-ftp) are the only file name handlers which offer process-related implementations. So it isn't wrong to document this, I believe. Since we disgree here, let Eli decide. > I also think that Tramp should actually not return nil, but signal an > error instead. make-process either returns a process object or signals > an error (except for a weird corner case with zero arguments), and I > think file name handlers handling make-process should behave the same. No, `make-process' does *not* raise an error, it returns a process object. The process itself aborts, which is a different thing. Tramp will behave similar. --8<---------------cut here---------------start------------->8--- (make-process :name "foo" :buffer (current-buffer) :command '("/bin/foo")) #<process foo> emacs: /bin/foo: No such file or directory Process foo exited abnormally with code 127 --8<---------------cut here---------------end--------------->8--- Best regards, Michael. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-23 10:26 ` Michael Albinus @ 2018-12-23 15:45 ` Eli Zaretskii 2018-12-23 16:36 ` Michael Albinus 0 siblings, 1 reply; 27+ messages in thread From: Eli Zaretskii @ 2018-12-23 15:45 UTC (permalink / raw) To: Michael Albinus; +Cc: phst, 28691, p.stephani2 > From: Michael Albinus <michael.albinus@gmx.de> > Cc: Eli Zaretskii <eliz@gnu.org>, Philipp Stephani <phst@google.com>, 28691@debbugs.gnu.org > Date: Sun, 23 Dec 2018 11:26:54 +0100 > > >> Tramp does *handle* all operations. But it does not *support* all of > >> them. In case it is not possible to offer an implementation for > >> `start-file-process' (as example), Tramp uses `ignore' as implementation. > > > > Yes, I've seen this, but it's a characteristic of Tramp, not of Emacs > > itself, so I don't think it should be documented in the Elisp manual > > itself. > > Tramp (and partly ange-ftp) are the only file name handlers which offer > process-related implementations. So it isn't wrong to document this, I > believe. > > Since we disgree here, let Eli decide. Can we say that a file handler _might_ decide not to support a certain operation, and describe what will happen in that case, without mentioning Tramp? ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-23 15:45 ` Eli Zaretskii @ 2018-12-23 16:36 ` Michael Albinus 2018-12-23 16:50 ` Eli Zaretskii 0 siblings, 1 reply; 27+ messages in thread From: Michael Albinus @ 2018-12-23 16:36 UTC (permalink / raw) To: Eli Zaretskii; +Cc: phst, 28691, p.stephani2 Eli Zaretskii <eliz@gnu.org> writes: Hi Eli, >> > Yes, I've seen this, but it's a characteristic of Tramp, not of Emacs >> > itself, so I don't think it should be documented in the Elisp manual >> > itself. >> >> Tramp (and partly ange-ftp) are the only file name handlers which offer >> process-related implementations. So it isn't wrong to document this, I >> believe. >> >> Since we disgree here, let Eli decide. > > Can we say that a file handler _might_ decide not to support a certain > operation, and describe what will happen in that case, without > mentioning Tramp? Earlier today, I've added to the Lisp manual the phrases --8<---------------cut here---------------start------------->8--- Depending on the implementation of the file name handler, it might not be possible to apply FILTER or SENTINEL to the resulting process object. *Note Filter Functions::, and *note Sentinels::. Some file name handlers may not support ‘make-process’. In such cases, this function does nothing and returns ‘nil’. --8<---------------cut here---------------end--------------->8--- I believe, this is sufficient neutral. Best regards, Michael. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-23 16:36 ` Michael Albinus @ 2018-12-23 16:50 ` Eli Zaretskii 0 siblings, 0 replies; 27+ messages in thread From: Eli Zaretskii @ 2018-12-23 16:50 UTC (permalink / raw) To: Michael Albinus; +Cc: phst, 28691, p.stephani2 > From: Michael Albinus <michael.albinus@gmx.de> > Cc: p.stephani2@gmail.com, phst@google.com, 28691@debbugs.gnu.org > Date: Sun, 23 Dec 2018 17:36:22 +0100 > > > Can we say that a file handler _might_ decide not to support a certain > > operation, and describe what will happen in that case, without > > mentioning Tramp? > > Earlier today, I've added to the Lisp manual the phrases > > --8<---------------cut here---------------start------------->8--- > Depending on the implementation of the file name handler, it might > not be possible to apply FILTER or SENTINEL to the resulting > process object. *Note Filter Functions::, and *note Sentinels::. > > Some file name handlers may not support ‘make-process’. In such > cases, this function does nothing and returns ‘nil’. > --8<---------------cut here---------------end--------------->8--- > > I believe, this is sufficient neutral. I agree. Philipp, does this wording work for you? ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-22 9:07 ` Eli Zaretskii 2018-12-22 9:31 ` Michael Albinus @ 2018-12-22 22:26 ` Philipp Stephani 1 sibling, 0 replies; 27+ messages in thread From: Philipp Stephani @ 2018-12-22 22:26 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Philipp Stephani, 28691-done Am Sa., 22. Dez. 2018 um 10:09 Uhr schrieb Eli Zaretskii <eliz@gnu.org>: > > > From: Philipp Stephani <p.stephani2@gmail.com> > > Cc: 28691@debbugs.gnu.org, > > Philipp Stephani <phst@google.com> > > Date: Mon, 17 Dec 2018 21:47:46 +0100 > > > > * 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'. > > Thanks, this LGTM, but some of the tests use the Emacs binary, while > others still use /bin/true for some reason. What am I missing? The one that uses /bin/true never invokes it, but only tests whether the full argument list is passed to the handler. I've changed this now to /some/binary to clarify that the actual string doesn't matter. > > Please also include etc/NEWS change in the log message. > Done and pushed as 039be4e025. Thanks! ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) [not found] ` <CAArVCkSzt=pcVG9ZRpdT6oSi30ExcqBGAQ5J8b3x0+gkOo+d2w@mail.gmail.com> 2018-12-17 19:20 ` Philipp Stephani @ 2018-12-17 19:33 ` Michael Albinus 2018-12-17 20:03 ` Drew Adams 2 siblings, 0 replies; 27+ messages in thread From: Michael Albinus @ 2018-12-17 19:33 UTC (permalink / raw) To: Philipp Stephani; +Cc: Philipp Stephani, 28691, Emacs developers Philipp Stephani <p.stephani2@gmail.com> writes: Hi Philipp, >> What happens, if no file name handler is found? Should there be a local >> process then, or should this be ignored (returning nil)? > > I think it should be a new process, like start-file-process does. > (make-process should always either return a process object or fail.) Why fail? See my answer to Eli, I prefer that it returns nil if it isn't possible to get a new process from the file name handler. >> And of course, the implementation of a file name handler is missing in >> tramp-adb.el, tramp-sh.el and tramp-smb.el. > > That should be part of a different bug. (Since make-process is more > capable than start-file-process, implementing Tramp support will be a > bit more difficult.) No new bug needed. I have the feeling it's me who has to implement it in Tramp. Or do you know a volunteer? I would appreciate it :-) Best regards, Michael. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) [not found] ` <CAArVCkSzt=pcVG9ZRpdT6oSi30ExcqBGAQ5J8b3x0+gkOo+d2w@mail.gmail.com> 2018-12-17 19:20 ` Philipp Stephani 2018-12-17 19:33 ` Michael Albinus @ 2018-12-17 20:03 ` Drew Adams 2 siblings, 0 replies; 27+ messages in thread From: Drew Adams @ 2018-12-17 20:03 UTC (permalink / raw) To: Philipp Stephani, Michael Albinus Cc: Philipp Stephani, 28691, Emacs developers Could these mails please be sent to only the bugs list or only emacs-devel, please? ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <838t0odpk8.fsf@gnu.org>]
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) [not found] ` <838t0odpk8.fsf@gnu.org> @ 2018-12-17 19:30 ` Michael Albinus 0 siblings, 0 replies; 27+ messages in thread From: Michael Albinus @ 2018-12-17 19:30 UTC (permalink / raw) To: Eli Zaretskii; +Cc: phst, 28691, p.stephani2, emacs-devel Eli Zaretskii <eliz@gnu.org> writes: Hi Eli, >> What happens, if no file name handler is found? Should there be a local >> process then, or should this be ignored (returning nil)? > > The proposed code runs the process locally, which I think is > reasonable. Yes, when default-directory is local. If default-directory is remote, and :file-handler indicates that a remote process shall run, the situation is different. If the corresponding file name handler offers an own make-network-process implementation, it returns that process. But if the file name handler misses an implementation, no local process shall run. This is the same situation as with start-file-process. The file name handler for ssh returns a process object: --8<---------------cut here---------------start------------->8--- (with-temp-buffer (let ((default-directory "/ssh::")) (start-file-process "foo" (current-buffer) "/bin/true"))) => #<process foo> --8<---------------cut here---------------end--------------->8--- But for Tramp methods which do not own a start-file-process implementation, no process is created: --8<---------------cut here---------------start------------->8--- (with-temp-buffer (let ((default-directory "/sftp::")) (start-file-process "foo" (current-buffer) "/bin/true"))) => nil --8<---------------cut here---------------end--------------->8--- This is documented in the Lisp Manual, see node "Asynchronous Processes": --8<---------------cut here---------------start------------->8--- Some file handlers may not support ‘start-file-process’ (for example the function ‘ange-ftp-hook-function’). In such cases, this function does nothing and returns ‘nil’. --8<---------------cut here---------------end--------------->8--- I believe make-network-process shall be have similar. Best regards, Michael. ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20181216233936.208568-1-phst@google.com>]
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) [not found] ` <20181216233936.208568-1-phst@google.com> @ 2018-12-17 17:34 ` Eli Zaretskii 0 siblings, 0 replies; 27+ messages in thread From: Eli Zaretskii @ 2018-12-17 17:34 UTC (permalink / raw) To: Philipp Stephani; +Cc: phst, 28691, emacs-devel > From: Philipp Stephani <p.stephani2@gmail.com> > Date: Mon, 17 Dec 2018 00:39:36 +0100 > Cc: Philipp Stephani <phst@google.com> > > --- a/etc/NEWS > +++ b/etc/NEWS > @@ -1407,6 +1407,10 @@ 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 search for a file name handler for > +'default-directory'. I think the feature you introducing is not to "search for a file name handler for 'default-directory'", the feature is to (attempt to) make a remote process using that file handler. I think the documentation should make that more explicit, because I very much doubt it could be otherwise gleaned from this description. I'd also suggest to say "... the current buffer's 'default-directory'", just to make sure people don't miss that. > +:file-handler FILE-HANDLER -- If FILE-HANDLER is non-nil, then search > +for a file name handler for `default-directory'. Likewise here. Also, please describe what happens if the search for the handler fails. > +(ert-deftest make-process/file-handler () > + "Check that the ‘:file-handler’ argument of ‘make-process’ > +works as expected." > + (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") /bin/true is not portable enough, I suggest to use Emacs itself instead. > + (let ((file-name-handler-alist > + (cons (cons (rx bos "test-handler:") #'file-handler) > + file-name-handler-alist)) I think it would be preferable to empty file-name-handler-alist of any other members -- that will make this test 100% deterministic. Thanks. ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <mailman.5866.1545079749.1284.bug-gnu-emacs@gnu.org>]
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) [not found] ` <mailman.5866.1545079749.1284.bug-gnu-emacs@gnu.org> @ 2018-12-18 1:57 ` Alan Mackenzie 2018-12-18 7:56 ` Michael Albinus 2018-12-22 21:05 ` Philipp Stephani 0 siblings, 2 replies; 27+ messages in thread From: Alan Mackenzie @ 2018-12-18 1:57 UTC (permalink / raw) To: Philipp Stephani; +Cc: 28691 Hello, Philipp, I hope I'm not late to the party, here, but.... What is this patch about, and what is it for? 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? 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? 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). ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-18 1:57 ` Alan Mackenzie @ 2018-12-18 7:56 ` Michael Albinus 2018-12-22 21:05 ` Philipp Stephani 1 sibling, 0 replies; 27+ messages in thread From: Michael Albinus @ 2018-12-18 7:56 UTC (permalink / raw) To: Alan Mackenzie; +Cc: 28691, Philipp Stephani Alan Mackenzie <acm@muc.de> writes: > Hello, Philipp, Hi Alan, > Are we talking about a "file name handler" or a "file handler", here? > Any chance of consistency? What is one of these? In what way does it > "handle" the file, or the file name? We're speaking about a "file name handler". And yes, even the documentation is sloppy with this, sometimes. I'll walk through the documentation, and I'll make it consistent. Next days. > 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? See (info "(elisp) Magic File Names") , or read the Tramp sources as an example :-) The patch prepares make-network-process to recognise a file name handler, if existent. The implementation in the handler (again, Tramp as example) is not part of this patch. Best regards, Michael. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) 2018-12-18 1:57 ` Alan Mackenzie 2018-12-18 7:56 ` Michael Albinus @ 2018-12-22 21:05 ` Philipp Stephani 1 sibling, 0 replies; 27+ messages in thread From: Philipp Stephani @ 2018-12-22 21:05 UTC (permalink / raw) To: Alan Mackenzie; +Cc: 28691 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). > ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2018-12-23 16:50 UTC | newest] Thread overview: 27+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-03 17:13 bug#28691: 27.0.50; make-process has no file-name-handler Stefan Monnier 2017-10-03 17:50 ` Michael Albinus 2017-10-03 18:34 ` Eli Zaretskii 2018-12-16 23:39 ` bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691) Philipp Stephani 2018-12-17 12:38 ` Michael Albinus 2018-12-17 17:35 ` Eli Zaretskii 2018-12-17 19:07 ` Philipp Stephani [not found] ` <CAArVCkSzt=pcVG9ZRpdT6oSi30ExcqBGAQ5J8b3x0+gkOo+d2w@mail.gmail.com> 2018-12-17 19:20 ` Philipp Stephani 2018-12-17 19:58 ` Eli Zaretskii 2018-12-17 20:47 ` Philipp Stephani 2018-12-22 9:07 ` Eli Zaretskii 2018-12-22 9:31 ` Michael Albinus 2018-12-22 21:08 ` Philipp Stephani 2018-12-23 7:41 ` Michael Albinus 2018-12-23 10:08 ` Philipp Stephani 2018-12-23 10:26 ` Michael Albinus 2018-12-23 15:45 ` Eli Zaretskii 2018-12-23 16:36 ` Michael Albinus 2018-12-23 16:50 ` Eli Zaretskii 2018-12-22 22:26 ` Philipp Stephani 2018-12-17 19:33 ` Michael Albinus 2018-12-17 20:03 ` Drew Adams [not found] ` <838t0odpk8.fsf@gnu.org> 2018-12-17 19:30 ` Michael Albinus [not found] ` <20181216233936.208568-1-phst@google.com> 2018-12-17 17:34 ` Eli Zaretskii [not found] ` <mailman.5866.1545079749.1284.bug-gnu-emacs@gnu.org> 2018-12-18 1:57 ` Alan Mackenzie 2018-12-18 7:56 ` Michael Albinus 2018-12-22 21:05 ` Philipp Stephani
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).