bug#73775: 30.0.90; vtable: can't handle 0 data rows

[Eli Zaretskii <eliz@gnu.org>]

> From: Joost Kremers <joostkremers@fastmail.fm>
> Cc: Eli Zaretskii <eliz@gnu.org>,  arstoffel@gmail.com,  73775@debbugs.gnu.org
> Date: Wed, 16 Oct 2024 18:19:41 +0200
> 
> Hi Adam,
> 
> Thanks for looking at this.
> 
> On Tue, Oct 15 2024, Adam Porter wrote:
> > Just a few minor suggestions:
> >
> > +  "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)
> >
> > I may be mistaken (as I haven't examined all of the relevant code), but if
> > CACHE is nil when this function is called, won't it always be null? If so,
> > you could check its value once, at first, rather than each time through
> > this loop.
> 
> Unfortunately, it has to be checked anew in every iteration, because it
> determines for each column individually if we need to (temporarily) set its
> width to 0. It also needs to keep the following `seq-max` from erroring out
> (due to `seq-map` returning `nil`).
> 
> > +    ;; 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)))
> >
> > You could use SEQ-COUNT here, which would avoid consing a new list.
> 
> There may even be a better way. If I keep track of the number of zero-width
> columns in the loop above, I don't even need to count them here. I've
> implemented that in the updated patch attached to this message.
> 
> > @@ -484,3 +495,8 @@ 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))))
> >
> > This could be a WHEN instead of a "one-armed IF".  :)
> 
> Yes, sirree! (I don't really agree with the "one-armed if should be
> when"-stance, but I'd be hard-pressed to say when I prefer "if" and when
> "when", and it's hardly a hill I want to die on, so I made the change. 😆 )

Thanks.  I wanted to install this, but at least the first patch no
longer applies; could you please rebase and resubmit?  When you do,
please mention the bug number in all the commit log messages.

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

This should be reformatted according to our conventions: start with a
capital letter, leave two spaces between sentences, and not to indent
lines.

> +** 'make-vtable' can create an empty vtable

This should end in a period.  Also, please mark the entry with "+++",
since the manuals were updated.