* bug#69454: Not possible to insert an empty vtable @ 2024-02-28 14:29 Eric Marsden 2024-03-09 8:54 ` Eli Zaretskii 0 siblings, 1 reply; 17+ messages in thread From: Eric Marsden @ 2024-02-28 14:29 UTC (permalink / raw) To: 69454 Hello, The following generates an error. It seems to me that it would be preferable to insert the header line and show zero rows for the vtable. (require 'vtable) (make-vtable :columns '("tweedle" "dum") :objects (list)) Debugger entered--Lisp error: (wrong-number-of-arguments #<subr max> 0) max() apply(max nil) seq-max(nil) #f(compiled-function (column index) #<bytecode -0x1c8aa8d5280f387a>)(#s(vtable-column :name "tweedle" :width nil :min-width nil :max-width nil :primary nil :align right :getter nil :formatter nil :displayer nil :-numerical t) 0) #f(compiled-function (elt) #<bytecode -0x13aa50143314c409>)(#s(vtable-column :name "tweedle" :width nil :min-width nil :max-width nil :primary nil :align right :getter nil :formatter nil :displayer nil :-numerical t)) mapcar(#f(compiled-function (elt) #<bytecode -0x13aa50143314c409>) (#s(vtable-column :name "tweedle" :width nil :min-width nil :max-width nil :primary nil :align right :getter nil :formatter nil :displayer nil :-numerical t) #s(vtable-column :name "dum" :width nil :min-width nil :max-width nil :primary nil :align right :getter nil :formatter nil :displayer nil :-numerical t))) #f(compiled-function #'sequence #<bytecode 0x1843ad21c7e878b4>)(#f(compiled-function (elt) #<bytecode -0x13aa50143314c409>) (#s(vtable-column :name "tweedle" :width nil :min-width nil :max-width nil :primary nil :align right :getter nil :formatter nil :displayer nil :-numerical t) #s(vtable-column :name "dum" :width nil :min-width nil :max-width nil :primary nil :align right :getter nil :formatter nil :displayer nil :-numerical t))) apply(#f(compiled-function #'sequence #<bytecode 0x1843ad21c7e878b4>) #f(compiled-function (elt) #<bytecode -0x13aa50143314c409>) (#s(vtable-column :name "tweedle" :width nil :min-width nil :max-width nil :primary nil :align right :getter nil :formatter nil :displayer nil :-numerical t) #s(vtable-column :name "dum" :width nil :min-width nil :max-width nil :primary nil :align right :getter nil :formatter nil :displayer nil :-numerical t)) nil) seq-map(#f(compiled-function (elt) #<bytecode -0x13aa50143314c409>) (#s(vtable-column :name "tweedle" :width nil :min-width nil :max-width nil :primary nil :align right :getter nil :formatter nil :displayer nil :-numerical t) #s(vtable-column :name "dum" :width nil :min-width nil :max-width nil :primary nil :align right :getter nil :formatter nil :displayer nil :-numerical t))) seq-map-indexed(#f(compiled-function (column index) #<bytecode -0x1c8aa8d5280f387a>) (#s(vtable-column :name "tweedle" :width nil :min-width nil :max-width nil :primary nil :align right :getter nil :formatter nil :displayer nil :-numerical t) #s(vtable-column :name "dum" :width nil :min-width nil :max-width nil :primary nil :align right :getter nil :formatter nil :displayer nil :-numerical t))) vtable--compute-widths(#<vtable vtable-158e2cf53118> nil) vtable--recompute-cache(#<vtable vtable-158e2cf53118>) vtable--ensure-cache(#<vtable vtable-158e2cf53118>) vtable-insert(#<vtable vtable-158e2cf53118>) make-vtable(:columns ("tweedle" "dum") :objects nil) (progn (make-vtable :columns '("tweedle" "dum") :objects (list))) elisp--eval-last-sexp(t) eval-last-sexp(t) eval-print-last-sexp(nil) GNU Emacs 29.2 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.41, cairo version 1.18.0) of 2024-02-27, modified by Debian ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-02-28 14:29 bug#69454: Not possible to insert an empty vtable Eric Marsden @ 2024-03-09 8:54 ` Eli Zaretskii 2024-03-11 19:57 ` Adam Porter 2024-04-30 9:10 ` Joost Kremers 0 siblings, 2 replies; 17+ messages in thread From: Eli Zaretskii @ 2024-03-09 8:54 UTC (permalink / raw) To: Eric Marsden, Lars Ingebrigtsen, Adam Porter; +Cc: 69454 > Date: Wed, 28 Feb 2024 15:29:11 +0100 > From: Eric Marsden <eric.marsden@risk-engineering.org> > > Hello, > > The following generates an error. It seems to me that it would be > preferable to insert the header line and show zero rows for the vtable. > > (require 'vtable) > (make-vtable :columns '("tweedle" "dum") :objects (list)) > > Debugger entered--Lisp error: (wrong-number-of-arguments #<subr max> 0) > max() > apply(max nil) > seq-max(nil) I'm not sure we want to support zero-size vtables. A better error message would be nice, though. What do others think? P.S. Adam, I took the liberty of adding you to this discussion, since you seem lately to be interested in vtable. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-03-09 8:54 ` Eli Zaretskii @ 2024-03-11 19:57 ` Adam Porter 2024-03-14 9:37 ` Eli Zaretskii 2024-04-30 9:10 ` Joost Kremers 1 sibling, 1 reply; 17+ messages in thread From: Adam Porter @ 2024-03-11 19:57 UTC (permalink / raw) To: Eli Zaretskii, Eric Marsden, Lars Ingebrigtsen; +Cc: 69454 Hi Eli, > P.S. Adam, I took the liberty of adding you to this discussion, since > you seem lately to be interested in vtable. Thanks for adding me. Indeed, I've found vtable to be very useful in my new listen.el package. On 3/9/24 02:54, Eli Zaretskii wrote: >> Date: Wed, 28 Feb 2024 15:29:11 +0100 >> From: Eric Marsden <eric.marsden@risk-engineering.org> >> >> Hello, >> >> The following generates an error. It seems to me that it would be >> preferable to insert the header line and show zero rows for the vtable. >> >> (require 'vtable) >> (make-vtable :columns '("tweedle" "dum") :objects (list)) >> >> Debugger entered--Lisp error: (wrong-number-of-arguments #<subr max> 0) >> max() >> apply(max nil) >> seq-max(nil) > > I'm not sure we want to support zero-size vtables. A better error > message would be nice, though. What do others think? I tend to agree with Eric that it would be helpful if vtable could handle having an empty objects collection value to insert, because it saves the application from having to wrap the rather large `make-vtable' form in a `when' block, like here: https://github.com/alphapapa/listen.el/blob/e9ea67350cf3b6cd870561c5e52d4b5255b04d34/listen-queue.el#L135 Also, it's possible that, after inserting a vtable, the collection of objects may be modified so that the collection is empty--then if the the vtable is reverted, it should be able to handle the case of the collection being empty. AFAICT there's not much the application could do to avoid errors in that case, other than working outside of vtable's revert API and calling the function that tested the collection and conditionally inserted the vtable in the first place--in which case the vtable revert API would seem useless. So IMO, when inserting or reverting a vtable, vtable ought to check whether the collection is empty; and if so, handle it gracefully, meaning that an "empty vtable" (whatever that would mean; maybe just one line of text saying that it's an empty collection) would still be inserted, and that if the collection became non-nil, it could be reverted and displayed properly. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-03-11 19:57 ` Adam Porter @ 2024-03-14 9:37 ` Eli Zaretskii 2024-03-16 0:14 ` Adam Porter 0 siblings, 1 reply; 17+ messages in thread From: Eli Zaretskii @ 2024-03-14 9:37 UTC (permalink / raw) To: Adam Porter; +Cc: larsi, 69454, eric.marsden > Date: Mon, 11 Mar 2024 14:57:20 -0500 > Cc: 69454@debbugs.gnu.org > From: Adam Porter <adam@alphapapa.net> > > Hi Eli, > > > P.S. Adam, I took the liberty of adding you to this discussion, since > > you seem lately to be interested in vtable. > > Thanks for adding me. Indeed, I've found vtable to be very useful in my > new listen.el package. > > On 3/9/24 02:54, Eli Zaretskii wrote: > >> Date: Wed, 28 Feb 2024 15:29:11 +0100 > >> From: Eric Marsden <eric.marsden@risk-engineering.org> > >> > >> Hello, > >> > >> The following generates an error. It seems to me that it would be > >> preferable to insert the header line and show zero rows for the vtable. > >> > >> (require 'vtable) > >> (make-vtable :columns '("tweedle" "dum") :objects (list)) > >> > >> Debugger entered--Lisp error: (wrong-number-of-arguments #<subr max> 0) > >> max() > >> apply(max nil) > >> seq-max(nil) > > > > I'm not sure we want to support zero-size vtables. A better error > > message would be nice, though. What do others think? > > I tend to agree with Eric that it would be helpful if vtable could > handle having an empty objects collection value to insert, because it > saves the application from having to wrap the rather large `make-vtable' > form in a `when' block, like here: > > https://github.com/alphapapa/listen.el/blob/e9ea67350cf3b6cd870561c5e52d4b5255b04d34/listen-queue.el#L135 > > Also, it's possible that, after inserting a vtable, the collection of > objects may be modified so that the collection is empty--then if the the > vtable is reverted, it should be able to handle the case of the > collection being empty. > > AFAICT there's not much the application could do to avoid errors in that > case, other than working outside of vtable's revert API and calling the > function that tested the collection and conditionally inserted the > vtable in the first place--in which case the vtable revert API would > seem useless. > > So IMO, when inserting or reverting a vtable, vtable ought to check > whether the collection is empty; and if so, handle it gracefully, > meaning that an "empty vtable" (whatever that would mean; maybe just one > line of text saying that it's an empty collection) would still be > inserted, and that if the collection became non-nil, it could be > reverted and displayed properly. Thanks. Would you or Eric like to submit a patch along these lines? ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-03-14 9:37 ` Eli Zaretskii @ 2024-03-16 0:14 ` Adam Porter 0 siblings, 0 replies; 17+ messages in thread From: Adam Porter @ 2024-03-16 0:14 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, 69454, eric.marsden On 3/14/24 04:37, Eli Zaretskii wrote: >> So IMO, when inserting or reverting a vtable, vtable ought to check >> whether the collection is empty; and if so, handle it gracefully, >> meaning that an "empty vtable" (whatever that would mean; maybe just one >> line of text saying that it's an empty collection) would still be >> inserted, and that if the collection became non-nil, it could be >> reverted and displayed properly. > > Thanks. Would you or Eric like to submit a patch along these lines? I'll put it on my Emacs to-do list, but I don't think I'll be able to make time for it anytime soon. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-03-09 8:54 ` Eli Zaretskii 2024-03-11 19:57 ` Adam Porter @ 2024-04-30 9:10 ` Joost Kremers 2024-04-30 12:14 ` Eli Zaretskii 2024-05-05 12:15 ` Joost Kremers 1 sibling, 2 replies; 17+ messages in thread From: Joost Kremers @ 2024-04-30 9:10 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Adam Porter, Lars Ingebrigtsen, 69454, Eric Marsden On Sat, Mar 09 2024, Eli Zaretskii wrote: >> Date: Wed, 28 Feb 2024 15:29:11 +0100 >> From: Eric Marsden <eric.marsden@risk-engineering.org> >> >> Hello, >> >> The following generates an error. It seems to me that it would be >> preferable to insert the header line and show zero rows for the vtable. >> >> (require 'vtable) >> (make-vtable :columns '("tweedle" "dum") :objects (list)) >> >> Debugger entered--Lisp error: (wrong-number-of-arguments #<subr max> 0) >> max() >> apply(max nil) >> seq-max(nil) > I ran into this same problem myself, trying to use vtable for my package Ebib[1]. I did some digging and found that the cause of the problem is not that the vtable is empty, but rather that the column widths cannot be determined. If you pass explicit widths for each column, `make-vtable` (or rather `vtable-insert`) works just fine with an empty table: ``` (make-vtable :columns '((:name "tweedle" :width 30) (:name "dum" :width 10)) :objects (list)) ``` The error occurs in `vtable--compute-widths`, which returns a vector with the widths of each column. For columns that don't have their width set explicitly, the width is computed on the basis of the elements in the column, but if there are no elements, that fails. > I'm not sure we want to support zero-size vtables. A better error > message would be nice, though. What do others think? For my purpose (i.e., Ebib), support for empty vtables would be a big plus. I wouldn't even want to display some sort of text or warning, just the header and nothing else. (I guess this could be made configurable, though. Something like an :if-empty slot specifying a function to call if the table is empty. This function could then display some text, give a warning in the minibuffer, raise an error, or do nothing at all.) In order to support empty vtables, the column width issue would have to be resolved, of course. My suggestion (again coming from my use-case) would be that if some columns have no :width slot, the remaining available width (i.e., the window width minus the explicit column widths) is divided evenly between them. Of course, that may turn out to be suboptimal once objects are added to the vtable, but I don't think it's unreasonable to expect the programmer to take that into account when using vtable.el. And the user always has the option of regenerating the table. (There's `vtable-revert-command`, after all.) For me, the reason why this would be useful is that the data that I want to display in a vtable has one field that can be very long, while the others are usually fairly short. In my current, custom table implementation, this long field is the right-most column and can thus use the full width of the window to display its data. This works fine with vtable, except if the table is empty. Footnotes: [1] https://github.com/joostkremers/ebib/tree/devel/vtable -- Joost Kremers Life has its moments ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-04-30 9:10 ` Joost Kremers @ 2024-04-30 12:14 ` Eli Zaretskii [not found] ` <8f5fb814-5d88-4ad3-b12a-8246325d5d21@alphapapa.net> 2024-05-05 12:15 ` Joost Kremers 1 sibling, 1 reply; 17+ messages in thread From: Eli Zaretskii @ 2024-04-30 12:14 UTC (permalink / raw) To: adam, Joost Kremers; +Cc: larsi, 69454, eric.marsden > From: Joost Kremers <joostkremers@fastmail.fm> > Cc: Eric Marsden <eric.marsden@risk-engineering.org>, Lars Ingebrigtsen > <larsi@gnus.org>, Adam Porter <adam@alphapapa.net>, > 69454@debbugs.gnu.org > Date: Tue, 30 Apr 2024 11:10:32 +0200 > > In order to support empty vtables, the column width issue would have to be > resolved, of course. My suggestion (again coming from my use-case) would be that > if some columns have no :width slot, the remaining available width (i.e., the > window width minus the explicit column widths) is divided evenly between them. Sounds reasonable, but I don't use vtables. Adam, WDYT? ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <8f5fb814-5d88-4ad3-b12a-8246325d5d21@alphapapa.net>]
* bug#69454: Not possible to insert an empty vtable [not found] ` <8f5fb814-5d88-4ad3-b12a-8246325d5d21@alphapapa.net> @ 2024-05-01 11:54 ` Eli Zaretskii 2024-05-02 7:31 ` Joost Kremers 1 sibling, 0 replies; 17+ messages in thread From: Eli Zaretskii @ 2024-05-01 11:54 UTC (permalink / raw) To: Adam Porter; +Cc: joostkremers, larsi, 69454, eric.marsden > Date: Tue, 30 Apr 2024 18:20:36 -0500 > Cc: eric.marsden@risk-engineering.org, larsi@gnus.org, 69454@debbugs.gnu.org > From: Adam Porter <adam@alphapapa.net> > > >> In order to support empty vtables, the column width issue would have to be > >> resolved, of course. My suggestion (again coming from my use-case) would be that > >> if some columns have no :width slot, the remaining available width (i.e., the > >> window width minus the explicit column widths) is divided evenly between them. > > > > Sounds reasonable, but I don't use vtables. Adam, WDYT? > > Well, I'm not sure how useful it would be to calculate the widths of > columns and show them if there are no data objects to render in them; > especially, if the columns have to be recalculated and the table has to > be re-rendered as soon as objects are shown in it. It would seem to > merely serve as confirmation of which columns are defined in the vtable. > > IOW, it seems that, if there are no objects to show, it might be just as > good to short-circuit the columns and just insert some kind of "[no > objects]" string. That's also reasonable. > I don't have a strong opinion on this, though. I think I would > generally favor whichever approach required the smallest change to the code. Agreed. Patches welcome. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable [not found] ` <8f5fb814-5d88-4ad3-b12a-8246325d5d21@alphapapa.net> 2024-05-01 11:54 ` Eli Zaretskii @ 2024-05-02 7:31 ` Joost Kremers 1 sibling, 0 replies; 17+ messages in thread From: Joost Kremers @ 2024-05-02 7:31 UTC (permalink / raw) To: Adam Porter; +Cc: 69454, Eli Zaretskii, larsi, eric.marsden Hi Adam, On Tue, Apr 30 2024, Adam Porter wrote: > Well, I'm not sure how useful it would be to calculate the widths of columns and > show them if there are no data objects to render in them; especially, if the > columns have to be recalculated and the table has to be re-rendered as soon as > objects are shown in it. Yes, that's what I meant when I said the code using vtable.el would have to take that into consideration. In my case, I do that by leaving only the last column's width undefined. > It would seem to merely serve as confirmation of which > columns are defined in the vtable. Yup, that is exactly right, but I think there are circumstances in which that may be useful. Specifically, cases where the data being displayed is generated by the user. (Think database front-end, even though I realise vtable.el isn't suitable for displaying extremely large amounts of data. In my use-case, the amount of data is manageable, though.) > IOW, it seems that, if there are no objects to show, it might be just as good to > short-circuit the columns and just insert some kind of "[no objects]" string. > > I don't have a strong opinion on this, though. I think I would generally favor > whichever approach required the smallest change to the code. In that case, go with your suggestion, because mine certainly would require bigger changes. 😀 -- Joost Kremers Life has its moments ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-04-30 9:10 ` Joost Kremers 2024-04-30 12:14 ` Eli Zaretskii @ 2024-05-05 12:15 ` Joost Kremers 2024-05-30 21:40 ` Joost Kremers 1 sibling, 1 reply; 17+ messages in thread From: Joost Kremers @ 2024-05-05 12:15 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Adam Porter, Lars Ingebrigtsen, 69454, Eric Marsden On Tue, Apr 30 2024, Joost Kremers wrote: > On Sat, Mar 09 2024, Eli Zaretskii wrote: >>> Date: Wed, 28 Feb 2024 15:29:11 +0100 >>> From: Eric Marsden <eric.marsden@risk-engineering.org> >>> >>> Hello, >>> >>> The following generates an error. It seems to me that it would be >>> preferable to insert the header line and show zero rows for the vtable. >>> >>> (require 'vtable) >>> (make-vtable :columns '("tweedle" "dum") :objects (list)) >>> >>> Debugger entered--Lisp error: (wrong-number-of-arguments #<subr max> 0) >>> max() >>> apply(max nil) >>> seq-max(nil) >> > I ran into this same problem myself, trying to use vtable for my package > Ebib[1]. I did some digging and found that the cause of the problem is not that > the vtable is empty, but rather that the column widths cannot be determined. As I just realised, the problem is actually bigger than that: given the current implementation of vtable-insert-object, it's not possible to add objects to an empty vtable. (Specifically, it uses nconc twice, once on the list of objects and once the cache, both of which fail silently if those are nil, and it uses setcar on the cache, which yields an error if the cache is nil.) Just wanted to mention that here for completeness' sake, if anyone ever decides to fix these issues. For now, the easiest thing to do is probably do disallow empty vtables altogether. -- Joost Kremers Life has its moments ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-05-05 12:15 ` Joost Kremers @ 2024-05-30 21:40 ` Joost Kremers 2024-05-30 21:52 ` Joost Kremers 0 siblings, 1 reply; 17+ messages in thread From: Joost Kremers @ 2024-05-30 21:40 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Adam Porter, Lars Ingebrigtsen, 69454, Eric Marsden [-- Attachment #1: Type: text/plain, Size: 1672 bytes --] Hi all, >>>> Date: Wed, 28 Feb 2024 15:29:11 +0100 >>>> From: Eric Marsden <eric.marsden@risk-engineering.org> >>>> >>>> Hello, >>>> >>>> The following generates an error. It seems to me that it would be >>>> preferable to insert the header line and show zero rows for the vtable. >>>> >>>> (require 'vtable) >>>> (make-vtable :columns '("tweedle" "dum") :objects (list)) >>>> >>>> Debugger entered--Lisp error: (wrong-number-of-arguments #<subr max> 0) >>>> max() >>>> apply(max nil) >>>> seq-max(nil) I did some work on fixing this, and would like to suggest the attached patches. There are three, which I could also squash into a single patch if preferred, but although related, they are conceptually separate, I think. They are: 0001-Allow-empty-vtable.patch This fixes the problem that if `make-vtable` is called without any objects and the widths of (some of) the columns aren't specified, `vtable--compute-widths` would error out. With the patch, columns without an explicit width get assigned equal parts of the remaining window width. 0002-Fix-recomputing-of-vtable-column-alignment.patch This one fixes what I believe to be another bug: `vtable--compute-columns` should be able to recompute the columns, including setting the alignment property. It didn't actually do the latter, though. 0003-Enable-inserting-new-objects-into-empty-vtable.patch This patch makes it possible to insert an object into an empty vtable. It does this by simply recreating and redisplaying the table, which I think makes the most sense. Comments? TIA -- 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 45a84160d5557ee2004fa7e045f52854b9c8e3c5 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/3] 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 cb7ea397314..07ef7d20020 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.45.1 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #3: 0002-Fix-recomputing-of-vtable-column-alignment.patch --] [-- Type: text/x-patch, Size: 1154 bytes --] From 34d179d791c10f1e7b8244542e4b59c58715ed47 Mon Sep 17 00:00:00 2001 From: Joost Kremers <joostkremers@fastmail.com> Date: Thu, 30 May 2024 23:17:37 +0200 Subject: [PATCH 2/3] Fix recomputing of vtable column alignment * lisp/emacs-lisp/vtable.el (vtable--compute-columns): Store new alignment. --- lisp/emacs-lisp/vtable.el | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lisp/emacs-lisp/vtable.el b/lisp/emacs-lisp/vtable.el index 07ef7d20020..b97eaf0e3f3 100644 --- a/lisp/emacs-lisp/vtable.el +++ b/lisp/emacs-lisp/vtable.el @@ -485,11 +485,10 @@ vtable--compute-columns (seq-map-indexed (lambda (column index) ;; This is used when displaying. - (unless (vtable-column-align column) - (setf (vtable-column-align column) - (if (elt numerical index) - 'right - 'left))) + (setf (vtable-column-align column) + (if (elt numerical index) + 'right + 'left)) ;; This is used for sorting. (setf (vtable-column--numerical column) (elt numerical index)) -- 2.45.1 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #4: 0003-Enable-inserting-new-objects-into-empty-vtable.patch --] [-- Type: text/x-patch, Size: 8558 bytes --] From 3c9a3dd95a7a82102bf721843ada3963144636fd Mon Sep 17 00:00:00 2001 From: Joost Kremers <joostkremers@fastmail.com> Date: Thu, 30 May 2024 23:20:00 +0200 Subject: [PATCH 3/3] 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 b97eaf0e3f3..5d152d2d284 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.45.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-05-30 21:40 ` Joost Kremers @ 2024-05-30 21:52 ` Joost Kremers 2024-05-31 5:24 ` Eli Zaretskii 0 siblings, 1 reply; 17+ messages in thread From: Joost Kremers @ 2024-05-30 21:52 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Adam Porter, Lars Ingebrigtsen, 69454, Eric Marsden On Thu, May 30 2024, Joost Kremers wrote: > 0002-Fix-recomputing-of-vtable-column-alignment.patch > > This one fixes what I believe to be another bug: `vtable--compute-columns` > should be able to recompute the columns, including setting the alignment > property. It didn't actually do the latter, though. Actually, forget about this one. I just realised that `vtable--compute-columns` doesn't override the alignment property because it may have been set explicitly in `make-vtable`. The actual problem is that if a table is created without data, the alignment property of each column is set to `right`, even though there's no reason to do so. I'll try and come up with a better patch. -- Joost Kremers Life has its moments ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-05-30 21:52 ` Joost Kremers @ 2024-05-31 5:24 ` Eli Zaretskii 2024-05-31 6:54 ` Joost Kremers 0 siblings, 1 reply; 17+ messages in thread From: Eli Zaretskii @ 2024-05-31 5:24 UTC (permalink / raw) To: Joost Kremers; +Cc: adam, larsi, 69454, eric.marsden > From: Joost Kremers <joostkremers@fastmail.fm> > Cc: Adam Porter <adam@alphapapa.net>, Lars Ingebrigtsen <larsi@gnus.org>, > 69454@debbugs.gnu.org, Eric Marsden <eric.marsden@risk-engineering.org> > Date: Thu, 30 May 2024 23:52:20 +0200 > > On Thu, May 30 2024, Joost Kremers wrote: > > 0002-Fix-recomputing-of-vtable-column-alignment.patch > > > > This one fixes what I believe to be another bug: `vtable--compute-columns` > > should be able to recompute the columns, including setting the alignment > > property. It didn't actually do the latter, though. > > Actually, forget about this one. I just realised that `vtable--compute-columns` > doesn't override the alignment property because it may have been set explicitly > in `make-vtable`. The actual problem is that if a table is created without data, > the alignment property of each column is set to `right`, even though there's no > reason to do so. > > I'll try and come up with a better patch. Thanks for working on this. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-05-31 5:24 ` Eli Zaretskii @ 2024-05-31 6:54 ` Joost Kremers 2024-06-02 17:49 ` Adam Porter 0 siblings, 1 reply; 17+ messages in thread From: Joost Kremers @ 2024-05-31 6:54 UTC (permalink / raw) To: Eli Zaretskii; +Cc: adam, larsi, 69454, eric.marsden [-- Attachment #1: Type: text/plain, Size: 1313 bytes --] >> From: Joost Kremers <joostkremers@fastmail.fm> >> Actually, forget about this one. I just realised that >> `vtable--compute-columns` doesn't override the alignment property because it >> may have been set explicitly in `make-vtable`. The actual problem is that if >> a table is created without data, the alignment property of each column is set >> to `right`, even though there's no reason to do so. Actually, that's not entirely true. The problem was that once the 'align' property was set, it could not be changed anymore. ('vtable-insert-object' assumed that it could, though.) The new patch makes this possible, but only if 'align' wasn't set explicitly in the call to 'make-vtable'. I'm including all three patches here, even though two of them haven't changed: 0001-Allow-empty-vtable.patch : same as before 0002-Enable-inserting-new-objects-into-empty-vtable.patch : same as before (though it was 0003 then) 0003-vtable-allow-resetting-column-alignment-when-table-d.patch : new, as described above. I haven't updated the documentation yet nor did I add a NEWS entry, because I first wanted to make sure you agree with the direction of this patch: Adam suggested empty vtables should not be allowed, but this patch explicitly allows them. Thanks, 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 45a84160d5557ee2004fa7e045f52854b9c8e3c5 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/3] 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 cb7ea397314..07ef7d20020 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.45.1 [-- 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 734ea02bc4dc90d29f94f595271bff40f05b7752 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/3] 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 07ef7d20020..c86ae7f0955 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.45.1 [-- 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 df8588c0cf589190900edda3a0082c0ac7ad0b74 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/3] 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 c86ae7f0955..3e9f5214db0 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.45.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-05-31 6:54 ` Joost Kremers @ 2024-06-02 17:49 ` Adam Porter 2024-06-03 12:13 ` Joost Kremers 0 siblings, 1 reply; 17+ messages in thread From: Adam Porter @ 2024-06-02 17:49 UTC (permalink / raw) To: Joost Kremers, Eli Zaretskii; +Cc: larsi, 69454, eric.marsden On 5/31/24 01:54, Joost Kremers wrote: > I haven't updated the documentation yet nor did I add a NEWS entry, because I > first wanted to make sure you agree with the direction of this patch: Adam > suggested empty vtables should not be allowed, but this patch explicitly allows > them. IIRC I only suggested that because it would mean fewer changes to the code, but if you've already written code to allow it, I don't object. :) Thanks for working on these issues. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-06-02 17:49 ` Adam Porter @ 2024-06-03 12:13 ` Joost Kremers 2024-06-08 12:34 ` Eli Zaretskii 0 siblings, 1 reply; 17+ messages in thread From: Joost Kremers @ 2024-06-03 12:13 UTC (permalink / raw) To: Adam Porter; +Cc: Eli Zaretskii, eric.marsden, larsi, 69454 [-- Attachment #1: Type: text/plain, Size: 293 bytes --] On Sun, Jun 02 2024, Adam Porter wrote: > IIRC I only suggested that because it would mean fewer changes to the code, but > if you've already written code to allow it, I don't object. :) In that case, I'm providing the same patch here, with an additional update for vtable.texi and NEWS: [-- 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 a87d2fc4637a058fad479b4ba5653947bdbb82bf 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 cb7ea397314..07ef7d20020 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.45.2 [-- 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 36b0fb11b27d8f6246a4683462823088c562b146 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 07ef7d20020..c86ae7f0955 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.45.2 [-- 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 63b47044325bc8d7357b6536d7575a5a73bbeb08 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 c86ae7f0955..3e9f5214db0 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.45.2 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #5: 0004-Update-vtable-documentation-and-NEWS.patch --] [-- Type: text/x-patch, Size: 2614 bytes --] From ebfc7ae51895d7dc468c737f2fe403fbd398d5e8 Mon Sep 17 00:00:00 2001 From: Joost Kremers <joostkremers@fastmail.com> Date: Mon, 3 Jun 2024 14:07:43 +0200 Subject: [PATCH 4/4] Update vtable documentation and NEWS * doc/misc/vtable.texi: Add note about empty vtables; add note about column width in empty vtables. * etc/NEWS: Add note about empty vtables. --- doc/misc/vtable.texi | 11 +++++++++++ etc/NEWS | 8 ++++++++ 2 files changed, 19 insertions(+) diff --git a/doc/misc/vtable.texi b/doc/misc/vtable.texi index 6003435385f..061547f5deb 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. @@ -569,6 +578,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 5a1f7f3e443..7089b27ed75 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -2806,6 +2806,14 @@ this was not possible.) In addition, LOCATION can be an integer, a (zero-based) index into the table at which the new object is inserted (BEFORE is ignored in this case). +** 'make-vtable' can create 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. + ** JSON --- -- 2.45.2 [-- Attachment #6: Type: text/plain, Size: 43 bytes --] -- Joost Kremers Life has its moments ^ permalink raw reply related [flat|nested] 17+ messages in thread
* bug#69454: Not possible to insert an empty vtable 2024-06-03 12:13 ` Joost Kremers @ 2024-06-08 12:34 ` Eli Zaretskii 0 siblings, 0 replies; 17+ messages in thread From: Eli Zaretskii @ 2024-06-08 12:34 UTC (permalink / raw) To: Joost Kremers; +Cc: adam, larsi, 69454, eric.marsden > From: Joost Kremers <joostkremers@fastmail.fm> > Cc: Eli Zaretskii <eliz@gnu.org>, larsi@gnus.org, 69454@debbugs.gnu.org, > eric.marsden@risk-engineering.org > Date: Mon, 03 Jun 2024 14:13:55 +0200 > > * lisp/emacs-lisp/vtable.el (vtable--compute-columns): if a column was Please begin the description with a capital letter. > 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, ^^ Please leave two spaces between sentences, per our conventions. > --- a/etc/NEWS > +++ b/etc/NEWS > @@ -2806,6 +2806,14 @@ this was not possible.) In addition, LOCATION can be an integer, a > (zero-based) index into the table at which the new object is inserted > (BEFORE is ignored in this case). > > +** 'make-vtable' can create empty vtable This entry should be marked with "+++", since you have updated the manual. Thanks. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-06-08 12:34 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-28 14:29 bug#69454: Not possible to insert an empty vtable Eric Marsden 2024-03-09 8:54 ` Eli Zaretskii 2024-03-11 19:57 ` Adam Porter 2024-03-14 9:37 ` Eli Zaretskii 2024-03-16 0:14 ` Adam Porter 2024-04-30 9:10 ` Joost Kremers 2024-04-30 12:14 ` Eli Zaretskii [not found] ` <8f5fb814-5d88-4ad3-b12a-8246325d5d21@alphapapa.net> 2024-05-01 11:54 ` Eli Zaretskii 2024-05-02 7:31 ` Joost Kremers 2024-05-05 12:15 ` Joost Kremers 2024-05-30 21:40 ` Joost Kremers 2024-05-30 21:52 ` Joost Kremers 2024-05-31 5:24 ` Eli Zaretskii 2024-05-31 6:54 ` Joost Kremers 2024-06-02 17:49 ` Adam Porter 2024-06-03 12:13 ` Joost Kremers 2024-06-08 12:34 ` Eli Zaretskii
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.