bug#66928: [PATCH 2/2] Update names to match the docstring

bug#66928: [PATCH 2/2] Update names to match the docstring

[Jeremy Bryant via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Tags: patch

Hello

I noticed that for the function below the docstring doesn't match the
arg names.  I've chosen to keep the docstring on the basis that it may
be what has been the most visible for a time, and update the names in
the code.

In general, is it TRT?  Do we have a convention to keep names or the
docstring.  Please let me know if this patch is useful in its current
form or a change is needed.

thanks
Jeremy 



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Update-names-to-match-the-docstring.patch --]
[-- Type: text/patch, Size: 1201 bytes --]

From 927b8ef441c1367ba1fcf08172f3eb6b739eddab Mon Sep 17 00:00:00 2001
From: Jeremy Bryant <jb@jeremybryant.net>
Date: Sat, 4 Nov 2023 12:38:29 +0000
Subject: [PATCH 2/2] Update names to match the docstring

* lisp/emacs-lisp/cl-extra.el (cl-remprop):
Change sym to symbol, tag to propname
---
 lisp/emacs-lisp/cl-extra.el | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lisp/emacs-lisp/cl-extra.el b/lisp/emacs-lisp/cl-extra.el
index 15be51bd651..909a1aacc47 100644
--- a/lisp/emacs-lisp/cl-extra.el
+++ b/lisp/emacs-lisp/cl-extra.el
@@ -635,13 +635,13 @@ cl--do-remf
     (and (cdr p) (progn (setcdr p (cdr (cdr (cdr p)))) t))))
 
 ;;;###autoload
-(defun cl-remprop (sym tag)
+(defun cl-remprop (symbol propname)
   "Remove from SYMBOL's plist the property PROPNAME and its value.
 \n(fn SYMBOL PROPNAME)"
-  (let ((plist (symbol-plist sym)))
-    (if (and plist (eq tag (car plist)))
-	(progn (setplist sym (cdr (cdr plist))) t)
-      (cl--do-remf plist tag))))
+  (let ((plist (symbol-plist symbol)))
+    (if (and plist (eq propname (car plist)))
+	(progn (setplist symbol (cdr (cdr plist))) t)
+      (cl--do-remf plist propname))))
 
 ;;; Streams.
 
-- 
2.40.1

bug#66928: [PATCH 2/2] Update names to match the docstring

[Eli Zaretskii]

> Date: Sat, 04 Nov 2023 12:49:27 +0000
> From:  Jeremy Bryant via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> I noticed that for the function below the docstring doesn't match the
> arg names.

That's why we have the "(fn SYMBOL PROPNAME)" part there.

bug#66928: [PATCH 2/2] Update names to match the docstring

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I noticed that for the function below the docstring doesn't match the
> arg names.

I agree it's an odd choice (requiring the use of `\n(fn SYMBOL PROPNAME)`),
but it's not a big problem since the (fn SYMBOL PROPNAME) makes sure
that the right names are presented in `C-h o`.

If you want to install a patch like you suggest, then please remove the
(fn SYMBOL PROPNAME) since it makes it redundant.
It'd be considered as a cosmetic patch.

One more thing: the first line of the commit should be a bit more
specific, e.g. it could start with "(cl-remprop):"


        Stefan

bug#66928: [PATCH 2/2] Update names to match the docstring

[Jeremy Bryant via Bug reports for GNU Emacs, the Swiss army knife of text editors]

thank you, I will work on this.

One question, the elisp manual mentions that the \(fn ARGLIST) facility
is particularly useful for macros.

Why would we use this for defuns?  What is TRT in general?

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I noticed that for the function below the docstring doesn't match the
>> arg names.
>
> I agree it's an odd choice (requiring the use of `\n(fn SYMBOL PROPNAME)`),
> but it's not a big problem since the (fn SYMBOL PROPNAME) makes sure
> that the right names are presented in `C-h o`.
>
> If you want to install a patch like you suggest, then please remove the
> (fn SYMBOL PROPNAME) since it makes it redundant.
> It'd be considered as a cosmetic patch.
>
> One more thing: the first line of the commit should be a bit more
> specific, e.g. it could start with "(cl-remprop):"
>
>
>         Stefan

bug#66928: [PATCH 2/2] Update names to match the docstring

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> One question, the elisp manual mentions that the \(fn ARGLIST) facility
> is particularly useful for macros.
>
> Why would we use this for defuns?

We use it for some defuns where ELisp's arglist functionality is not
refined enough to express the intention of the programmer.
For example, the "real" arglist may be

    name args &optional docstring &rest body

but the intended arglist is

    name args [docstring] &rest body

i.e. if `docstring` is not a string it's taken as the first instruction
of `body`.


        Stefan

bug#66928: [PATCH 2/2] Update names to match the docstring

