unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#64536: 30.0.50; Make cl-print put buttons on ellipses
@ 2023-07-09  4:42 Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-07-13 23:01 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-08-11  7:53 ` Ihor Radchenko
  0 siblings, 2 replies; 11+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-07-09  4:42 UTC (permalink / raw)
  To: 64536; +Cc: Gemini Lasswell

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

Package: Emacs
Version: 30.0.50


Currently, in *Backtrace* we have a nice behavior for cl-printed objects
where they're truncated by default to a manageable size but we can click
on the "..." to expand them when needed.

The patch below moves most of that functionality to `cl-print.el` such
that it can be enjoyed "everywhere".

I think the patch is almost ready for `master` (beside the usual etc/NEWS
bit and checking whether the manual needs to be updated) but would
welcome other people's opinions.


        Stefan

[-- Attachment #2: cl-print-ellipses.patch --]
[-- Type: text/x-diff, Size: 17548 bytes --]

diff --git a/lisp/button.el b/lisp/button.el
index f043073ea86..002064fbea0 100644
--- a/lisp/button.el
+++ b/lisp/button.el
@@ -123,7 +123,7 @@ define-button-type
 
 In addition, the keyword argument :supertype may be used to specify a
 `button-type' from which NAME inherits its default property values
-(however, the inheritance happens only when NAME is defined; subsequent
+\(however, the inheritance happens only when NAME is defined; subsequent
 changes to a supertype are not reflected in its subtypes)."
   (declare (indent defun))
   (let ((catsym (make-symbol (concat (symbol-name name) "-button")))
diff --git a/lisp/emacs-lisp/backtrace.el b/lisp/emacs-lisp/backtrace.el
index 57912c854b0..4c6d9596b3f 100644
--- a/lisp/emacs-lisp/backtrace.el
+++ b/lisp/emacs-lisp/backtrace.el
@@ -135,8 +135,7 @@ backtrace--pop-to-buffer-pos
 ;; Font Locking support
 
 (defconst backtrace--font-lock-keywords
-  '((backtrace--match-ellipsis-in-string
-     (1 'button prepend)))
+  '()
   "Expressions to fontify in Backtrace mode.
 Fontify these in addition to the expressions Emacs Lisp mode
 fontifies.")
@@ -154,16 +153,6 @@ backtrace-font-lock-keywords-2
           backtrace--font-lock-keywords)
   "Gaudy level highlighting for Backtrace mode.")
 
-(defun backtrace--match-ellipsis-in-string (bound)
-  ;; Fontify ellipses within strings as buttons.
-  ;; This is necessary because ellipses are text property buttons
-  ;; instead of overlay buttons, which is done because there could
-  ;; be a large number of them.
-  (when (re-search-forward "\\(\\.\\.\\.\\)\"" bound t)
-    (and (get-text-property (- (point) 2) 'cl-print-ellipsis)
-         (get-text-property (- (point) 3) 'cl-print-ellipsis)
-         (get-text-property (- (point) 4) 'cl-print-ellipsis))))
-
 ;;; Xref support
 
 (defun backtrace--xref-backend () 'elisp)
@@ -425,11 +414,11 @@ backtrace--set-locals-visible-overlay
 
 (defun backtrace--change-button-skip (beg end value)
   "Change the skip property on all buttons between BEG and END.
-Set it to VALUE unless the button is a `backtrace-ellipsis' button."
+Set it to VALUE unless the button is a `cl-print-ellipsis' button."
   (let ((inhibit-read-only t))
     (setq beg (next-button beg))
     (while (and beg (< beg end))
-      (unless (eq (button-type beg) 'backtrace-ellipsis)
+      (unless (memq (button-type beg) '(backtrace-ellipsis cl-print-ellipsis))
           (button-put beg 'skip value))
       (setq beg (next-button beg)))))
 
@@ -497,33 +486,15 @@ backtrace--set-feature
                          `(backtrace-index ,index backtrace-view ,view))
     (goto-char min)))
 
-(defun backtrace-expand-ellipsis (button)
-  "Expand display of the elided form at BUTTON."
-  (goto-char (button-start button))
-  (unless (get-text-property (point) 'cl-print-ellipsis)
-    (if (and (> (point) (point-min))
-             (get-text-property (1- (point)) 'cl-print-ellipsis))
-        (backward-char)
-      (user-error "No ellipsis to expand here")))
-  (let* ((end (next-single-property-change (point) 'cl-print-ellipsis))
-         (begin (previous-single-property-change end 'cl-print-ellipsis))
-         (value (get-text-property begin 'cl-print-ellipsis))
-         (props (backtrace-get-text-properties begin))
+(defun backtrace--expand-ellipsis (orig-fun begin end val _length &rest args)
+  "Wrapper to expand an ellipsis.
+For use on `cl-print-expand-ellipsis-function'."
+  (let* ((props (backtrace-get-text-properties begin))
          (inhibit-read-only t))
     (backtrace--with-output-variables (backtrace-get-view)
-      (delete-region begin end)
-      (insert (cl-print-to-string-with-limit #'cl-print-expand-ellipsis value
-                                          backtrace-line-length))
-      (setq end (point))
-      (goto-char begin)
-      (while (< (point) end)
-        (let ((next (next-single-property-change (point) 'cl-print-ellipsis
-                                                 nil end)))
-          (when (get-text-property (point) 'cl-print-ellipsis)
-            (make-text-button (point) next :type 'backtrace-ellipsis))
-          (goto-char next)))
-      (goto-char begin)
-      (add-text-properties begin end props))))
+      (let ((end (apply orig-fun begin end val backtrace-line-length args)))
+        (add-text-properties begin end props)
+        end))))
 
 (defun backtrace-expand-ellipses (&optional no-limit)
   "Expand display of all \"...\"s in the backtrace frame at point.
@@ -696,13 +667,6 @@ backtrace-print
                  (recenter window-line)))
       (goto-char (point-min)))))
 
-;; Define button type used for ...'s.
-;; Set skip property so you don't have to TAB through 100 of them to
-;; get to the next function name.
-(define-button-type 'backtrace-ellipsis
-  'skip t 'action #'backtrace-expand-ellipsis
-  'help-echo "mouse-2, RET: expand this ellipsis")
-
 (defun backtrace-print-to-string (obj &optional limit)
   "Return a printed representation of OBJ formatted for backtraces.
 Attempt to get the length of the returned string under LIMIT
@@ -719,15 +683,6 @@ backtrace--print-to-string
     (insert (cl-print-to-string-with-limit #'backtrace--print sexp limit))
     ;; Add a unique backtrace-form property.
     (put-text-property (point-min) (point) 'backtrace-form (gensym))
-    ;; Make buttons from all the "..."s.  Since there might be many of
-    ;; them, use text property buttons.
-    (goto-char (point-min))
-    (while (< (point) (point-max))
-      (let ((end (next-single-property-change (point) 'cl-print-ellipsis
-                                              nil (point-max))))
-        (when (get-text-property (point) 'cl-print-ellipsis)
-          (make-text-button (point) end :type 'backtrace-ellipsis))
-        (goto-char end)))
     (buffer-string)))
 
 (defun backtrace-print-frame (frame view)
@@ -918,6 +873,8 @@ backtrace-mode
   (setq-local filter-buffer-substring-function #'backtrace--filter-visible)
   (setq-local indent-line-function 'lisp-indent-line)
   (setq-local indent-region-function 'lisp-indent-region)
+  (add-function :around (local 'cl-print-expand-ellipsis-function)
+                #'backtrace--expand-ellipsis)
   (add-hook 'xref-backend-functions #'backtrace--xref-backend nil t))
 
 (put 'backtrace-mode 'mode-class 'special)
diff --git a/lisp/emacs-lisp/cl-print.el b/lisp/emacs-lisp/cl-print.el
index 9578d556421..4d64249255a 100644
--- a/lisp/emacs-lisp/cl-print.el
+++ b/lisp/emacs-lisp/cl-print.el
@@ -54,9 +54,12 @@ cl-print-object
   (prin1 object stream))
 
 (cl-defgeneric cl-print-object-contents (_object _start _stream)
-  "Dispatcher to print the contents of OBJECT on STREAM.
-Print the contents starting with the item at START, without
-delimiters."
+  "Dispatcher to print partial contents of OBJECT on STREAM.
+This is used when replacing an ellipsis with the contents it
+represents.  OBJECT is the object that has been partially printed
+and START represents the place at which the contents where
+replaced with an ellipsis.
+Print the contents hidden by the ellipsis to STREAM."
   ;; Every cl-print-object method which can print an ellipsis should
   ;; have a matching cl-print-object-contents method to expand an
   ;; ellipsis.
@@ -65,7 +68,7 @@ cl-print-object-contents
 (cl-defmethod cl-print-object ((object cons) stream)
   (if (and cl-print--depth (natnump print-level)
            (> cl-print--depth print-level))
-      (cl-print-insert-ellipsis object 0 stream)
+      (cl-print-insert-ellipsis object nil stream)
     (let ((car (pop object)))
       (if (and print-quoted
                (memq car '(\, quote function \` \,@ \,.))
@@ -107,7 +110,7 @@ cl-print-object-contents
 (cl-defmethod cl-print-object ((object vector) stream)
   (if (and cl-print--depth (natnump print-level)
            (> cl-print--depth print-level))
-      (cl-print-insert-ellipsis object 0 stream)
+      (cl-print-insert-ellipsis object nil stream)
     (princ "[" stream)
     (cl-print--vector-contents object 0 stream)
     (princ "]" stream)))
@@ -129,6 +132,8 @@ cl-print-object-contents
   (cl-print--vector-contents object start stream)) ;FIXME: η-redex!
 
 (cl-defmethod cl-print-object ((object hash-table) stream)
+  ;; FIXME: Make it possible to see the contents, like `prin1' does,
+  ;; e.g. using ellipsis.  Make sure `cl-fill' can pretty print the result!
   (princ "#<hash-table " stream)
   (princ (hash-table-test object) stream)
   (princ " " stream)
@@ -158,6 +163,9 @@ cl-print-compiled-button
 
 (autoload 'disassemble-1 "disass")
 
+;; FIXME: Don't degenerate to `prin1' for the contents of char-tables
+;; and records!
+
 (cl-defmethod cl-print-object ((object compiled-function) stream)
   (unless stream (setq stream standard-output))
   ;; We use "#f(...)" rather than "#<...>" so that pp.el gives better results.
@@ -212,7 +220,7 @@ cl-print-object
 (cl-defmethod cl-print-object ((object cl-structure-object) stream)
   (if (and cl-print--depth (natnump print-level)
            (> cl-print--depth print-level))
-      (cl-print-insert-ellipsis object 0 stream)
+      (cl-print-insert-ellipsis object nil stream)
     (princ "#s(" stream)
     (princ (cl--struct-class-name (cl-find-class (type-of object))) stream)
     (cl-print--struct-contents object 0 stream)
@@ -250,7 +258,7 @@ cl-print-object
              cl-print--depth
              (natnump print-level)
              (> cl-print--depth print-level))
-        (cl-print-insert-ellipsis object 0 stream)
+        (cl-print-insert-ellipsis object nil stream)
       ;; Print all or part of the string
       (when has-properties
         (princ "#(" stream))
@@ -325,6 +333,7 @@ cl-print-object-contents
 (cl-defmethod cl-print-object :around (object stream)
   ;; FIXME: Only put such an :around method on types where it's relevant.
   (let ((cl-print--depth (if cl-print--depth (1+ cl-print--depth) 1)))
+    ;; FIXME: Handle print-depth here once and forall?
     (cond
      (print-circle
       (let ((n (gethash object cl-print--number-table)))
@@ -401,10 +410,53 @@ cl-print--preprocess
         (cl-print--find-sharing object print-number-table)))
     print-number-table))
 
+(define-button-type 'cl-print-ellipsis
+  'skip t 'action #'cl-print-expand-ellipsis
+  'help-echo "mouse-2, RET: expand this ellipsis")
+
+(defvar cl-print-expand-ellipsis-function
+  #'cl-print--default-expand-ellipsis
+  "Function to tweak the way ellipses are expanded.
+The function is called with 3 arguments, BEG, END, and FUNC.
+BEG and END delimit the ellipsis that will be replaced.
+FUNC is the function that will do the expansion.
+It should be called with a single argument specifying the desired
+limit of the expansion's length, as used in `cl-print-to-string-with-limit'.
+FUNC will return the position of the end of the newly printed text.")
+
+(defun cl-print--default-expand-ellipsis (begin end value line-length)
+  (delete-region begin end)
+  (insert (cl-print-to-string-with-limit
+           #'cl-print--expand-ellipsis value line-length))
+  (point))
+
+
+(defun cl-print-expand-ellipsis (&optional button)
+  "Expand display of the elided form at BUTTON.
+BUTTON can also be a buffer position or nil (to mean point)."
+  (interactive)
+  (goto-char (cond
+              ((null button) (point))
+              (t (button-start button))))
+  (unless (get-text-property (point) 'cl-print-ellipsis)
+    (if (and (> (point) (point-min))
+             (get-text-property (1- (point)) 'cl-print-ellipsis))
+        (backward-char)
+      (user-error "No ellipsis to expand here")))
+  (let* ((end (next-single-property-change (point) 'cl-print-ellipsis))
+         (begin (previous-single-property-change end 'cl-print-ellipsis))
+         (value (get-text-property begin 'cl-print-ellipsis)))
+    ;; FIXME: Rather than `t' (i.e. reuse the print-length/level unchanged),
+    ;; I think it would make sense to increase the level by 1 and to
+    ;; double the length at each expansion step.
+    (funcall cl-print-expand-ellipsis-function
+             begin end value t)
+    (goto-char begin)))
+
 (defun cl-print-insert-ellipsis (object start stream)
   "Print \"...\" to STREAM with the `cl-print-ellipsis' text property.
 Save state in the text property in order to print the elided part
-of OBJECT later.  START should be 0 if the whole OBJECT is being
+of OBJECT later.  START should be nil if the whole OBJECT is being
 elided, otherwise it should be an index or other pointer into the
 internals of OBJECT which can be passed to
 `cl-print-object-contents' at a future time."
@@ -423,11 +475,12 @@ cl-print-propertize-ellipsis
 `cl-print-insert-ellipsis'."
   (let ((value (list object start cl-print--number-table
                      cl-print--currently-printing)))
+    ;; FIXME: Make it into a button!
     (with-current-buffer stream
-      (put-text-property beg end 'cl-print-ellipsis value stream))))
+      (put-text-property beg end 'cl-print-ellipsis value stream)
+      (make-text-button beg end :type 'cl-print-ellipsis))))
 
-;;;###autoload
-(defun cl-print-expand-ellipsis (value stream)
+(defun cl-print--expand-ellipsis (value stream)
   "Print the expansion of an ellipsis to STREAM.
 VALUE should be the value of the `cl-print-ellipsis' text property
 which was attached to the ellipsis by `cl-prin1'."
@@ -439,7 +492,7 @@ cl-print-expand-ellipsis
         (cl-print--currently-printing (nth 3 value)))
     (when (eq object (car cl-print--currently-printing))
       (pop cl-print--currently-printing))
-    (if (equal start 0)
+    (if (memq start '(0 nil))
         (cl-print-object object stream)
       (cl-print-object-contents object start stream))))
 
@@ -474,22 +527,26 @@ cl-print-to-string-with-limit
 the arguments VALUE and STREAM and which should respect
 `print-length' and `print-level'.  LIMIT may be nil or zero in
 which case PRINT-FUNCTION will be called with `print-level' and
-`print-length' bound to nil.
+`print-length' bound to nil, and it can also be t in which case
+PRINT-FUNCTION will be called with the current values of `print-level'
+and `print-length'.
+
 
 Use this function with `cl-prin1' to print an object,
-abbreviating it with ellipses to fit within a size limit.  Use
-this function with `cl-prin1-expand-ellipsis' to expand an
-ellipsis, abbreviating the expansion to stay within a size
-limit."
-  (setq limit (and (natnump limit)
-                   (not (zerop limit))
-                   limit))
+abbreviating it with ellipses to fit within a size limit."
+  (setq limit (and (not (eq limit 0)) limit))
   ;; Since this is used by the debugger when stack space may be
   ;; limited, if you increase print-level here, add more depth in
   ;; call_debugger (bug#31919).
-  (let* ((print-length (when limit (min limit 50)))
-         (print-level (when limit (min 8 (truncate (log limit)))))
-         (delta-length (when limit
+  (let* ((print-length (cond
+                        ((null limit) nil)
+                        ((eq limit t) print-length)
+                        (t (min limit 50))))
+         (print-level (cond
+                        ((null limit) nil)
+                        ((eq limit t) print-level)
+                        (t (min 8 (truncate (log limit))))))
+         (delta-length (when (natnump limit)
                          (max 1 (truncate (/ print-length print-level))))))
     (with-temp-buffer
       (catch 'done
@@ -499,7 +556,7 @@ cl-print-to-string-with-limit
           (let ((result (- (point-max) (point-min))))
             ;; Stop when either print-level is too low or the value is
             ;; successfully printed in the space allowed.
-            (when (or (not limit) (< result limit) (<= print-level 2))
+            (when (or (not (natnump limit)) (< result limit) (<= print-level 2))
               (throw 'done (buffer-string)))
             (let* ((ratio (/ result limit))
                    (delta-level (max 1 (min (- print-level 2) ratio))))
diff --git a/test/lisp/emacs-lisp/cl-print-tests.el b/test/lisp/emacs-lisp/cl-print-tests.el
index af94dae310c..37d354afe18 100644
--- a/test/lisp/emacs-lisp/cl-print-tests.el
+++ b/test/lisp/emacs-lisp/cl-print-tests.el
@@ -113,7 +113,7 @@ cl-print-tests-check-ellipsis-expansion
     (should pos)
     (setq value (get-text-property pos 'cl-print-ellipsis result))
     (should (equal expected result))
-    (should (equal expanded (with-output-to-string (cl-print-expand-ellipsis
+    (should (equal expanded (with-output-to-string (cl-print--expand-ellipsis
                                                     value nil))))))
 
 (defun cl-print-tests-check-ellipsis-expansion-rx (obj expected expanded)
@@ -122,7 +122,7 @@ cl-print-tests-check-ellipsis-expansion-rx
          (value (get-text-property pos 'cl-print-ellipsis result)))
     (should (string-match expected result))
     (should (string-match expanded (with-output-to-string
-                                     (cl-print-expand-ellipsis value nil))))))
+                                     (cl-print--expand-ellipsis value nil))))))
 
 (ert-deftest cl-print-tests-print-to-string-with-limit ()
   (let* ((thing10 (make-list 10 'a))

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

* bug#64536: 30.0.50; Make cl-print put buttons on ellipses
  2023-07-09  4:42 bug#64536: 30.0.50; Make cl-print put buttons on ellipses Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-07-13 23:01 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-08-11  7:53 ` Ihor Radchenko
  1 sibling, 0 replies; 11+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-07-13 23:01 UTC (permalink / raw)
  To: 64536-done; +Cc: Gemini Lasswell

> Currently, in *Backtrace* we have a nice behavior for cl-printed objects
> where they're truncated by default to a manageable size but we can click
> on the "..." to expand them when needed.
>
> The patch below moves most of that functionality to `cl-print.el` such
> that it can be enjoyed "everywhere".

Pushed to `master`.


        Stefan






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

* bug#64536: 30.0.50; Make cl-print put buttons on ellipses
  2023-07-09  4:42 bug#64536: 30.0.50; Make cl-print put buttons on ellipses Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-07-13 23:01 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-08-11  7:53 ` Ihor Radchenko
  2023-08-12  5:23   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 11+ messages in thread
From: Ihor Radchenko @ 2023-08-11  7:53 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Gemini Lasswell, 64536

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> -;; Define button type used for ...'s.
> -;; Set skip property so you don't have to TAB through 100 of them to
> -;; get to the next function name.
> -(define-button-type 'backtrace-ellipsis
> -  'skip t 'action #'backtrace-expand-ellipsis
> -  'help-echo "mouse-2, RET: expand this ellipsis")

Isn't this a breaking change?
I just got inspector.el broken because it uses 'backtrace-ellipsis
button type.

At least, I'd expect to see the removal documented in NEWS.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>





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

* bug#64536: 30.0.50; Make cl-print put buttons on ellipses
  2023-08-11  7:53 ` Ihor Radchenko
@ 2023-08-12  5:23   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-08-12  8:39     ` Ihor Radchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-08-12  5:23 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: Gemini Lasswell, 64536

>> -;; Define button type used for ...'s.
>> -;; Set skip property so you don't have to TAB through 100 of them to
>> -;; get to the next function name.
>> -(define-button-type 'backtrace-ellipsis
>> -  'skip t 'action #'backtrace-expand-ellipsis
>> -  'help-echo "mouse-2, RET: expand this ellipsis")
>
> Isn't this a breaking change?
> I just got inspector.el broken because it uses 'backtrace-ellipsis
> button type.

Apparently it's a breaking change, indeed.

Hmm... we could re-add it, but with the new `cl-print.el` your code
should simply not use `backtrace-ellipsis` at all because `cl-print.el`
did it for you already.

IOW, I think a patch like the one below my sig is the better option.

> At least, I'd expect to see the removal documented in NEWS.

How 'bout:

     The code that allowed "..." to be expanded in the "*Backtrace*" buffer
     should now work anywhere the data is generated by 'cl-print'.
     
    +*** The 'backtrace-ellipsis' button is replaced by 'cl-print-ellipsis'.
    +
     *** hash-tables' contents can be expanded via the ellipsis.
     
    +*** Modes can control the expansion via 'cl-print-expand-ellipsis-function'.
    +
     ** Modeline elements can now be right-aligned.
     Anything following the symbol 'mode-line-format-right-align' in
     'mode-line-format' will be right-aligned.  Exactly where it is


-- Stefan


diff --git a/inspector.el b/inspector.el
index fa9b1cc778..e00209f2be 100644
--- a/inspector.el
+++ b/inspector.el
@@ -1,6 +1,6 @@
 ;;; inspector.el --- Tool for inspection of Emacs Lisp objects  -*- lexical-binding: t -*-
 
-;; Copyright (C) 2021-2022 Free Software Foundation, Inc.
+;; Copyright (C) 2021-2023 Free Software Foundation, Inc.
 
 ;; Author: Mariano Montone <marianomontone@gmail.com>
 ;; URL: https://github.com/mmontone/emacs-inspector
@@ -243,13 +243,14 @@ LIMIT controls the truncation."
     (put-text-property (point-min) (point) 'inspector-form (gensym))
     ;; Make buttons from all the "..."s.  Since there might be many of
     ;; them, use text property buttons.
-    (goto-char (point-min))
-    (while (< (point) (point-max))
-      (let ((end (next-single-property-change (point) 'cl-print-ellipsis
-                                              nil (point-max))))
-        (when (get-text-property (point) 'cl-print-ellipsis)
-          (make-text-button (point) end :type 'backtrace-ellipsis))
-        (goto-char end)))
+    (unless (boundp 'cl-print-expand-ellipsis-function) ;Emacs-30
+      (goto-char (point-min))
+      (while (< (point) (point-max))
+        (let ((end (next-single-property-change (point) 'cl-print-ellipsis
+                                                nil (point-max))))
+          (when (get-text-property (point) 'cl-print-ellipsis)
+            (make-text-button (point) end :type 'backtrace-ellipsis))
+          (goto-char end))))
     (buffer-string)))
 
 (cl-defgeneric inspector--face-for-object (object)







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

* bug#64536: 30.0.50; Make cl-print put buttons on ellipses
  2023-08-12  5:23   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-08-12  8:39     ` Ihor Radchenko
  2023-08-12 18:17       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 11+ messages in thread
From: Ihor Radchenko @ 2023-08-12  8:39 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Gemini Lasswell, 64536

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

> Hmm... we could re-add it, but with the new `cl-print.el` your code
> should simply not use `backtrace-ellipsis` at all because `cl-print.el`
> did it for you already.
>
> IOW, I think a patch like the one below my sig is the better option.

Sure. Thanks for the patch! I will let inspector.el author know about
it.

However, other users of `backtrace-ellipsis' button may be affected if
they used it for different purposes.

May it be possible to create an obsolete alias for this button type?

>> At least, I'd expect to see the removal documented in NEWS.
>
> How 'bout:
>
>      The code that allowed "..." to be expanded in the "*Backtrace*" buffer
>      should now work anywhere the data is generated by 'cl-print'.
>      
>     +*** The 'backtrace-ellipsis' button is replaced by 'cl-print-ellipsis'.

Looks reasonable.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>





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

* bug#64536: 30.0.50; Make cl-print put buttons on ellipses
  2023-08-12  8:39     ` Ihor Radchenko
@ 2023-08-12 18:17       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-08-13  9:26         ` Ihor Radchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-08-12 18:17 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: Gemini Lasswell, 64536

> However, other users of `backtrace-ellipsis' button may be affected if
> they used it for different purposes.

That's also what I thought at first, but the case of `inspect.el` made
me realize that it's likely that just letting them use
`backtrace-ellipsis` button is often not the right solution anyway.

> May it be possible to create an obsolete alias for this button type?

And I don't think we have such aliases.


        Stefan






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

* bug#64536: 30.0.50; Make cl-print put buttons on ellipses
  2023-08-12 18:17       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-08-13  9:26         ` Ihor Radchenko
  2023-08-13 16:02           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 11+ messages in thread
From: Ihor Radchenko @ 2023-08-13  9:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Gemini Lasswell, 64536

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

>> However, other users of `backtrace-ellipsis' button may be affected if
>> they used it for different purposes.
>
> That's also what I thought at first, but the case of `inspect.el` made
> me realize that it's likely that just letting them use
> `backtrace-ellipsis` button is often not the right solution anyway.

I am pretty sure that some uses of `backtrace-ellipsis' have nothing to
do with printing Elisp values. At least, there is no reason why they
should be limited to.

>> May it be possible to create an obsolete alias for this button type?
>
> And I don't think we have such aliases.

May it be added?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>





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

* bug#64536: 30.0.50; Make cl-print put buttons on ellipses
  2023-08-13  9:26         ` Ihor Radchenko
@ 2023-08-13 16:02           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-08-14 11:01             ` Ihor Radchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-08-13 16:02 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: Gemini Lasswell, 64536

>> That's also what I thought at first, but the case of `inspect.el` made
>> me realize that it's likely that just letting them use
>> `backtrace-ellipsis` button is often not the right solution anyway.
> I am pretty sure that some uses of `backtrace-ellipsis' have nothing to
> do with printing Elisp values.

Could you expand on what makes you think so?
I grepped for it in Melpa and couldn't find any use of it at all, FWIW.

> At least, there is no reason why they should be limited to.

It was rather tightly integrated with `cl-print`, tho:
the button called `backtrace-expand-ellipsis` which itself looked for
a `cl-print-ellipsis` text-property and called
`cl-print-to-string-with-limit` and `cl-print-expand-ellipsis` on it.

>>> May it be possible to create an obsolete alias for this button type?
>> And I don't think we have such aliases.
> May it be added?

Patch welcome (especially if it comes together with some kind of
obsolescence mechanism).


        Stefan






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

* bug#64536: 30.0.50; Make cl-print put buttons on ellipses
  2023-08-13 16:02           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-08-14 11:01             ` Ihor Radchenko
  2023-08-14 14:07               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 11+ messages in thread
From: Ihor Radchenko @ 2023-08-14 11:01 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Gemini Lasswell, 64536

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

>> I am pretty sure that some uses of `backtrace-ellipsis' have nothing to
>> do with printing Elisp values.
>
> Could you expand on what makes you think so?

Theoretical backward-compatibility considerations.

> I grepped for it in Melpa and couldn't find any use of it at all, FWIW.

You are right. I also searched in
https://github.com/search?q=backtrace-ellipsis+language%3A%22Emacs+Lisp%22&type=code&l=Emacs+Lisp&p=1

And found nothing apart from Emacs forks and inspector.el.
So, it should likely be safe to remove this button.

>> At least, there is no reason why they should be limited to.
>
> It was rather tightly integrated with `cl-print`, tho:
> the button called `backtrace-expand-ellipsis` which itself looked for
> a `cl-print-ellipsis` text-property and called
> `cl-print-to-string-with-limit` and `cl-print-expand-ellipsis` on it.

I see. Another +1 to silent removal. I thought that is it less
specialized (given that its name is public).

>>>> May it be possible to create an obsolete alias for this button type?
>>> And I don't think we have such aliases.
>> May it be added?
>
> Patch welcome (especially if it comes together with some kind of
> obsolescence mechanism).

I am not sure if this can be done on byte compiler level.
For runtime, after skimming through button.el, it looks like
`button-category-symbol' might be used to throw a warning when obsolete
button type is being used.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>





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

* bug#64536: 30.0.50; Make cl-print put buttons on ellipses
  2023-08-14 11:01             ` Ihor Radchenko
@ 2023-08-14 14:07               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-08-14 15:15                 ` Ihor Radchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-08-14 14:07 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: Gemini Lasswell, 64536

>>>>> May it be possible to create an obsolete alias for this button type?
>>>> And I don't think we have such aliases.
>>> May it be added?
>> Patch welcome (especially if it comes together with some kind of
>> obsolescence mechanism).
> I am not sure if this can be done on byte compiler level.

Not sure either.  A compiler-macro placed on `make-text-button` and
`make-button` could look for a `:type` arg and check its obsolescence,
but I don't know how effective it would be.

> For runtime, after skimming through button.el, it looks like
> `button-category-symbol' might be used to throw a warning when obsolete
> button type is being used.

Runtime check are easier and more reliable, indeed, but also more
annoying and directed at the wrong person :-(


        Stefan






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

* bug#64536: 30.0.50; Make cl-print put buttons on ellipses
  2023-08-14 14:07               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-08-14 15:15                 ` Ihor Radchenko
  0 siblings, 0 replies; 11+ messages in thread
From: Ihor Radchenko @ 2023-08-14 15:15 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Gemini Lasswell, 64536

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

>> I am not sure if this can be done on byte compiler level.
>
> Not sure either.  A compiler-macro placed on `make-text-button` and
> `make-button` could look for a `:type` arg and check its obsolescence,
> but I don't know how effective it would be.

Maybe not that bad.
Of course, it will miss :type values not known at compile time, but it
is the same issue even with ordinary obsolete variables/functions that
are used not by their literal name.

>> For runtime, after skimming through button.el, it looks like
>> `button-category-symbol' might be used to throw a warning when obsolete
>> button type is being used.
>
> Runtime check are easier and more reliable, indeed, but also more
> annoying and directed at the wrong person :-(

Makes sense. Although, they can be useful for maintainers if such checks
can be enabled explicitly, by flipping a variable.
We used such approach for org-element--cache-self-verify.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>





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

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

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-09  4:42 bug#64536: 30.0.50; Make cl-print put buttons on ellipses Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-07-13 23:01 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-11  7:53 ` Ihor Radchenko
2023-08-12  5:23   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-12  8:39     ` Ihor Radchenko
2023-08-12 18:17       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-13  9:26         ` Ihor Radchenko
2023-08-13 16:02           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-14 11:01             ` Ihor Radchenko
2023-08-14 14:07               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-14 15:15                 ` Ihor Radchenko

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