[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: bug#28691: [PATCH] Add file name handler support for 'make-process'
Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
Mon, 17 Dec 2018 19:34:23 +0200
> From: Philipp Stephani <address@hidden>
> Date: Mon, 17 Dec 2018 00:39:36 +0100
> Cc: Philipp Stephani <address@hidden>
> --- 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
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
> + (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.