bug#69454: Not possible to insert an empty vtable

bug#69454: Not possible to insert an empty vtable

[Eric Marsden]

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

bug#69454: Not possible to insert an empty vtable

[Eli Zaretskii]

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

bug#69454: Not possible to insert an empty vtable

[Adam Porter]

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.

bug#69454: Not possible to insert an empty vtable

[Eli Zaretskii]

> 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?

bug#69454: Not possible to insert an empty vtable

[Adam Porter]

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.

bug#69454: Not possible to insert an empty vtable

[Joost Kremers]

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

bug#69454: Not possible to insert an empty vtable

[Eli Zaretskii]

> 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?

bug#69454: Not possible to insert an empty vtable

[Eli Zaretskii]

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

bug#69454: Not possible to insert an empty vtable

[Joost Kremers]

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

bug#69454: Not possible to insert an empty vtable

[Joost Kremers]

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