unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#65030: 30.0.50; Check keyword args of make-process
@ 2023-08-03  6:47 Helmut Eller
  2023-08-05  9:21 ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Helmut Eller @ 2023-08-03  6:47 UTC (permalink / raw)
  To: 65030

[-- Attachment #1: Type: text/plain, Size: 608 bytes --]

The functions make-process and make-network-process have many keyword
arguments and it's somewhat easy to misspell some of them.  E.g. using
:coding-system instead of :coding.  These functions don't detect such
mistakes at runtime.  What would people think about adding some checks
as a compiler macro as with the patch below?

I didn't know where to put this, so I just left it in bytecomp.el.

Perhaps the advertised-calling-convention declaration could do this, but
since keyword arguments seem to be generally discouraged, a special case
for make-process and make-network-process maybe simpler.

Helmut


[-- Attachment #2: 0001-Check-keyword-args-of-make-process.patch --]
[-- Type: text/x-diff, Size: 6874 bytes --]

From 758ba9b8b26333fc60ca4693616c5f2b355d4fcc Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Thu, 3 Aug 2023 08:33:40 +0200
Subject: [PATCH] Check keyword args of make-process

The functions make-process and make-network-process have many
keyword args and it's easy to misspell some of them.

Use a compiler macro to warn about some possible mistakes.

* lisp/emacs-lisp/bytecomp.el (bytecomp--check-keyword-args): New
  helper.
  (make-process, make-network-process): Define a compiler macro that
  performs some checks but doesn't anything else.

* test/lisp/emacs-lisp/bytecomp-tests.el: Add some tests.

* test/lisp/emacs-lisp/bytecomp-resources/:
  (warn-make-process-missing-keyword-arg.el,
   warn-make-process-missing-keyword-value.el,
   warn-make-process-repeated-keyword-arg.el,
   warn-make-process-unknown-keyword-arg.el): New test files
---
 lisp/emacs-lisp/bytecomp.el                   | 61 +++++++++++++++++++
 .../warn-make-process-missing-keyword-arg.el  |  3 +
 ...warn-make-process-missing-keyword-value.el |  3 +
 .../warn-make-process-repeated-keyword-arg.el |  3 +
 .../warn-make-process-unknown-keyword-arg.el  |  4 ++
 test/lisp/emacs-lisp/bytecomp-tests.el        | 16 +++++
 6 files changed, 90 insertions(+)
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-arg.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-value.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-repeated-keyword-arg.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-unknown-keyword-arg.el

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 5b1d958e6c2..d3a434b5593 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -5782,6 +5782,67 @@ bytecomp--backward-word
       form    ; arity error
     `(forward-word (- (or ,arg 1)))))
 
+(defun bytecomp--check-keyword-args (form arglist allowed-keys required-keys)
+  (let ((fun (car form)))
+    (cl-flet ((missing (form keyword)
+		(byte-compile-warn-x
+		 form
+		 "`%S´ called without required keyword argument %S"
+		 fun keyword))
+	      (unrecognized (form keyword)
+		(byte-compile-warn-x
+		 form
+		 "`%S´ called with unknown keyword argument %S"
+		 fun keyword))
+	      (duplicate (form keyword)
+		(byte-compile-warn-x
+		 form
+		 "`%S´ called with repeated keyword argument %S"
+		 fun keyword))
+              (missing-val (form keyword)
+		(byte-compile-warn-x
+		 form
+		 "missing value for keyword argument %S"
+		 keyword)))
+      (let* ((seen '())
+	     (l arglist))
+	(while (consp l)
+	  (let ((key (car l)))
+	    (cond ((and (keywordp key) (memq key allowed-keys))
+		   (cond ((memq key seen)
+			  (duplicate l key))
+			 (t
+			  (push key seen))))
+		  (t (unrecognized l key)))
+            (when (null (cdr l))
+              (missing-val l key)))
+	  (setq l (cddr l)))
+        (dolist (key required-keys)
+	  (unless (memq key seen)
+	    (missing form key))))))
+  form)
+
+(put 'make-process 'compiler-macro
+     #'(lambda (form &rest args)
+         (bytecomp--check-keyword-args
+          form args
+          '(:name
+            :buffer :command :coding :noquery :stop :connection-type
+            :filter :sentinel :stderr :file-handler)
+          '(:name :command))))
+
+(put 'make-network-process 'compiler-macro
+     #'(lambda (form &rest args)
+         (bytecomp--check-keyword-args
+          form args
+          '(:name
+            :buffer :host :service :type :family :local :remote :coding
+            :nowait :noquery :stop :filter :filter-multibyte :sentinel
+            :log :plist :tls-parameters :server :broadcast :dontroute
+            :keepalive :linger :oobinline :priority :reuseaddr :bindtodevice
+            :use-external-socket)
+          '(:name :service))))
+
 (provide 'byte-compile)
 (provide 'bytecomp)
 
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-arg.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-arg.el
new file mode 100644
index 00000000000..9369e78ff54
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-arg.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defun foo ()
+  (make-process :name "ls"))
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-value.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-value.el
new file mode 100644
index 00000000000..4226349afef
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-value.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defun foo ()
+  (make-process :name "ls" :command))
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-repeated-keyword-arg.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-repeated-keyword-arg.el
new file mode 100644
index 00000000000..18250f14ee9
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-repeated-keyword-arg.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defun foo ()
+  (make-process :name "ls" :command "ls" :name "ls"))
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-unknown-keyword-arg.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-unknown-keyword-arg.el
new file mode 100644
index 00000000000..4721035780b
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-unknown-keyword-arg.el
@@ -0,0 +1,4 @@
+;;; -*- lexical-binding: t -*-
+(defun foo ()
+  (make-process :name "ls" :command "ls"
+                :coding-system 'binary))
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index 593fd117685..6ed907ead7b 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -1199,6 +1199,22 @@ bytecomp--tests-obsolete-var
  "nowarn-inline-after-defvar.el"
  "Lexical argument shadows" 'reverse)
 
+(bytecomp--define-warning-file-test
+ "warn-make-process-missing-keyword-arg.el"
+ "called without required keyword argument :command")
+
+(bytecomp--define-warning-file-test
+ "warn-make-process-unknown-keyword-arg.el"
+ "called with unknown keyword argument :coding-system")
+
+(bytecomp--define-warning-file-test
+ "warn-make-process-repeated-keyword-arg.el"
+ "called with repeated keyword argument :name")
+
+(bytecomp--define-warning-file-test
+ "warn-make-process-missing-keyword-value.el"
+ "missing value for keyword argument :command")
+
 \f
 ;;;; Macro expansion.
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-03  6:47 bug#65030: 30.0.50; Check keyword args of make-process Helmut Eller
@ 2023-08-05  9:21 ` Eli Zaretskii
  2023-08-05 23:07   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2023-08-05  9:21 UTC (permalink / raw)
  To: Helmut Eller, Mattias Engdegård; +Cc: 65030

> From: Helmut Eller <eller.helmut@gmail.com>
> Date: Thu, 03 Aug 2023 08:47:44 +0200
> 
> The functions make-process and make-network-process have many keyword
> arguments and it's somewhat easy to misspell some of them.  E.g. using
> :coding-system instead of :coding.  These functions don't detect such
> mistakes at runtime.  What would people think about adding some checks
> as a compiler macro as with the patch below?
> 
> I didn't know where to put this, so I just left it in bytecomp.el.
> 
> Perhaps the advertised-calling-convention declaration could do this, but
> since keyword arguments seem to be generally discouraged, a special case
> for make-process and make-network-process maybe simpler.

Mattias, Robert and Stefan: any comments on this?

Thanks.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-05  9:21 ` Eli Zaretskii
@ 2023-08-05 23:07   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-08-06  4:58     ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-08-05 23:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Mattias Engdegård, Robert Pluim, 65030, Helmut Eller

>> The functions make-process and make-network-process have many keyword
>> arguments and it's somewhat easy to misspell some of them.  E.g. using
>> :coding-system instead of :coding.  These functions don't detect such
>> mistakes at runtime.  What would people think about adding some checks
>> as a compiler macro as with the patch below?

Good idea, thanks.


        Stefan






^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-05 23:07   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-08-06  4:58     ` Eli Zaretskii
  2023-08-06  8:49       ` Mattias Engdegård
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2023-08-06  4:58 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: mattiase, rpluim, 65030, eller.helmut

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Helmut Eller <eller.helmut@gmail.com>,  Mattias Engdegård
>  <mattiase@acm.org>, Robert Pluim <rpluim@gmail.com>,
>   65030@debbugs.gnu.org
> Date: Sat, 05 Aug 2023 19:07:53 -0400
> 
> >> The functions make-process and make-network-process have many keyword
> >> arguments and it's somewhat easy to misspell some of them.  E.g. using
> >> :coding-system instead of :coding.  These functions don't detect such
> >> mistakes at runtime.  What would people think about adding some checks
> >> as a compiler macro as with the patch below?
> 
> Good idea, thanks.

Any specific comments to the proposed patch?  Or do you think it is
good to go?





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-06  4:58     ` Eli Zaretskii
@ 2023-08-06  8:49       ` Mattias Engdegård
  2023-08-08  8:52         ` Robert Pluim
  0 siblings, 1 reply; 21+ messages in thread
From: Mattias Engdegård @ 2023-08-06  8:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: rpluim, 65030, eller.helmut, Stefan Monnier

6 aug. 2023 kl. 06.58 skrev Eli Zaretskii <eliz@gnu.org>:

> Any specific comments to the proposed patch?  Or do you think it is
> good to go?

Good to go as far as I'm concerned. It will be genuinely useful, and I see no serious problems with the implementation.

It can be extended but that would not prevent it from being committed as-is. For example, something that detects omitted values in the middle, not just the end, of the argument list. (The feasibility of this depends on the likelihood of argument values being keywords themselves.)

The main objection is that `make-process`, due to its design, is often called indirectly using `apply` which would not trigger the application of this compiler macro, so perhaps we should improve the error handling of Fmake_process instead.







^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-06  8:49       ` Mattias Engdegård
@ 2023-08-08  8:52         ` Robert Pluim
  2023-08-08  9:16           ` Mattias Engdegård
  2023-08-08 13:37           ` Visuwesh
  0 siblings, 2 replies; 21+ messages in thread
From: Robert Pluim @ 2023-08-08  8:52 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: Eli Zaretskii, 65030, eller.helmut, Stefan Monnier

>>>>> On Sun, 6 Aug 2023 10:49:59 +0200, Mattias Engdegård <mattiase@acm.org> said:

    Mattias> 6 aug. 2023 kl. 06.58 skrev Eli Zaretskii <eliz@gnu.org>:
    >> Any specific comments to the proposed patch?  Or do you think it is
    >> good to go?

    Mattias> Good to go as far as I'm concerned. It will be genuinely useful, and I
    Mattias> see no serious problems with the implementation.

    Mattias> It can be extended but that would not prevent it from being committed
    Mattias> as-is. For example, something that detects omitted values in the
    Mattias> middle, not just the end, of the argument list. (The feasibility of
    Mattias> this depends on the likelihood of argument values being keywords
    Mattias> themselves.)

I donʼt think any of the `make-process' keywords accept keywords as
values, but missing values tends to cause catastrophic failure, so I
donʼt think itʼs that common a mistake.

    Mattias> The main objection is that `make-process`, due to its design, is often
    Mattias> called indirectly using `apply` which would not trigger the
    Mattias> application of this compiler macro, so perhaps we should improve the
    Mattias> error handling of Fmake_process instead.

As long as that improvement results in warnings for mistakes such as
misspelled keywords, rather than errors.

Robert
-- 





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08  8:52         ` Robert Pluim
@ 2023-08-08  9:16           ` Mattias Engdegård
  2023-08-08  9:27             ` Robert Pluim
  2023-08-08 12:16             ` Eli Zaretskii
  2023-08-08 13:37           ` Visuwesh
  1 sibling, 2 replies; 21+ messages in thread
From: Mattias Engdegård @ 2023-08-08  9:16 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Eli Zaretskii, 65030, eller.helmut, Stefan Monnier

8 aug. 2023 kl. 10.52 skrev Robert Pluim <rpluim@gmail.com>:

> I donʼt think any of the `make-process' keywords accept keywords as
> values, but missing values tends to cause catastrophic failure, so I
> donʼt think itʼs that common a mistake.

No, the usefulness of good compiler warnings goes far beyond that. A determined programmer may eventually get something working despite an error-prone API, but decent diagnostics will speed up the process manifold by pointing out mistakes before the code is even run. (With flymake/flycheck, even faster.)

>    Mattias> The main objection is that `make-process`, due to its design, is often
>    Mattias> called indirectly using `apply` which would not trigger the
>    Mattias> application of this compiler macro, so perhaps we should improve the
>    Mattias> error handling of Fmake_process instead.
> 
> As long as that improvement results in warnings for mistakes such as
> misspelled keywords, rather than errors.

Of course not. Incorrect arguments detected at run-time should elicit errors, as they do now.
The point being that for something as complex as make-process they should be more helpful than just `wrong-type-argument`.






^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08  9:16           ` Mattias Engdegård
@ 2023-08-08  9:27             ` Robert Pluim
  2023-08-08  9:42               ` Mattias Engdegård
  2023-08-08 12:16             ` Eli Zaretskii
  1 sibling, 1 reply; 21+ messages in thread
From: Robert Pluim @ 2023-08-08  9:27 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: Eli Zaretskii, 65030, eller.helmut, Stefan Monnier

>>>>> On Tue, 8 Aug 2023 11:16:07 +0200, Mattias Engdegård <mattiase@acm.org> said:
    >> 
    >> As long as that improvement results in warnings for mistakes such as
    >> misspelled keywords, rather than errors.

    Mattias> Of course not. Incorrect arguments detected at run-time
    Mattias> should elicit errors, as they do now.  The point being
    Mattias> that for something as complex as make-process they should
    Mattias> be more helpful than just `wrong-type-argument`.

You want to transform working code that passes ':coding-system' to
`make-process' into code that signals an error? (yes, it doesnʼt work
100% as the programmer thinks it does, but that means we should warn,
not error).

Robert
-- 





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08  9:27             ` Robert Pluim
@ 2023-08-08  9:42               ` Mattias Engdegård
  0 siblings, 0 replies; 21+ messages in thread
From: Mattias Engdegård @ 2023-08-08  9:42 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Eli Zaretskii, 65030, eller.helmut, Stefan Monnier

8 aug. 2023 kl. 11.27 skrev Robert Pluim <rpluim@gmail.com>:

> You want to transform working code that passes ':coding-system' to
> `make-process' into code that signals an error?

That's probably a good idea (unless we are talking about a specific keyword that has to be ignored for compatibility), but I think we should start by improving existing errors.

However it should not preclude compile-time warnings of the kind proposed in the patch, which again can be applied more or less as-is.






^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08  9:16           ` Mattias Engdegård
  2023-08-08  9:27             ` Robert Pluim
@ 2023-08-08 12:16             ` Eli Zaretskii
  2023-08-08 13:05               ` Mattias Engdegård
  1 sibling, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2023-08-08 12:16 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: rpluim, 65030, eller.helmut, monnier

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Tue, 8 Aug 2023 11:16:07 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>,
>         eller.helmut@gmail.com, 65030@debbugs.gnu.org
> 
> >    Mattias> The main objection is that `make-process`, due to its design, is often
> >    Mattias> called indirectly using `apply` which would not trigger the
> >    Mattias> application of this compiler macro, so perhaps we should improve the
> >    Mattias> error handling of Fmake_process instead.
> > 
> > As long as that improvement results in warnings for mistakes such as
> > misspelled keywords, rather than errors.
> 
> Of course not. Incorrect arguments detected at run-time should elicit errors, as they do now.
> The point being that for something as complex as make-process they should be more helpful than just `wrong-type-argument`.

Misspelled arguments to make-process don't elicit errors as of now, do
they?





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08 12:16             ` Eli Zaretskii
@ 2023-08-08 13:05               ` Mattias Engdegård
  2023-08-08 13:18                 ` Robert Pluim
  0 siblings, 1 reply; 21+ messages in thread
From: Mattias Engdegård @ 2023-08-08 13:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: rpluim, 65030, eller.helmut, monnier

8 aug. 2023 kl. 14.16 skrev Eli Zaretskii <eliz@gnu.org>:

> Misspelled arguments to make-process don't elicit errors as of now, do
> they?

No and that would be good to fix as well, but we should first make existing errors understandable.

For example, (wrong-type-argument stringp nil) is not a human-readable way to say "You forgot the :name parameter which is actually mandatory despite the docs making this almost impossible to find out"






^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08 13:05               ` Mattias Engdegård
@ 2023-08-08 13:18                 ` Robert Pluim
  2023-08-08 16:38                   ` Mattias Engdegård
  0 siblings, 1 reply; 21+ messages in thread
From: Robert Pluim @ 2023-08-08 13:18 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: Eli Zaretskii, 65030, eller.helmut, monnier

>>>>> On Tue, 8 Aug 2023 15:05:06 +0200, Mattias Engdegård <mattiase@acm.org> said:

    Mattias> 8 aug. 2023 kl. 14.16 skrev Eli Zaretskii <eliz@gnu.org>:
    >> Misspelled arguments to make-process don't elicit errors as of now, do
    >> they?

    Mattias> No and that would be good to fix as well, but we should
    Mattias> first make existing errors understandable.

    Mattias> For example, (wrong-type-argument stringp nil) is not a
    Mattias> human-readable way to say "You forgot the :name parameter
    Mattias> which is actually mandatory despite the docs making this
    Mattias> almost impossible to find out"

Youʼll get no objections from me for that. Perhaps we need a
CHECK_STRING_WITH_MESSAGE macro or similar.

Robert
-- 





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08  8:52         ` Robert Pluim
  2023-08-08  9:16           ` Mattias Engdegård
@ 2023-08-08 13:37           ` Visuwesh
  2023-08-08 13:52             ` Robert Pluim
  1 sibling, 1 reply; 21+ messages in thread
From: Visuwesh @ 2023-08-08 13:37 UTC (permalink / raw)
  To: Robert Pluim
  Cc: Mattias Engdegård, Eli Zaretskii, 65030, eller.helmut,
	Stefan Monnier

[Tuesday August 08, 2023] Robert Pluim wrote:

>>>>>> On Sun, 6 Aug 2023 10:49:59 +0200, Mattias Engdegård <mattiase@acm.org> said:
>
>     Mattias> 6 aug. 2023 kl. 06.58 skrev Eli Zaretskii <eliz@gnu.org>:
>     >> Any specific comments to the proposed patch?  Or do you think it is
>     >> good to go?
>
>     Mattias> Good to go as far as I'm concerned. It will be genuinely useful, and I
>     Mattias> see no serious problems with the implementation.
>
>     Mattias> It can be extended but that would not prevent it from being committed
>     Mattias> as-is. For example, something that detects omitted values in the
>     Mattias> middle, not just the end, of the argument list. (The feasibility of
>     Mattias> this depends on the likelihood of argument values being keywords
>     Mattias> themselves.)
>
> I donʼt think any of the `make-process' keywords accept keywords as
> values, but missing values tends to cause catastrophic failure, so I
> donʼt think itʼs that common a mistake.

Can you not pass keywords as non-nil values to :query and :stop?
Something like,

    (make-process :name NAME :command COMMAND :query :yes)






^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08 13:37           ` Visuwesh
@ 2023-08-08 13:52             ` Robert Pluim
  0 siblings, 0 replies; 21+ messages in thread
From: Robert Pluim @ 2023-08-08 13:52 UTC (permalink / raw)
  To: Visuwesh
  Cc: Mattias Engdegård, Eli Zaretskii, 65030, eller.helmut,
	Stefan Monnier

>>>>> On Tue, 08 Aug 2023 19:07:32 +0530, Visuwesh <visuweshm@gmail.com> said:

    Visuwesh> [Tuesday August 08, 2023] Robert Pluim wrote:
    >>>>>>> On Sun, 6 Aug 2023 10:49:59 +0200, Mattias Engdegård <mattiase@acm.org> said:
    >> 
    Mattias> 6 aug. 2023 kl. 06.58 skrev Eli Zaretskii <eliz@gnu.org>:
    >> >> Any specific comments to the proposed patch?  Or do you think it is
    >> >> good to go?
    >> 
    Mattias> Good to go as far as I'm concerned. It will be genuinely useful, and I
    Mattias> see no serious problems with the implementation.
    >> 
    Mattias> It can be extended but that would not prevent it from being committed
    Mattias> as-is. For example, something that detects omitted values in the
    Mattias> middle, not just the end, of the argument list. (The feasibility of
    Mattias> this depends on the likelihood of argument values being keywords
    Mattias> themselves.)
    >> 
    >> I donʼt think any of the `make-process' keywords accept keywords as
    >> values, but missing values tends to cause catastrophic failure, so I
    >> donʼt think itʼs that common a mistake.

    Visuwesh> Can you not pass keywords as non-nil values to :query and :stop?
    Visuwesh> Something like,

    Visuwesh>     (make-process :name NAME :command COMMAND :query :yes)

Only because :yes is non-nil. Also the docstring says:

    :noquery BOOL -- When exiting Emacs, query the user if BOOL is nil and
    the process is running.  If BOOL is not given, query before exiting.

which implies you can say

    (make-process :name "foo" :noquery :stop nil)

which is not true, since :noquery is checked with `plist-get'.

Whilst weʼre at it, this is inaccurate as well:

    :stop BOOL -- BOOL must be nil.  The `:stop' key is ignored otherwise
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

since the following will error:

    (make-process :name "foo" :command "ls" :stop t)


Robert
-- 





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08 13:18                 ` Robert Pluim
@ 2023-08-08 16:38                   ` Mattias Engdegård
  2023-08-08 17:14                     ` Helmut Eller
  2023-08-08 17:43                     ` Eli Zaretskii
  0 siblings, 2 replies; 21+ messages in thread
From: Mattias Engdegård @ 2023-08-08 16:38 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Eli Zaretskii, 65030, eller.helmut, monnier

Pushed to master: the warning patch (thank you!) and a modest :name error improvement. This should make things a lot better than they used to be.

Are we done for now?






^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08 16:38                   ` Mattias Engdegård
@ 2023-08-08 17:14                     ` Helmut Eller
  2023-08-08 20:15                       ` Mattias Engdegård
  2023-08-08 17:43                     ` Eli Zaretskii
  1 sibling, 1 reply; 21+ messages in thread
From: Helmut Eller @ 2023-08-08 17:14 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: Robert Pluim, 65030, Eli Zaretskii, monnier

On Tue, Aug 08 2023, Mattias Engdegård wrote:

> Pushed to master: the warning patch (thank you!) and a modest :name
> error improvement. This should make things a lot better than they used
> to be.

Thank you.

> Are we done for now?

Just a nitpick: (make-process) with 0 arguments doesn't signal a runtime
error.  The

  if (nargs == 0)
    return Qnil;

is now probably counterproductive.

Helmut





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08 16:38                   ` Mattias Engdegård
  2023-08-08 17:14                     ` Helmut Eller
@ 2023-08-08 17:43                     ` Eli Zaretskii
  2023-08-08 17:51                       ` Mattias Engdegård
  2023-08-08 17:54                       ` Stefan Kangas
  1 sibling, 2 replies; 21+ messages in thread
From: Eli Zaretskii @ 2023-08-08 17:43 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: rpluim, 65030, eller.helmut, monnier

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Tue, 8 Aug 2023 18:38:03 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, monnier@iro.umontreal.ca,
>         eller.helmut@gmail.com, 65030@debbugs.gnu.org
> 
> Pushed to master: the warning patch (thank you!) and a modest :name error improvement. This should make things a lot better than they used to be.
> 
> Are we done for now?

I wish: the build is now broken:

  Finding pointers to doc strings...
  Finding pointers to doc strings...done
  Dumping under the name bootstrap-emacs.pdmp
  Dumping fingerprint: 7776ea71b2c2039613aeb93d6ca789f55809907a652a1c1d86e6cbdb881
  5025a
  Dump complete
  Byte counts: header=100 hot=16245400 discardable=136576 cold=11545712
  Reloc counts: hot=1160567 discardable=5641
  ANCIENT=yes make -C ../lisp compile-first EMACS="../src/bootstrap-emacs.exe"
  make[3]: Entering directory `/d/gnu/git/emacs/trunk/lisp'
    ELC      emacs-lisp/macroexp.elc
    ELC      emacs-lisp/cconv.elc
    ELC      emacs-lisp/byte-opt.elc
    ELC      emacs-lisp/bytecomp.elc
    ELC      emacs-lisp/loaddefs-gen.elc
    ELC      emacs-lisp/radix-tree.elc

  In toplevel form:
  emacs-lisp/loaddefs-gen.el:42:2: Error: Eager macro-expansion failure: (void-variable name)
  Makefile:333: recipe for target `emacs-lisp/loaddefs-gen.elc' failed
  make[3]: *** [emacs-lisp/loaddefs-gen.elc] Error 1
  make[3]: *** Waiting for unfinished jobs....
  make[3]: Leaving directory `/d/gnu/git/emacs/trunk/lisp'
  Makefile:1011: recipe for target `bootstrap-emacs.pdmp' failed
  make[2]: *** [bootstrap-emacs.pdmp] Error 2
  make[2]: Leaving directory `/d/gnu/git/emacs/trunk/src'
  Makefile:554: recipe for target `src' failed
  make[1]: *** [src] Error 2

And this is _after_ deleting emacs, bootstrap-emacs, and temacs, and
deleting all the *.elc files.  So no stale *.elc files are involved, I
think.

(And no, I don't want to bootstrap, and I don't think it will fix the
problem, given the above.)

Line 42 of loaddefs-gen.el is where it require's lisp-mnt, FWIW.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08 17:43                     ` Eli Zaretskii
@ 2023-08-08 17:51                       ` Mattias Engdegård
  2023-08-08 17:54                       ` Stefan Kangas
  1 sibling, 0 replies; 21+ messages in thread
From: Mattias Engdegård @ 2023-08-08 17:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Robert Pluim, 65030, eller.helmut, Stefan Monnier

8 aug. 2023 kl. 19.43 skrev Eli Zaretskii <eliz@gnu.org>:

> I wish: the build is now broken:

So very sorry, I'll deal with it right away (and revert if a timely remedy isn't possible).






^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08 17:43                     ` Eli Zaretskii
  2023-08-08 17:51                       ` Mattias Engdegård
@ 2023-08-08 17:54                       ` Stefan Kangas
  2023-08-08 18:28                         ` Eli Zaretskii
  1 sibling, 1 reply; 21+ messages in thread
From: Stefan Kangas @ 2023-08-08 17:54 UTC (permalink / raw)
  To: Eli Zaretskii
  Cc: Mattias Engdegård, rpluim, 65030, eller.helmut, monnier

> Line 42 of loaddefs-gen.el is where it require's lisp-mnt, FWIW.

That's my bad, should be fixed now.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08 17:54                       ` Stefan Kangas
@ 2023-08-08 18:28                         ` Eli Zaretskii
  0 siblings, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2023-08-08 18:28 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: mattiase, rpluim, 65030, eller.helmut, monnier

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Tue, 8 Aug 2023 19:54:29 +0200
> Cc: Mattias Engdegård <mattiase@acm.org>, rpluim@gmail.com, 
> 	65030@debbugs.gnu.org, eller.helmut@gmail.com, monnier@iro.umontreal.ca
> 
> > Line 42 of loaddefs-gen.el is where it require's lisp-mnt, FWIW.
> 
> That's my bad, should be fixed now.

Thanks.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#65030: 30.0.50; Check keyword args of make-process
  2023-08-08 17:14                     ` Helmut Eller
@ 2023-08-08 20:15                       ` Mattias Engdegård
  0 siblings, 0 replies; 21+ messages in thread
From: Mattias Engdegård @ 2023-08-08 20:15 UTC (permalink / raw)
  To: Helmut Eller; +Cc: Robert Pluim, 65030, Eli Zaretskii, monnier

8 aug. 2023 kl. 19.14 skrev Helmut Eller <eller.helmut@gmail.com>:

> Just a nitpick: (make-process) with 0 arguments doesn't signal a runtime
> error.  The
> 
>  if (nargs == 0)
>    return Qnil;
> 
> is now probably counterproductive.

Always was, and undocumented.






^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2023-08-08 20:15 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-03  6:47 bug#65030: 30.0.50; Check keyword args of make-process Helmut Eller
2023-08-05  9:21 ` Eli Zaretskii
2023-08-05 23:07   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-06  4:58     ` Eli Zaretskii
2023-08-06  8:49       ` Mattias Engdegård
2023-08-08  8:52         ` Robert Pluim
2023-08-08  9:16           ` Mattias Engdegård
2023-08-08  9:27             ` Robert Pluim
2023-08-08  9:42               ` Mattias Engdegård
2023-08-08 12:16             ` Eli Zaretskii
2023-08-08 13:05               ` Mattias Engdegård
2023-08-08 13:18                 ` Robert Pluim
2023-08-08 16:38                   ` Mattias Engdegård
2023-08-08 17:14                     ` Helmut Eller
2023-08-08 20:15                       ` Mattias Engdegård
2023-08-08 17:43                     ` Eli Zaretskii
2023-08-08 17:51                       ` Mattias Engdegård
2023-08-08 17:54                       ` Stefan Kangas
2023-08-08 18:28                         ` Eli Zaretskii
2023-08-08 13:37           ` Visuwesh
2023-08-08 13:52             ` Robert Pluim

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).