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

From: Adam Porter <[email-masked]>
Date: Tue, 15 Oct 2024 23:12:08 -0500

Hi Joost,

On 10/14/24 06:25, Joost Kremers wrote:
> On Sun, Oct 13 2024, Eli Zaretskii wrote:
>>> Date: Sat, 12 Oct 2024 19:12:50 -0500
>>> Cc: [email-masked]
>>> From: Adam Porter <[email-masked]>
>>>
>>>> Adam, any comments or suggestions?
>>>
>>> I may be mistaken, but this seems like a duplicate of bug#69454.
>>
>> II had a vague memory this was already discussed, but couldn't find
>> such a bug.  Thanks for pointing this out.
>>
>> We seem to have dropped the ball on that one.  Joost, would you please
>> rebase on the current master and resubmit?
> 
> Here is the patch again, rebased on current master (or what was current
> master earlier today... :D )
> 
> As before, there are three patches, because they solve different issues
> that stood in the way of having empty vtables. I could squash them into
> one, of course. Also, the documentation and NEWS update are separate.

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.

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

@@ -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".  :)