* bug#73775: 30.0.90; vtable: can't handle 0 data rows
2024-10-13 5:27 ` Eli Zaretskii
2024-10-13 8:00 ` Joost Kremers
@ 2024-10-14 11:25 ` Joost Kremers
1 sibling, 0 replies; 6+ messages in thread
From: Joost Kremers @ 2024-10-14 11:25 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Adam Porter, 73775, arstoffel
[-- Attachment #1: Type: text/plain, Size: 884 bytes --]
On Sun, Oct 13 2024, Eli Zaretskii wrote:
>> Date: Sat, 12 Oct 2024 19:12:50 -0500
>> Cc: 73775@debbugs.gnu.org
>> From: Adam Porter <adam@alphapapa.net>
>>
>> > Adam, any comments or suggestions?
>>
>> I may be mistaken, but this seems like a duplicate of bug#69454.
>
> II had a vague memory this was already discussed, but couldn't find
> such a bug. Thanks for pointing this out.
>
> We seem to have dropped the ball on that one. Joost, would you please
> rebase on the current master and resubmit?
Here is the patch again, rebased on current master (or what was current
master earlier today... :D )
As before, there are three patches, because they solve different issues
that stood in the way of having empty vtables. I could squash them into
one, of course. Also, the documentation and NEWS update are separate.
Regards,
Joost
--
Joost Kremers
Life has its moments
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-empty-vtable.patch --]
[-- Type: text/x-patch, Size: 4361 bytes --]
From c6aa49efb559875be10e0558dcd6dc3067333f6a Mon Sep 17 00:00:00 2001
From: Joost Kremers <joostkremers@fastmail.com>
Date: Thu, 30 May 2024 13:28:00 +0200
Subject: [PATCH 1/4] Allow empty vtable
* lisp/emacs-lisp/vtable.el (vtable--compute-widths): Set default width
for columns that have no explicit width and no data.
---
lisp/emacs-lisp/vtable.el | 67 ++++++++++++++++++++++++---------------
1 file changed, 41 insertions(+), 26 deletions(-)
diff --git a/lisp/emacs-lisp/vtable.el b/lisp/emacs-lisp/vtable.el
index d58c6894c16..ddb7b342a3e 100644
--- a/lisp/emacs-lisp/vtable.el
+++ b/lisp/emacs-lisp/vtable.el
@@ -850,32 +850,47 @@ vtable--compute-width
(error "Invalid spec: %s" spec))))
(defun vtable--compute-widths (table cache)
- "Compute the display widths for TABLE."
- (seq-into
- (seq-map-indexed
- (lambda (column index)
- (let ((width
- (or
- ;; Explicit widths.
- (and (vtable-column-width column)
- (vtable--compute-width table (vtable-column-width column)))
- ;; Compute based on the displayed widths of
- ;; the data.
- (seq-max (seq-map (lambda (elem)
- (nth 1 (elt (cdr elem) index)))
- cache)))))
- ;; Let min-width/max-width specs have their say.
- (when-let ((min-width (and (vtable-column-min-width column)
- (vtable--compute-width
- table (vtable-column-min-width column)))))
- (setq width (max width min-width)))
- (when-let ((max-width (and (vtable-column-max-width column)
- (vtable--compute-width
- table (vtable-column-max-width column)))))
- (setq width (min width max-width)))
- width))
- (vtable-columns table))
- 'vector))
+ "Compute the display widths for TABLE.
+CACHE is TABLE's cache data as returned by `vtable--compute-cache'."
+ (let ((widths (seq-map-indexed
+ (lambda (column index)
+ (let ((width
+ (or
+ ;; Explicit widths.
+ (and (vtable-column-width column)
+ (vtable--compute-width table (vtable-column-width column)))
+ ;; If the vtable is empty and no explicit width is given,
+ ;; set its width to 0 and deal with it below.
+ (if (null cache)
+ 0)
+ ;; Otherwise, compute based on the displayed widths of the
+ ;; data.
+ (seq-max (seq-map (lambda (elem)
+ (nth 1 (elt (cdr elem) index)))
+ cache)))))
+ ;; Let min-width/max-width specs have their say.
+ (when-let ((min-width (and (vtable-column-min-width column)
+ (vtable--compute-width
+ table (vtable-column-min-width column)))))
+ (setq width (max width min-width)))
+ (when-let ((max-width (and (vtable-column-max-width column)
+ (vtable--compute-width
+ table (vtable-column-max-width column)))))
+ (setq width (min width max-width)))
+ width))
+ (vtable-columns table))))
+ ;; If there are any zero-width columns, divide the remaining window
+ ;; width evenly over them.
+ (when (member 0 widths)
+ (let* ((combined-width (apply #'+ widths))
+ (n-0cols (length (seq-keep #'zerop widths)))
+ (default-width (/ (- (window-width nil t) combined-width) n-0cols)))
+ (setq widths (mapcar (lambda (width)
+ (if (zerop width)
+ default-width
+ width))
+ widths))))
+ (seq-into widths 'vector)))
(defun vtable--compute-cache (table)
(seq-map
--
2.47.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Enable-inserting-new-objects-into-empty-vtable.patch --]
[-- Type: text/x-patch, Size: 8558 bytes --]
From 272773f89ccd287fb5de15f3d9feff043afe091e Mon Sep 17 00:00:00 2001
From: Joost Kremers <joostkremers@fastmail.com>
Date: Thu, 30 May 2024 23:20:00 +0200
Subject: [PATCH 2/4] Enable inserting new objects into empty vtable
* lisp/emacs-lisp/vtable.el (vtable-insert-object): If the vtable is
empty, add the new object and recreate + redisplay the table.
---
lisp/emacs-lisp/vtable.el | 151 +++++++++++++++++++-------------------
1 file changed, 77 insertions(+), 74 deletions(-)
diff --git a/lisp/emacs-lisp/vtable.el b/lisp/emacs-lisp/vtable.el
index ddb7b342a3e..0a399f636eb 100644
--- a/lisp/emacs-lisp/vtable.el
+++ b/lisp/emacs-lisp/vtable.el
@@ -368,86 +368,89 @@ vtable-insert-object
case.
This also updates the displayed table."
- ;; FIXME: Inserting an object into an empty vtable currently isn't
- ;; possible. `nconc' fails silently (twice), and `setcar' on the cache
- ;; raises an error.
+ ;; If the vtable is empty, just add the object and regenerate the
+ ;; table.
(if (null (vtable-objects table))
- (error "[vtable] Cannot insert object into empty vtable"))
- ;; First insert into the objects.
- (let ((pos (if location
- (if (integerp location)
- (prog1
- (nthcdr location (vtable-objects table))
- ;; Do not prepend if index is too large:
- (setq before nil))
- (or (memq location (vtable-objects table))
- ;; Prepend if `location' is not found and
- ;; `before' is non-nil:
- (and before (vtable-objects table))))
- ;; If `location' is nil and `before' is non-nil, we
- ;; prepend the new object.
- (if before (vtable-objects table)))))
- (if (or before ; If `before' is non-nil, `pos' should be, as well.
- (and pos (integerp location)))
- ;; Add the new object before.
- (let ((old-object (car pos)))
- (setcar pos object)
- (setcdr pos (cons old-object (cdr pos))))
- ;; Otherwise, add the object after.
- (if pos
- ;; Splice the object into the list.
- (setcdr pos (cons object (cdr pos)))
- ;; Otherwise, append the object.
- (nconc (vtable-objects table) (list object)))))
- ;; Then adjust the cache and display.
- (save-excursion
- (vtable-goto-table table)
- (let* ((cache (vtable--cache table))
- (inhibit-read-only t)
- (keymap (get-text-property (point) 'keymap))
- (ellipsis (if (vtable-ellipsis table)
- (propertize (truncate-string-ellipsis)
- 'face (vtable-face table))
- ""))
- (ellipsis-width (string-pixel-width ellipsis))
- (elem (if location ; This binding mirrors the binding of `pos' above.
- (if (integerp location)
- (nth location (car cache))
- (or (assq location (car cache))
- (and before (caar cache))))
- (if before (caar cache))))
- (pos (memq elem (car cache)))
- (line (cons object (vtable--compute-cached-line table object))))
- (if (or before
+ (progn
+ (setf (vtable-objects table) (list object))
+ (vtable--recompute-numerical table (vtable--compute-cached-line table object))
+ (vtable-goto-table table)
+ (vtable-revert-command))
+ ;; First insert into the objects.
+ (let ((pos (if location
+ (if (integerp location)
+ (prog1
+ (nthcdr location (vtable-objects table))
+ ;; Do not prepend if index is too large:
+ (setq before nil))
+ (or (memq location (vtable-objects table))
+ ;; Prepend if `location' is not found and
+ ;; `before' is non-nil:
+ (and before (vtable-objects table))))
+ ;; If `location' is nil and `before' is non-nil, we
+ ;; prepend the new object.
+ (if before (vtable-objects table)))))
+ (if (or before ; If `before' is non-nil, `pos' should be, as well.
(and pos (integerp location)))
- ;; Add the new object before:.
- (let ((old-line (car pos)))
- (setcar pos line)
- (setcdr pos (cons old-line (cdr pos)))
- (unless (vtable-goto-object (car elem))
- (vtable-beginning-of-table)))
+ ;; Add the new object before.
+ (let ((old-object (car pos)))
+ (setcar pos object)
+ (setcdr pos (cons old-object (cdr pos))))
;; Otherwise, add the object after.
(if pos
;; Splice the object into the list.
- (progn
- (setcdr pos (cons line (cdr pos)))
- (if (vtable-goto-object location)
- (forward-line 1) ; Insert *after*.
- (vtable-end-of-table)))
+ (setcdr pos (cons object (cdr pos)))
;; Otherwise, append the object.
- (setcar cache (nconc (car cache) (list line)))
- (vtable-end-of-table)))
- (let ((start (point)))
- ;; FIXME: We have to adjust colors in lines below this if we
- ;; have :row-colors.
- (vtable--insert-line table line 0
- (nth 1 cache) (vtable--spacer table)
- ellipsis ellipsis-width)
- (add-text-properties start (point) (list 'keymap keymap
- 'vtable table)))
- ;; We may have inserted a non-numerical value into a previously
- ;; all-numerical table, so recompute.
- (vtable--recompute-numerical table (cdr line)))))
+ (nconc (vtable-objects table) (list object)))))
+ ;; Then adjust the cache and display.
+ (save-excursion
+ (vtable-goto-table table)
+ (let* ((cache (vtable--cache table))
+ (inhibit-read-only t)
+ (keymap (get-text-property (point) 'keymap))
+ (ellipsis (if (vtable-ellipsis table)
+ (propertize (truncate-string-ellipsis)
+ 'face (vtable-face table))
+ ""))
+ (ellipsis-width (string-pixel-width ellipsis))
+ (elem (if location ; This binding mirrors the binding of `pos' above.
+ (if (integerp location)
+ (nth location (car cache))
+ (or (assq location (car cache))
+ (and before (caar cache))))
+ (if before (caar cache))))
+ (pos (memq elem (car cache)))
+ (line (cons object (vtable--compute-cached-line table object))))
+ (if (or before
+ (and pos (integerp location)))
+ ;; Add the new object before:.
+ (let ((old-line (car pos)))
+ (setcar pos line)
+ (setcdr pos (cons old-line (cdr pos)))
+ (unless (vtable-goto-object (car elem))
+ (vtable-beginning-of-table)))
+ ;; Otherwise, add the object after.
+ (if pos
+ ;; Splice the object into the list.
+ (progn
+ (setcdr pos (cons line (cdr pos)))
+ (if (vtable-goto-object location)
+ (forward-line 1) ; Insert *after*.
+ (vtable-end-of-table)))
+ ;; Otherwise, append the object.
+ (setcar cache (nconc (car cache) (list line)))
+ (vtable-end-of-table)))
+ (let ((start (point)))
+ ;; FIXME: We have to adjust colors in lines below this if we
+ ;; have :row-colors.
+ (vtable--insert-line table line 0
+ (nth 1 cache) (vtable--spacer table)
+ ellipsis ellipsis-width)
+ (add-text-properties start (point) (list 'keymap keymap
+ 'vtable table)))
+ ;; We may have inserted a non-numerical value into a previously
+ ;; all-numerical table, so recompute.
+ (vtable--recompute-numerical table (cdr line))))))
(defun vtable-column (table index)
"Return the name of the INDEXth column in TABLE."
--
2.47.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-vtable-allow-resetting-column-alignment-when-table-d.patch --]
[-- Type: text/x-patch, Size: 3025 bytes --]
From abd9d2b02381e35a81188757099bb7d4343a067a Mon Sep 17 00:00:00 2001
From: Joost Kremers <joostkremers@fastmail.com>
Date: Fri, 31 May 2024 01:38:54 +0200
Subject: [PATCH 3/4] vtable: allow resetting column alignment when table data
changes
* lisp/emacs-lisp/vtable.el (vtable--compute-columns): if a column was
not created with an explicit 'align' property, allow changing this
property when the column data changes from numeric to non-numeric (or
vice versa). This makes it possible to add data to an empty table,
because in a table without data all columns are assumed to be numeric
and right-aligned.
---
lisp/emacs-lisp/vtable.el | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/lisp/emacs-lisp/vtable.el b/lisp/emacs-lisp/vtable.el
index 0a399f636eb..b14b9510fb1 100644
--- a/lisp/emacs-lisp/vtable.el
+++ b/lisp/emacs-lisp/vtable.el
@@ -45,7 +45,8 @@ vtable-column
getter
formatter
displayer
- -numerical)
+ -numerical
+ -aligned)
(defclass vtable ()
((columns :initarg :columns :accessor vtable-columns)
@@ -473,7 +474,17 @@ vtable--get-value
(t
(elt object index))))
-(defun vtable--compute-columns (table)
+(defun vtable--compute-columns (table &optional recompute)
+ "Compute column specs for TABLE.
+Set the `align', `-aligned' and `-numerical' properties of each column.
+If the column contains only numerical data, set `-numerical' to t,
+otherwise to nil. `-aligned' indicates whether the column has an
+`align' property set by the user. If it does, `align' is not touched,
+otherwise it is set to `right' for numeric columns and to `left' for
+non-numeric columns.
+
+If RECOMPUTE is non-nil, do not set `-aligned'. This can be used to
+recompute the column specs when the table data has changed."
(let ((numerical (make-vector (length (vtable-columns table)) t))
(columns (vtable-columns table)))
;; First determine whether there are any all-numerical columns.
@@ -484,11 +495,16 @@ vtable--compute-columns
table))
(setf (elt numerical index) nil)))
(vtable-columns table)))
+ ;; Check if any columns have an explicit `align' property.
+ (unless recompute
+ (dolist (column (vtable-columns table))
+ (if (vtable-column-align column)
+ (setf (vtable-column--aligned column) t))))
;; Then fill in defaults.
(seq-map-indexed
(lambda (column index)
;; This is used when displaying.
- (unless (vtable-column-align column)
+ (unless (vtable-column--aligned column)
(setf (vtable-column-align column)
(if (elt numerical index)
'right
@@ -813,7 +829,7 @@ vtable--recompute-numerical
(setq recompute t)))
line)
(when recompute
- (vtable--compute-columns table))))
+ (vtable--compute-columns table t))))
(defun vtable--set-header-line (table widths spacer)
(setq header-line-format
--
2.47.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-Update-vtable-documentation.patch --]
[-- Type: text/x-patch, Size: 2576 bytes --]
From a60d29310c900086b17f05d802a9854f3e708b6e Mon Sep 17 00:00:00 2001
From: Joost Kremers <joostkremers@fastmail.com>
Date: Mon, 14 Oct 2024 13:10:57 +0200
Subject: [PATCH 4/4] Update vtable documentation
* docs/misc/vtable.texi: Document creation of empty vtables.
* etc/NEWS: Announce empty vtables.
---
doc/misc/vtable.texi | 11 +++++++++++
etc/NEWS | 9 +++++++++
2 files changed, 20 insertions(+)
diff --git a/doc/misc/vtable.texi b/doc/misc/vtable.texi
index 795d7fad037..b2ead47d0bc 100644
--- a/doc/misc/vtable.texi
+++ b/doc/misc/vtable.texi
@@ -264,6 +264,10 @@ Making A Table
more elements in the sequence than there is in @code{:columns}, only
the @code{:columns} first elements are displayed.
+If the @code{:objects} list is empty (and no @code{:objects-function} is
+defined), an empty vtable is created. In this case, a @code{:columns}
+spec must be provided.
+
@item :objects-function
It's often convenient to generate the objects dynamically (for
instance, to make reversion work automatically). In that case, this
@@ -295,6 +299,11 @@ Making A Table
@var{n} percent of the window's width.
@end table
+If no @code{width} is provided, the width is calculated based on the
+column data (provided in the @code{:objects} list or through the
+@code{:objects-function}) or, if there is no data, on the basis of the
+window width.
+
@item min-width
This uses the same format as @code{width}, but specifies the minimum
width (and overrides @code{width} if @code{width} is smaller than this.
@@ -576,6 +585,8 @@ Interface Functions
index is too small, or appended if it is too large. In this case,
@var{before} is ignored.
+If @var{table} is empty, @var{location} and @var{before} are ignored.
+
This also updates the displayed table.
@end defun
diff --git a/etc/NEWS b/etc/NEWS
index 4346fb4aedd..ecd22499767 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -662,6 +662,15 @@ This lets a Lisp program access the core functionality of the
program, choosing the program according to the operating system's
conventions.
+** 'make-vtable' can create an empty vtable
+It is now possible to create a vtable without data, by leaving the
+':objects' list empty, or by providing a ':objects-function' that
+(initially) produces no data. In such a case, it is necessary to
+provide a ':columns' spec, so that the number of columns and their
+widths can be determined. Columns widths can be set explicitly, or they
+will be calculated based on the window width.
+
+
\f
* Changes in Emacs 31.1 on Non-Free Operating Systems
--
2.47.0
^ permalink raw reply related [flat|nested] 6+ messages in thread