* 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; 37+ 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] 37+ 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 ` [PATCH] Add file name handler support for 'make-process' (Bug#28691) Philipp Stephani
` (2 subsequent siblings)
3 siblings, 1 reply; 37+ 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] 37+ 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; 37+ 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] 37+ 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 ` [PATCH] Add file name handler support for 'make-process' (Bug#28691) Philipp Stephani
@ 2018-12-16 23:39 ` Philipp Stephani
2018-12-17 12:38 ` Michael Albinus
[not found] ` <mailman.5866.1545079749.1284.bug-gnu-emacs@gnu.org>
3 siblings, 1 reply; 37+ 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] 37+ messages in thread
* [PATCH] Add file name handler support for 'make-process' (Bug#28691)
@ 2018-12-16 23:39 ` Philipp Stephani
2018-12-17 17:34 ` bug#28691: " Eli Zaretskii
2018-12-17 17:34 ` Eli Zaretskii
0 siblings, 2 replies; 37+ 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] 37+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-16 23:39 ` Philipp Stephani
@ 2018-12-17 12:38 ` Michael Albinus
2018-12-17 17:35 ` Eli Zaretskii
` (3 more replies)
0 siblings, 4 replies; 37+ 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] 37+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-16 23:39 ` [PATCH] Add file name handler support for 'make-process' (Bug#28691) Philipp Stephani
@ 2018-12-17 17:34 ` Eli Zaretskii
2018-12-17 17:34 ` Eli Zaretskii
1 sibling, 0 replies; 37+ 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] 37+ messages in thread
* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-16 23:39 ` [PATCH] Add file name handler support for 'make-process' (Bug#28691) Philipp Stephani
2018-12-17 17:34 ` bug#28691: " Eli Zaretskii
@ 2018-12-17 17:34 ` Eli Zaretskii
2018-12-17 17:54 ` Stefan Monnier
1 sibling, 1 reply; 37+ 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] 37+ 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 17:35 ` Eli Zaretskii
` (2 subsequent siblings)
3 siblings, 0 replies; 37+ 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] 37+ messages in thread
* Re: 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 17:35 ` Eli Zaretskii
2018-12-17 19:30 ` Michael Albinus
2018-12-17 19:30 ` Michael Albinus
2018-12-17 19:07 ` Philipp Stephani
2018-12-17 19:07 ` Philipp Stephani
3 siblings, 2 replies; 37+ 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] 37+ messages in thread
* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-17 17:34 ` Eli Zaretskii
@ 2018-12-17 17:54 ` Stefan Monnier
2018-12-17 19:49 ` Michael Albinus
0 siblings, 1 reply; 37+ messages in thread
From: Stefan Monnier @ 2018-12-17 17:54 UTC (permalink / raw)
To: emacs-devel
> 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.
"Remote" is just one possibility. It can also be local under
a different UID, or it could be local in a virtual machine, ...
The way I think of it is that this argument to `make-process` tells
Emacs that this `make-process` call should be considered a file
operation, in the sense that the place where the process is run will be
chosen according to default-directory and file-name-handler-alist.
Stefan
^ permalink raw reply [flat|nested] 37+ 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 17:35 ` Eli Zaretskii
@ 2018-12-17 19:07 ` Philipp Stephani
2018-12-17 19:07 ` Philipp Stephani
3 siblings, 0 replies; 37+ 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] 37+ messages in thread
* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-17 12:38 ` Michael Albinus
` (2 preceding siblings ...)
2018-12-17 19:07 ` Philipp Stephani
@ 2018-12-17 19:07 ` Philipp Stephani
2018-12-17 19:20 ` Philipp Stephani
` (3 more replies)
3 siblings, 4 replies; 37+ 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] 37+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-17 19:07 ` Philipp Stephani
@ 2018-12-17 19:20 ` Philipp Stephani
2018-12-17 19:58 ` Eli Zaretskii
2018-12-17 19:33 ` Michael Albinus
` (2 subsequent siblings)
3 siblings, 1 reply; 37+ 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] 37+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-17 17:35 ` Eli Zaretskii
2018-12-17 19:30 ` Michael Albinus
@ 2018-12-17 19:30 ` Michael Albinus
1 sibling, 0 replies; 37+ 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] 37+ messages in thread
* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-17 17:35 ` Eli Zaretskii
@ 2018-12-17 19:30 ` Michael Albinus
2018-12-17 19:30 ` Michael Albinus
1 sibling, 0 replies; 37+ 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] 37+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-17 19:07 ` Philipp Stephani
2018-12-17 19:20 ` Philipp Stephani
@ 2018-12-17 19:33 ` Michael Albinus
2018-12-17 20:03 ` Drew Adams
2018-12-17 20:03 ` Drew Adams
3 siblings, 0 replies; 37+ 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] 37+ messages in thread
* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-17 17:54 ` Stefan Monnier
@ 2018-12-17 19:49 ` Michael Albinus
2018-12-17 19:57 ` Stefan Monnier
0 siblings, 1 reply; 37+ messages in thread
From: Michael Albinus @ 2018-12-17 19:49 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
Hi Stefan,
> "Remote" is just one possibility. It can also be local under
> a different UID, or it could be local in a virtual machine, ...
The current implementation regards both cases also as "remote", provided
by Tramp. file-remote-p returns non-nil.
It might be unfortune, but I don't recommend to change the function's name.
> The way I think of it is that this argument to `make-process` tells
> Emacs that this `make-process` call should be considered a file
> operation, in the sense that the place where the process is run will be
> chosen according to default-directory and file-name-handler-alist.
Yes.
> Stefan
Best regards, Michael.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-17 19:49 ` Michael Albinus
@ 2018-12-17 19:57 ` Stefan Monnier
2018-12-17 20:01 ` Eli Zaretskii
0 siblings, 1 reply; 37+ messages in thread
From: Stefan Monnier @ 2018-12-17 19:57 UTC (permalink / raw)
To: Michael Albinus; +Cc: emacs-devel
>> "Remote" is just one possibility. It can also be local under
>> a different UID, or it could be local in a virtual machine, ...
> The current implementation regards both cases also as "remote", provided
> by Tramp. file-remote-p returns non-nil.
> It might be unfortune, but I don't recommend to change the function's name.
I'm not suggesting to rename that function, but I think we should try and
keep this as an exception rather than start using the word "remote"
always meaning "as defined by file-remote-p" rather than the more
common meaning.
Stefan
^ permalink raw reply [flat|nested] 37+ 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; 37+ 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] 37+ messages in thread
* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-17 19:57 ` Stefan Monnier
@ 2018-12-17 20:01 ` Eli Zaretskii
0 siblings, 0 replies; 37+ messages in thread
From: Eli Zaretskii @ 2018-12-17 20:01 UTC (permalink / raw)
To: Stefan Monnier; +Cc: michael.albinus, emacs-devel
> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Mon, 17 Dec 2018 14:57:04 -0500
> Cc: emacs-devel@gnu.org
>
> >> "Remote" is just one possibility. It can also be local under
> >> a different UID, or it could be local in a virtual machine, ...
> > The current implementation regards both cases also as "remote", provided
> > by Tramp. file-remote-p returns non-nil.
> > It might be unfortune, but I don't recommend to change the function's name.
>
> I'm not suggesting to rename that function, but I think we should try and
> keep this as an exception rather than start using the word "remote"
> always meaning "as defined by file-remote-p" rather than the more
> common meaning.
I think you can trust me to understand all that. I used "remote" to
make my point more clear, expressed with simpler wording, that's all.
^ permalink raw reply [flat|nested] 37+ messages in thread
* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-17 19:07 ` Philipp Stephani
2018-12-17 19:20 ` Philipp Stephani
2018-12-17 19:33 ` Michael Albinus
@ 2018-12-17 20:03 ` Drew Adams
2018-12-17 20:03 ` Drew Adams
3 siblings, 0 replies; 37+ 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] 37+ messages in thread
* RE: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
2018-12-17 19:07 ` Philipp Stephani
` (2 preceding siblings ...)
2018-12-17 20:03 ` Drew Adams
@ 2018-12-17 20:03 ` Drew Adams
3 siblings, 0 replies; 37+ 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] 37+ 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; 37+ 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] 37+ messages in thread
* 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; 37+ 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] 37+ 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; 37+ 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] 37+ 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; 37+ 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] 37+ 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; 37+ 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] 37+ 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; 37+ 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] 37+ 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; 37+ 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] 37+ 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; 37+ 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] 37+ 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; 37+ 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] 37+ 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; 37+ 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] 37+ 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; 37+ 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] 37+ 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; 37+ 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] 37+ 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; 37+ 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] 37+ 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; 37+ 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] 37+ messages in thread
end of thread, other threads:[~2018-12-23 16:50 UTC | newest]
Thread overview: 37+ 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 ` [PATCH] Add file name handler support for 'make-process' (Bug#28691) Philipp Stephani
2018-12-17 17:34 ` bug#28691: " Eli Zaretskii
2018-12-17 17:34 ` Eli Zaretskii
2018-12-17 17:54 ` Stefan Monnier
2018-12-17 19:49 ` Michael Albinus
2018-12-17 19:57 ` Stefan Monnier
2018-12-17 20:01 ` Eli Zaretskii
2018-12-16 23:39 ` Philipp Stephani
2018-12-17 12:38 ` Michael Albinus
2018-12-17 17:35 ` Eli Zaretskii
2018-12-17 17:35 ` Eli Zaretskii
2018-12-17 19:30 ` Michael Albinus
2018-12-17 19:30 ` Michael Albinus
2018-12-17 19:07 ` Philipp Stephani
2018-12-17 19:07 ` Philipp Stephani
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
2018-12-17 20:03 ` Drew Adams
[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 external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.