[Jeremy Bryant via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Interesting, thank you 
Perhaps it would be of interest to add some text to the elisp manual in
(elisp) Function Documentation

Are there any conventions for manual additions?
in particular to keep the number of printed pages limited is there a way
to say 'don't print this node'?

thanks
Jeremy


Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> One question, the elisp manual mentions that the \(fn ARGLIST) facility
>> is particularly useful for macros.
>>
>> Why would we use this for defuns?
>
> We use it for some defuns where ELisp's arglist functionality is not
> refined enough to express the intention of the programmer.
> For example, the "real" arglist may be
>
>     name args &optional docstring &rest body
>
> but the intended arglist is
>
>     name args [docstring] &rest body
>
> i.e. if `docstring` is not a string it's taken as the first instruction
> of `body`.
>
>
>         Stefan

bug#66928: [PATCH 2/2] Update names to match the docstring

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Perhaps it would be of interest to add some text to the elisp manual
> in (elisp) Function Documentation

There's already a note about the (fn ...) convention, but if you have an
idea of something that would have been helpful to, please send us your
suggestion :-)

> Are there any conventions for manual additions?
> in particular to keep the number of printed pages limited is there a way
> to say 'don't print this node'?

I think there were ways to keep some parts out of the printed manual,
but I'm not sure this is relevant to the ELisp manual any more: is it
still printed?

The FSF shop still carries the Emacs manual and the ELisp intro, but
I can't see the ELisp manual in there.


        Stefan

bug#66928: [PATCH 2/2] Update names to match the docstring

[Eli Zaretskii]

> Cc: 66928@debbugs.gnu.org
> Date: Sun, 12 Nov 2023 16:02:00 -0500
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> > Are there any conventions for manual additions?
> > in particular to keep the number of printed pages limited is there a way
> > to say 'don't print this node'?
> 
> I think there were ways to keep some parts out of the printed manual,

Yes, use @ifnottex..@end ifnottex.

> but I'm not sure this is relevant to the ELisp manual any more: is it
> still printed?
> 
> The FSF shop still carries the Emacs manual and the ELisp intro, but
> I can't see the ELisp manual in there.

Richard?

bug#66928: [PATCH 2/2] Update names to match the docstring

[Jeremy Bryant via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Attached is a proposed patch to install, with the two changes requested.

This is cosmetic patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-lisp-emacs-lisp-cl-extra.el-cl-remprop-Update-names-.patch --]
[-- Type: text/x-diff, Size: 1232 bytes --]

From 4036426119ca939070f5353671eff9f22ed66598 Mon Sep 17 00:00:00 2001
From: Jeremy Bryant <jb@jeremybryant.net>
Date: Sat, 4 Nov 2023 23:29:10 +0000
Subject: [PATCH 2/2] * lisp/emacs-lisp/cl-extra.el (cl-remprop): Update names
 to match docstring

---
 lisp/emacs-lisp/cl-extra.el | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lisp/emacs-lisp/cl-extra.el b/lisp/emacs-lisp/cl-extra.el
index de5eb9c2d92..e41bf1cdb0b 100644
--- a/lisp/emacs-lisp/cl-extra.el
+++ b/lisp/emacs-lisp/cl-extra.el
@@ -633,13 +633,12 @@ cl--do-remf
     (and (cdr p) (progn (setcdr p (cdr (cdr (cdr p)))) t))))
 
 ;;;###autoload
-(defun cl-remprop (sym tag)
-  "Remove from SYMBOL's plist the property PROPNAME and its value.
-\n(fn SYMBOL PROPNAME)"
-  (let ((plist (symbol-plist sym)))
-    (if (and plist (eq tag (car plist)))
-	(progn (setplist sym (cdr (cdr plist))) t)
-      (cl--do-remf plist tag))))
+(defun cl-remprop (symbol propname)
+  "Remove from SYMBOL's plist the property PROPNAME and its value."
+  (let ((plist (symbol-plist symbol)))
+    (if (and plist (eq propname (car plist)))
+	(progn (setplist symbol (cdr (cdr plist))) t)
+      (cl--do-remf plist propname))))
 
 ;;; Streams.
 
-- 
2.40.1


[-- Attachment #3: Type: text/plain, Size: 654 bytes --]


Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I noticed that for the function below the docstring doesn't match the
>> arg names.
>
> I agree it's an odd choice (requiring the use of `\n(fn SYMBOL PROPNAME)`),
> but it's not a big problem since the (fn SYMBOL PROPNAME) makes sure
> that the right names are presented in `C-h o`.
>
> If you want to install a patch like you suggest, then please remove the
> (fn SYMBOL PROPNAME) since it makes it redundant.
> It'd be considered as a cosmetic patch.
>
> One more thing: the first line of the commit should be a bit more
> specific, e.g. it could start with "(cl-remprop):"
>
>
>         Stefan

bug#66928: [PATCH 2/2] Update names to match the docstring

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I noticed that for the function below the docstring doesn't match the
> arg names.  I've chosen to keep the docstring on the basis that it may

This is now fixed, closing,


        Stefan