unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#43621: 28.0.50; Clean up EIEIO
@ 2020-09-25 19:49 Michael Heerdegen
  2020-09-25 20:53 ` Stefan Monnier
  0 siblings, 1 reply; 2+ messages in thread
From: Michael Heerdegen @ 2020-09-25 19:49 UTC (permalink / raw)
  To: 43621; +Cc: Eric Abrahamsen, Stefan Monnier

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


Hi,

I'm currently seriously learning and using EIEIO for the first time.
Here are my findings (so far):

(1) Manual

(1a) (info "(eieio) Introduction")

| Method dispatch
|      EIEO does not support method dispatch for built-in types and
|      multiple arguments types.  In other words, method dispatch only
|      looks at the first argument, and this one must be an EIEIO type.

What's said about dispatching only looking at the first argument is not
true anymore, right?  And

| ‘:around’ method tag
|      This CLOS method tag is non-functional.

Also not correct any more, right?

That whole paragraph should be overhauled and adapted to the obviously
new state of the code I guess.


(1b) Manual (info "(eieio) Building Classes")

| Whenever defclass is used to create a new class, a predicate is created
| for it, named ‘CLASS-NAME-p’:
| 
|  -- Function: CLASS-NAME-p object
|      Return non-‘nil’ if and only if OBJECT is of the class CLASS-NAME.

I found it surprising that the predicate fails for instances of classes
that inherit from CLASS-NAME.  If this is intended, I guess this should
be mentioned...?


(2) `info-lookup' doesn't work for eieio functions.  I guess it should?


(3) Doc of `defclass':

|  :writer     - A function symbol which will `write' an object's slot.

I find "`write'" confusing: there is no function `write' (so why quote
it like this?), and it should better be "set" anyway, since in other
places (in eieio!) "write" is used as synonym for "print".


(4) Whitespace in generated docs

In quite a few places generated docs have newlines missing or misplaced
space characters and such things.  Stuff like this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: eieio-whitespace-example.patch --]
[-- Type: text/x-diff, Size: 1100 bytes --]

diff --git a/lisp/emacs-lisp/cl-extra.el b/lisp/emacs-lisp/cl-extra.el
index 5bf74792c0..5701fcf026 100644
--- a/lisp/emacs-lisp/cl-extra.el
+++ b/lisp/emacs-lisp/cl-extra.el
@@ -800,8 +800,9 @@ cl--describe-class

     ;; Type's documentation.
     (let ((doc (cl--class-docstring class)))
-      (when doc
-        (insert "\n" doc "\n\n")))
+      (if doc
+          (insert "\n" doc "\n\n")
+        (insert "\n")))

     ;; Describe all the slots in this class.
     (cl--describe-class-slots class)
diff --git a/lisp/emacs-lisp/eieio-opt.el b/lisp/emacs-lisp/eieio-opt.el
index 59af7e12d2..b9bab42470 100644
--- a/lisp/emacs-lisp/eieio-opt.el
+++ b/lisp/emacs-lisp/eieio-opt.el
@@ -136,9 +136,9 @@ eieio-help-constructor
 	  (def (symbol-function ctr)))
       (goto-char (point-min))
       (prin1 ctr)
-      (insert (format " is an %s object constructor function"
+      (insert (format " is an%s object constructor function"
 		      (if (autoloadp def)
-			  "autoloaded"
+			  " autoloaded"
 			"")))
       (when (and (autoloadp def)
 		 (null location))

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


Some love is needed here.


Ok, that's what I found so far.

TIA, Michael.




In GNU Emacs 28.0.50 (build 49, x86_64-pc-linux-gnu, GTK+ Version 3.24.23, cairo version 1.16.0)
 of 2020-09-25 built on drachen
Repository revision: d10ef50c9b7c8eeb04194edc8d729dcc5a41b8f6
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Debian GNU/Linux bullseye/sid


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

* bug#43621: 28.0.50; Clean up EIEIO
  2020-09-25 19:49 bug#43621: 28.0.50; Clean up EIEIO Michael Heerdegen
@ 2020-09-25 20:53 ` Stefan Monnier
  0 siblings, 0 replies; 2+ messages in thread
From: Stefan Monnier @ 2020-09-25 20:53 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: eric, 43621

> I'm currently seriously learning and using EIEIO for the first time.
> Here are my findings (so far):
>
> (1) Manual
>
> (1a) (info "(eieio) Introduction")
>
> | Method dispatch
> |      EIEO does not support method dispatch for built-in types and
> |      multiple arguments types.  In other words, method dispatch only
> |      looks at the first argument, and this one must be an EIEIO type.
>
> What's said about dispatching only looking at the first argument is not
> true anymore, right?  And

Indeed.  EIEIO's method dispatch has been thrown away, and instead
cl-generic.el provides it separately from EIEIO.  IOW nowadays "EIEIO"
refers to the facilities surrounding `defclass` but not method dispatch
any more.

Also: cl-generic *can* dispatch on built-in types (and defstruct types).

> | ‘:around’ method tag
> |      This CLOS method tag is non-functional.
> Also not correct any more, right?
> That whole paragraph should be overhauled and adapted to the obviously
> new state of the code I guess.

And moved out of `eieio.texi`.

> | Whenever defclass is used to create a new class, a predicate is created
> | for it, named ‘CLASS-NAME-p’:
> | 
> |  -- Function: CLASS-NAME-p object
> |      Return non-‘nil’ if and only if OBJECT is of the class CLASS-NAME.
>
> I found it surprising that the predicate fails for instances of classes
> that inherit from CLASS-NAME.  If this is intended, I guess this should
> be mentioned...?

Indeed.  It might also be better to promote `(cl-typep EXP '<CLASS>)`
instead of `<CLASS>-p`, which behaves in the more "normal" way.

> (2) `info-lookup' doesn't work for eieio functions.  I guess it should?

It would be nice to make it work, yes.

> (3) Doc of `defclass':
> |  :writer     - A function symbol which will `write' an object's slot.
> I find "`write'" confusing: there is no function `write' (so why quote
> it like this?), and it should better be "set" anyway, since in other
> places (in eieio!) "write" is used as synonym for "print".

Agreed.  It should also be improved to clarify what arguments the writer
takes (IIUC the writer function will take 2 args (the object and
a value for the slot), so the doc needs to clarify in which order those
args are supposed to come).

> Some love is needed here.

Can you provide that love?


        Stefan






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

end of thread, other threads:[~2020-09-25 20:53 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-25 19:49 bug#43621: 28.0.50; Clean up EIEIO Michael Heerdegen
2020-09-25 20:53 ` Stefan Monnier

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).