unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Applying print-length to (length string) is suboptimal.
@ 2023-09-17 19:57 Alan Mackenzie
  2023-09-18  9:46 ` Alan Mackenzie
  0 siblings, 1 reply; 2+ messages in thread
From: Alan Mackenzie @ 2023-09-17 19:57 UTC (permalink / raw)
  To: emacs-devel

Hello, Emacs.

Also with reference to bug #65680.

In cl-print-object (lisp/emacs-lisp/cl-print.el), the version for
strings, print-length is taken as a bound on the length of the string to
be printed.

Why?

The whole idea of print-length (and print-level) is, as far as I can make
out, to prevent infinite printing when there's a circular list, for
example.  This cannot occur for strings.

This use of print-length is relatively recent, originating in this
commit:

    commit 8a7620955b4d859caecd9a5dc9f2a986baf994fd
    Author: Gemini Lasswell <gazally@runbox.com>
    Date:   Fri Jun 15 10:26:13 2018 -0700

        Add methods for strings to cl-print

print-length is not used in the function prin1 (as contrasted with
cl-prin1).

The problem with using print-length for the max. string length is that it
is not designed for this.  When print-length is, say, 50, almost any list
or vector can be printed in full, going up to several hundred printed
characters.  With a string, the string gets rudely truncated at a mere 50
characters, which is rarely wanted.  This is a problem with the error
message at the top of a backtrace, where cl-print-string-with-limit
limits print-length to a maximum of 50 (See bug #65680).

What do people think of removing this observance of print-length from the
string version of cl-print-object?

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Applying print-length to (length string) is suboptimal.
  2023-09-17 19:57 Applying print-length to (length string) is suboptimal Alan Mackenzie
@ 2023-09-18  9:46 ` Alan Mackenzie
  0 siblings, 0 replies; 2+ messages in thread
From: Alan Mackenzie @ 2023-09-18  9:46 UTC (permalink / raw)
  To: emacs-devel

Hello, Emacs.

On Sun, Sep 17, 2023 at 19:57:07 +0000, Alan Mackenzie wrote:
> Also with reference to bug #65680.

> In cl-print-object (lisp/emacs-lisp/cl-print.el), the version for
> strings, print-length is taken as a bound on the length of the string to
> be printed.

> Why?

> The whole idea of print-length (and print-level) is, as far as I can make
> out, to prevent infinite printing when there's a circular list, for
> example.  This cannot occur for strings.

> This use of print-length is relatively recent, originating in this
> commit:

>     commit 8a7620955b4d859caecd9a5dc9f2a986baf994fd
>     Author: Gemini Lasswell <gazally@runbox.com>
>     Date:   Fri Jun 15 10:26:13 2018 -0700

>         Add methods for strings to cl-print

> print-length is not used in the function prin1 (as contrasted with
> cl-prin1).

> The problem with using print-length for the max. string length is that it
> is not designed for this.  When print-length is, say, 50, almost any list
> or vector can be printed in full, going up to several hundred printed
> characters.  With a string, the string gets rudely truncated at a mere 50
> characters, which is rarely wanted.  This is a problem with the error
> message at the top of a backtrace, where cl-print-string-with-limit
> limits print-length to a maximum of 50 (See bug #65680).

> What do people think of removing this observance of print-length from the
> string version of cl-print-object?

I'm now convinced this is the right way to fix bug #65680, and also the
right thing to do generally.

So I'm intending to apply the following patch (together with a
corresponding amendment to cl-print-tests.el):



diff --git a/lisp/emacs-lisp/cl-print.el b/lisp/emacs-lisp/cl-print.el
index 71929caabb8..437000ba2a7 100644
--- a/lisp/emacs-lisp/cl-print.el
+++ b/lisp/emacs-lisp/cl-print.el
@@ -264,27 +264,17 @@ cl-print--struct-contents
 (cl-defmethod cl-print-object ((object string) stream)
   (unless stream (setq stream standard-output))
   (let* ((has-properties (or (text-properties-at 0 object)
-                             (next-property-change 0 object)))
-         (len (length object))
-         (limit (if (natnump print-length) (min print-length len) len)))
+                             (next-property-change 0 object))))
     (if (and has-properties
              cl-print--depth
              (natnump print-level)
              (> cl-print--depth print-level))
         (cl-print-insert-ellipsis object nil stream)
-      ;; Print all or part of the string
+      ;; Print the string.
       (when has-properties
         (princ "#(" stream))
-      (if (= limit len)
-          (prin1 (if has-properties (substring-no-properties object) object)
-                 stream)
-        (let ((part (concat (substring-no-properties object 0 limit) "...")))
-          (prin1 part stream)
-          (when (bufferp stream)
-            (with-current-buffer stream
-              (cl-print-propertize-ellipsis object limit
-                                            (- (point) 4)
-                                            (- (point) 1) stream)))))
+      (prin1 (if has-properties (substring-no-properties object) object)
+             stream)
       ;; Print the property list.
       (when has-properties
         (cl-print--string-props object 0 stream)


..  Does anybody have any objections?


-- 
Alan Mackenzie (Nuremberg, Germany).



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

end of thread, other threads:[~2023-09-18  9:46 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-17 19:57 Applying print-length to (length string) is suboptimal Alan Mackenzie
2023-09-18  9:46 ` Alan Mackenzie

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