unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#28587: 26.0.60; Don't write object name strings in object-write method
@ 2017-09-24 21:11 Eric Abrahamsen
  2017-10-22  3:29 ` Stefan Monnier
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Abrahamsen @ 2017-09-24 21:11 UTC (permalink / raw)
  To: 28587

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


If we're going to ignore them, we might as well not write them to begin
with.

Are there any other places that expect the presence of this name string?


In GNU Emacs 26.0.60 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.21)
 of 2017-09-22 built on clem
Repository revision: 908af46abdb2c19ff3c72543e4fadf8e0ed82d2b

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-print-object-name-string-in-object-write-meth.patch --]
[-- Type: text/x-diff, Size: 1843 bytes --]

From 77a270ec916854ecb7e9ccaca444fe8ccbeab843 Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Sun, 24 Sep 2017 14:07:26 -0700
Subject: [PATCH] Do not print object name string in object-write method

* lisp/emacs-lisp/eieio.el (object-write): The object name string is
  obsolete, might as well save some bytes.
* lisp/emacs-lisp/eieio-base.el (eieio-persistent-convert-list-to-object):
  Check for presence of object name string, and ignore.
---
 lisp/emacs-lisp/eieio-base.el | 7 +++++--
 lisp/emacs-lisp/eieio.el      | 2 --
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/lisp/emacs-lisp/eieio-base.el b/lisp/emacs-lisp/eieio-base.el
index 6b39b4f262..4c6fbae12c 100644
--- a/lisp/emacs-lisp/eieio-base.el
+++ b/lisp/emacs-lisp/eieio-base.el
@@ -255,8 +255,11 @@ eieio-persistent-convert-list-to-object
 Note: This function recurses when a slot of :type of some object is
 identified, and needing more object creation."
   (let* ((objclass (nth 0 inputlist))
-	 ;; (objname (nth 1 inputlist))
-	 (slots (nthcdr 2 inputlist))
+	 ;; Earlier versions of `object-write' added a string name for
+	 ;; the object, now obsolete.
+	 (slots (nthcdr
+                 (if (stringp (nth 1 inputlist)) 2 1)
+                 inputlist))
 	 (createslots nil)
 	 (class
 	  (progn
diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index 75f1097acf..448d5e6fe2 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -874,8 +874,6 @@ eieio-print-depth
     (princ (make-string (* eieio-print-depth 2) ? ))
     (princ "(")
     (princ (symbol-name (eieio--class-constructor (eieio-object-class this))))
-    (princ " ")
-    (prin1 (eieio-object-name-string this))
     (princ "\n")
     ;; Loop over all the public slots
     (let ((slots (eieio--class-slots cv))
-- 
2.14.1


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

* bug#28587: 26.0.60; Don't write object name strings in object-write method
  2017-09-24 21:11 bug#28587: 26.0.60; Don't write object name strings in object-write method Eric Abrahamsen
@ 2017-10-22  3:29 ` Stefan Monnier
  2017-11-08 20:01   ` Eric Abrahamsen
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2017-10-22  3:29 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 28587

> If we're going to ignore them, we might as well not write them to begin
> with.

Agreed.

> --- a/lisp/emacs-lisp/eieio-base.el
> +++ b/lisp/emacs-lisp/eieio-base.el
> @@ -255,8 +255,11 @@ eieio-persistent-convert-list-to-object
>  Note: This function recurses when a slot of :type of some object is
>  identified, and needing more object creation."
>    (let* ((objclass (nth 0 inputlist))
> -	 ;; (objname (nth 1 inputlist))
> -	 (slots (nthcdr 2 inputlist))
> +	 ;; Earlier versions of `object-write' added a string name for
> +	 ;; the object, now obsolete.
> +	 (slots (nthcdr
> +                 (if (stringp (nth 1 inputlist)) 2 1)
> +                 inputlist))
>  	 (createslots nil)
>  	 (class
>  	  (progn

This looks good, feel free to install it into `master'.

> diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
> index 75f1097acf..448d5e6fe2 100644
> --- a/lisp/emacs-lisp/eieio.el
> +++ b/lisp/emacs-lisp/eieio.el
> @@ -874,8 +874,6 @@ eieio-print-depth
>      (princ (make-string (* eieio-print-depth 2) ? ))
>      (princ "(")
>      (princ (symbol-name (eieio--class-constructor (eieio-object-class this))))
> -    (princ " ")
> -    (prin1 (eieio-object-name-string this))
>      (princ "\n")
>      ;; Loop over all the public slots
>      (let ((slots (eieio--class-slots cv))

This is more problematic since data generated with this hunk will be
incompatible with an Emacs which doesn't have the other hunk applied.
So I think this should be conditional on a defcustom and by default this
defcustom should cause the code to still behave as before.


        Stefan





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

* bug#28587: 26.0.60; Don't write object name strings in object-write method
  2017-10-22  3:29 ` Stefan Monnier
@ 2017-11-08 20:01   ` Eric Abrahamsen
  2017-11-08 20:31     ` Stefan Monnier
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Abrahamsen @ 2017-11-08 20:01 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 28587

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


On 10/21/17 23:29 PM, Stefan Monnier wrote:

[...]

> This is more problematic since data generated with this hunk will be
> incompatible with an Emacs which doesn't have the other hunk applied.
> So I think this should be conditional on a defcustom and by default this
> defcustom should cause the code to still behave as before.

So where we're at now is, the previous chunk has gone into both master
and emacs-26. The next patch (below) would only go into master, so as to
be more conservative about what's emitted. Everything defaults to
current behavior.

This patch does two things: permits omission of the object name strings,
and permits shutting off indentation (which helps a lot for shrinking
the file size). In a previous version I had `eieio-print-depth' do
double duty as a boolean and an integer; this version has a separate
defvar for controlling whether indentation is printed at all.
`eieio-print-depth' still gets incremented; someone might find that
useful.

Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Provide-more-control-over-writing-of-objects-in-obje.patch --]
[-- Type: text/x-diff, Size: 4118 bytes --]

From b75d0469dc986a73cbac6351537c30e535ce2890 Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Wed, 8 Nov 2017 11:58:31 -0800
Subject: [PATCH] Provide more control over writing of objects in object-write

* lisp/emacs-lisp/eieio.el (eieio-print-indentation,
  eieio-print-object-name): New variables controlling whether an
  object name is printed for each object, and whether an object's
  contents are indented or not. Object names are obsoleted; omitting
  indentation reduces the size of persistence files.
---
 lisp/emacs-lisp/eieio.el | 38 +++++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index ca91c5a871..e07392d8c9 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -847,7 +847,17 @@ 'constructor
   (princ (object-print object) stream))
 
 (defvar eieio-print-depth 0
-  "When printing, keep track of the current indentation depth.")
+  "The current indentation depth while printing.
+Ignored if `eieio-print-indentation' is nil.")
+
+(defvar eieio-print-indentation t
+  "When non-nil, indent contents of printed objects.")
+
+(defvar eieio-print-object-name t
+  "When non-nil write the object name in `object-write'.
+Does not affect objects subclassing `eieio-named'.  Writing
+object names is deprecated as of Emacs 24.4; consider setting
+this to nil.")
 
 (cl-defgeneric object-write (this &optional comment)
   "Write out object THIS to the current stream.
@@ -860,9 +870,10 @@ eieio-print-depth
   If optional COMMENT is non-nil, include comments when outputting
 this object."
   (when comment
-    (princ ";; Object ")
-    (princ (eieio-object-name-string this))
-    (princ "\n")
+    (when eieio-print-object-name
+      (princ ";; Object ")
+      (princ (eieio-object-name-string this))
+      (princ "\n"))
     (princ comment)
     (princ "\n"))
   (let* ((cl (eieio-object-class this))
@@ -871,11 +882,13 @@ eieio-print-depth
     ;; It should look like this:
     ;; (<constructor> <name> <slot> <slot> ... )
     ;; Each slot's slot is writen using its :writer.
-    (princ (make-string (* eieio-print-depth 2) ? ))
+    (when eieio-print-indentation
+      (princ (make-string (* eieio-print-depth 2) ? )))
     (princ "(")
     (princ (symbol-name (eieio--class-constructor (eieio-object-class this))))
-    (princ " ")
-    (prin1 (eieio-object-name-string this))
+    (when eieio-print-object-name
+      (princ " ")
+      (prin1 (eieio-object-name-string this)))
     (princ "\n")
     ;; Loop over all the public slots
     (let ((slots (eieio--class-slots cv))
@@ -889,7 +902,8 @@ eieio-print-depth
               (unless (or (not i) (equal v (cl--slot-descriptor-initform slot)))
                 (unless (bolp)
                   (princ "\n"))
-                (princ (make-string (* eieio-print-depth 2) ? ))
+                (when eieio-print-indentation
+                  (princ (make-string (* eieio-print-depth 2) ? )))
                 (princ (symbol-name i))
                 (if (alist-get :printer (cl--slot-descriptor-props slot))
                     ;; Use our public printer
@@ -904,7 +918,7 @@ eieio-print-depth
                              "\n" " "))
                   (eieio-override-prin1 v))))))))
     (princ ")")
-    (when (= eieio-print-depth 0)
+    (when (zerop eieio-print-depth)
       (princ "\n"))))
 
 (defun eieio-override-prin1 (thing)
@@ -923,14 +937,16 @@ eieio-list-prin1
       (progn
 	(princ "'")
 	(prin1 list))
-    (princ (make-string (* eieio-print-depth 2) ? ))
+    (when eieio-print-indentation
+      (princ (make-string (* eieio-print-depth 2) ? )))
     (princ "(list")
     (let ((eieio-print-depth (1+ eieio-print-depth)))
       (while list
 	(princ "\n")
 	(if (eieio-object-p (car list))
 	    (object-write (car list))
-	  (princ (make-string (* eieio-print-depth 2) ? ))
+          (when eieio-print-indentation
+	   (princ (make-string (* eieio-print-depth) ? )))
 	  (eieio-override-prin1 (car list)))
 	(setq list (cdr list))))
     (princ ")")))
-- 
2.15.0


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

* bug#28587: 26.0.60; Don't write object name strings in object-write method
  2017-11-08 20:01   ` Eric Abrahamsen
@ 2017-11-08 20:31     ` Stefan Monnier
  2017-11-08 21:01       ` Eric Abrahamsen
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2017-11-08 20:31 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 28587

> So where we're at now is, the previous chunk has gone into both master
> and emacs-26.

Yay!

> The next patch (below) would only go into master, so as to
> be more conservative about what's emitted. Everything defaults to
> current behavior.

Looks good, except for:

> +  "When non-nil write the object name in `object-write'.
> +Does not affect objects subclassing `eieio-named'.  Writing
> +object names is deprecated as of Emacs 24.4; consider setting
> +this to nil.")

AFAIK names are deprecated in calls to EIEIO constructors since
Emacs-25 (rather 24.4), and it's only since Emacs-26 that they are
optional in "eieio-presistent" written representations.


        Stefan





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

* bug#28587: 26.0.60; Don't write object name strings in object-write method
  2017-11-08 20:31     ` Stefan Monnier
@ 2017-11-08 21:01       ` Eric Abrahamsen
  2017-11-08 22:05         ` Stefan Monnier
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Abrahamsen @ 2017-11-08 21:01 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 28587


On 11/08/17 15:31 PM, Stefan Monnier wrote:
>> So where we're at now is, the previous chunk has gone into both master
>> and emacs-26.
>
> Yay!
>
>> The next patch (below) would only go into master, so as to
>> be more conservative about what's emitted. Everything defaults to
>> current behavior.
>
> Looks good, except for:
>
>> +  "When non-nil write the object name in `object-write'.
>> +Does not affect objects subclassing `eieio-named'.  Writing
>> +object names is deprecated as of Emacs 24.4; consider setting
>> +this to nil.")
>
> AFAIK names are deprecated in calls to EIEIO constructors since
> Emacs-25 (rather 24.4), and it's only since Emacs-26 that they are
> optional in "eieio-presistent" written representations.

Okay I see, I got a little confused there. I checked the emacs 25 branch
and it ignores an object name string (emacs 24 requires it), so maybe
the only important thing is to say:

"Note that prior to Emacs version 25 the object name is required."

How's that?





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

* bug#28587: 26.0.60; Don't write object name strings in object-write method
  2017-11-08 21:01       ` Eric Abrahamsen
@ 2017-11-08 22:05         ` Stefan Monnier
  2017-11-08 22:08           ` Eric Abrahamsen
                             ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Stefan Monnier @ 2017-11-08 22:05 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 28587

>> AFAIK names are deprecated in calls to EIEIO constructors since
>> Emacs-25 (rather 24.4), and it's only since Emacs-26 that they are
>> optional in "eieio-presistent" written representations.
> Okay I see, I got a little confused there. I checked the emacs 25 branch
> and it ignores an object name string (emacs 24 requires it), so maybe
> the only important thing is to say:
> "Note that prior to Emacs version 25 the object name is required."

I think the only important part is that Emacs<26 requires it (tho only
Emacs<25 makes actual use of it, and even then, this use is mostly
irrelevant, except maybe in corner cases in the course of debugging).


        Stefan





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

* bug#28587: 26.0.60; Don't write object name strings in object-write method
  2017-11-08 22:05         ` Stefan Monnier
@ 2017-11-08 22:08           ` Eric Abrahamsen
  2017-11-11  1:39           ` Eric Abrahamsen
  2017-11-22 22:30           ` Eric Abrahamsen
  2 siblings, 0 replies; 9+ messages in thread
From: Eric Abrahamsen @ 2017-11-08 22:08 UTC (permalink / raw)
  To: 28587

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>> AFAIK names are deprecated in calls to EIEIO constructors since
>>> Emacs-25 (rather 24.4), and it's only since Emacs-26 that they are
>>> optional in "eieio-presistent" written representations.
>> Okay I see, I got a little confused there. I checked the emacs 25 branch
>> and it ignores an object name string (emacs 24 requires it), so maybe
>> the only important thing is to say:
>> "Note that prior to Emacs version 25 the object name is required."
>
> I think the only important part is that Emacs<26 requires it (tho only
> Emacs<25 makes actual use of it, and even then, this use is mostly
> irrelevant, except maybe in corner cases in the course of debugging).

Right! Bah, I keep getting this wrong. Emacs<26 will fail without it.
I'll make that note, and leave it at that.






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

* bug#28587: 26.0.60; Don't write object name strings in object-write method
  2017-11-08 22:05         ` Stefan Monnier
  2017-11-08 22:08           ` Eric Abrahamsen
@ 2017-11-11  1:39           ` Eric Abrahamsen
  2017-11-22 22:30           ` Eric Abrahamsen
  2 siblings, 0 replies; 9+ messages in thread
From: Eric Abrahamsen @ 2017-11-11  1:39 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 28587


On 11/08/17 17:05 PM, Stefan Monnier wrote:
>>> AFAIK names are deprecated in calls to EIEIO constructors since
>>> Emacs-25 (rather 24.4), and it's only since Emacs-26 that they are
>>> optional in "eieio-presistent" written representations.
>> Okay I see, I got a little confused there. I checked the emacs 25 branch
>> and it ignores an object name string (emacs 24 requires it), so maybe
>> the only important thing is to say:
>> "Note that prior to Emacs version 25 the object name is required."
>
> I think the only important part is that Emacs<26 requires it (tho only
> Emacs<25 makes actual use of it, and even then, this use is mostly
> irrelevant, except maybe in corner cases in the course of debugging).

Okay, there it goes, with a modified docstring. Tests are passing here;
if no one complains in the next couple days, I'll close this bug report.

Eric





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

* bug#28587: 26.0.60; Don't write object name strings in object-write method
  2017-11-08 22:05         ` Stefan Monnier
  2017-11-08 22:08           ` Eric Abrahamsen
  2017-11-11  1:39           ` Eric Abrahamsen
@ 2017-11-22 22:30           ` Eric Abrahamsen
  2 siblings, 0 replies; 9+ messages in thread
From: Eric Abrahamsen @ 2017-11-22 22:30 UTC (permalink / raw)
  To: 28587-done






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

end of thread, other threads:[~2017-11-22 22:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-24 21:11 bug#28587: 26.0.60; Don't write object name strings in object-write method Eric Abrahamsen
2017-10-22  3:29 ` Stefan Monnier
2017-11-08 20:01   ` Eric Abrahamsen
2017-11-08 20:31     ` Stefan Monnier
2017-11-08 21:01       ` Eric Abrahamsen
2017-11-08 22:05         ` Stefan Monnier
2017-11-08 22:08           ` Eric Abrahamsen
2017-11-11  1:39           ` Eric Abrahamsen
2017-11-22 22:30           ` Eric Abrahamsen

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