all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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
  0 siblings, 0 replies; 14+ 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] 14+ messages in thread

end of thread, other threads:[~2024-05-31  6:54 UTC | newest]

Thread overview: 14+ 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

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.