GNU bug report logs - #69837
29.2; vtable-update-object only works in visible windows

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 69837 in the body.
You can then email your comments to 69837 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Adam Porter <adam <at> alphapapa.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.2; vtable-update-object only works in visible windows
Date: Sat, 16 Mar 2024 22:41:37 -0500

Hi,

I've discovered that `vtable-update-object' only works in buffers that 
are in visible windows.  When trying to update vtables in buffers that 
aren't, the object being updated or replaced fails to be found in the 
cache, apparently because `vtable--cache-key' uses `window-width' in its 
value (so maybe if, when the vtable buffer is not visible, the selected 
window happens to have the same width as the window in which the vtable 
buffer was previously displayed, it will work by chance).

In my case, the vtable is being updated in the background from a timer, 
so its buffer may or may not be visible.  It's preferable to call 
`vtable-update-object' to update two rows in it rather than reverting 
the whole table, because it can be relatively slow to revert the whole 
table when it's large.

I'm not sure of the best way to fix this.  Maybe the vtable's last-used 
window width could be cached and used for the cache key in case its 
buffer is not visible.  I guess that would risk displaying the vtable 
sub-optimally if the next time it's displayed its window width is 
different, but that might be worth it; in that case, the user could 
always revert the table if necessary.

I'm willing to work on a patch for this, but I'd appreciate any 
guidance, since I'm far from an expert on this library, and there are 
many nuances when it comes to buffers, windows, their attributes, etc.

Thanks,
Adam

Message #8 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Adam Porter <adam <at> alphapapa.net>
Cc: 69837 <at> debbugs.gnu.org
Subject: Re: bug#69837: 29.2;
 vtable-update-object only works in visible windows
Date: Sun, 17 Mar 2024 08:25:46 +0200

> Date: Sat, 16 Mar 2024 22:41:37 -0500
> From: Adam Porter <adam <at> alphapapa.net>
> 
> I've discovered that `vtable-update-object' only works in buffers that 
> are in visible windows.  When trying to update vtables in buffers that 
> aren't, the object being updated or replaced fails to be found in the 
> cache, apparently because `vtable--cache-key' uses `window-width' in its 
> value (so maybe if, when the vtable buffer is not visible, the selected 
> window happens to have the same width as the window in which the vtable 
> buffer was previously displayed, it will work by chance).

Does using with-selected-window help to solve the issue?

If not, can you show a recipe, starting from "emacs -Q", that
reproduces the problem, so we could study it in more detail?

Message #11 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Adam Porter <adam <at> alphapapa.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 69837 <at> debbugs.gnu.org
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows
Date: Sun, 17 Mar 2024 21:05:56 -0500

Hi Eli,

On 3/17/24 01:25, Eli Zaretskii wrote:
>> Date: Sat, 16 Mar 2024 22:41:37 -0500
>> From: Adam Porter <adam <at> alphapapa.net>
>>
>> I've discovered that `vtable-update-object' only works in buffers that
>> are in visible windows.  When trying to update vtables in buffers that
>> aren't, the object being updated or replaced fails to be found in the
>> cache, apparently because `vtable--cache-key' uses `window-width' in its
>> value (so maybe if, when the vtable buffer is not visible, the selected
>> window happens to have the same width as the window in which the vtable
>> buffer was previously displayed, it will work by chance).
> 
> Does using with-selected-window help to solve the issue?

Sometimes, but not reliably, because it doesn't matter whether the 
window is selected (and the buffer might not have a window, anyway). 
I'll try to give a step-by-step explanation:

1. vtable-update-object looks in the vtable's cache to find the cached 
information about the representation of the object being updated or 
replaced.

2. In the process of doing that, it calls vtable--cache-key, which 
returns a cons cell containing the frame-terminal and window-width.

3. So if the selected frame is on the same terminal, and the selected 
window has the same width, as the ones when the vtable was last 
generated, the cache key will match.  This will allow 
vtable-update-object to access the cached values and look for the object 
in them.

But if the terminal is different, or if the selected window's width is 
different, the cache key will be different, so vtable-update-object will 
fail, even when it could potentially succeed.

In my case, the vtable's buffer is (or can be) in a window that is in a 
non-current tab (using tab-bar-mode) in the same frame, so its window is 
not visible.  And so if the selected window in the current tab has the 
same width as the vtable's window, vtable-update-object may work.  But 
if the widths don't match, it will fail.

(I suppose it may also fail if the vtable's buffer's window is visible 
but has changed width since the vtable was generated, but I haven't 
tested that.)

> If not, can you show a recipe, starting from "emacs -Q", that
> reproduces the problem, so we could study it in more detail?

I had hoped to avoid writing that much code to demonstrate it.  But if 
the explanation above doesn't suffice, let me know and I will.

Thanks,
Adam

Message #14 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Adam Porter <adam <at> alphapapa.net>
Cc: 69837 <at> debbugs.gnu.org
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows
Date: Mon, 18 Mar 2024 19:05:17 +0200

> Date: Sun, 17 Mar 2024 21:05:56 -0500
> Cc: 69837 <at> debbugs.gnu.org
> From: Adam Porter <adam <at> alphapapa.net>
> 
> Hi Eli,
> 
> On 3/17/24 01:25, Eli Zaretskii wrote:
> >> Date: Sat, 16 Mar 2024 22:41:37 -0500
> >> From: Adam Porter <adam <at> alphapapa.net>
> >>
> >> I've discovered that `vtable-update-object' only works in buffers that
> >> are in visible windows.  When trying to update vtables in buffers that
> >> aren't, the object being updated or replaced fails to be found in the
> >> cache, apparently because `vtable--cache-key' uses `window-width' in its
> >> value (so maybe if, when the vtable buffer is not visible, the selected
> >> window happens to have the same width as the window in which the vtable
> >> buffer was previously displayed, it will work by chance).
> > 
> > Does using with-selected-window help to solve the issue?
> 
> Sometimes, but not reliably, because it doesn't matter whether the 
> window is selected (and the buffer might not have a window, anyway). 
> I'll try to give a step-by-step explanation:

I'm sorry, I'm not familiar with the design of vtable, so I will have
to ask you possibly stupid questions.  Apologies in advance.

> 1. vtable-update-object looks in the vtable's cache to find the cached 
> information about the representation of the object being updated or 
> replaced.

Is this cache used just to speed up something (and so if the object is
not in the cache, Emacs will just work harder to obtain the same
information), or is finding the object in the cache absolutely
necessary for working with the object?

> 2. In the process of doing that, it calls vtable--cache-key, which 
> returns a cons cell containing the frame-terminal and window-width.

If working with an object needs its window-width, does it mean objects
are tightly coupled to their windows?  If so, what happens when the
user or Emacs resizes the window? are the cached objects recomputed to
reflect that?

> 3. So if the selected frame is on the same terminal, and the selected 
> window has the same width, as the ones when the vtable was last 
> generated, the cache key will match.  This will allow 
> vtable-update-object to access the cached values and look for the object 
> in them.

This again seems to imply that an object is tightly coupled to its
window, and changes affecting the window must recompute the cached
value.  Right?

Are these keys (window-width, terminal, etc.) only used to look up the
object, or are they needed for processing the objects as well?

> > If not, can you show a recipe, starting from "emacs -Q", that
> > reproduces the problem, so we could study it in more detail?
> 
> I had hoped to avoid writing that much code to demonstrate it.  But if 
> the explanation above doesn't suffice, let me know and I will.

It doesn't have to be code, it could be a (possibly long) list of
instructions what to type.

Message #17 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Adam Porter <adam <at> alphapapa.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 69837 <at> debbugs.gnu.org
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows
Date: Tue, 19 Mar 2024 20:41:59 -0500

Hi Eli,

On 3/18/24 12:05, Eli Zaretskii wrote:

> I'm sorry, I'm not familiar with the design of vtable, so I will
> have to ask you possibly stupid questions.  Apologies in advance.
I'm not very familiar with it, either, so I will have to give you
possibly stupid answers.  :)  But I will try to give accurate ones.

>> 1. vtable-update-object looks in the vtable's cache to find the
>> cached information about the representation of the object being
>> updated or replaced.
>
> Is this cache used just to speed up something (and so if the object
> is not in the cache, Emacs will just work harder to obtain the same
> information), or is finding the object in the cache absolutely
> necessary for working with the object?
It appears that the caching is integral to the rendering of vtables.  In
`vtable-insert`, a comment explains:

    ;; We maintain a cache per screen/window width, so that we render
    ;; correctly if Emacs is open on two different screens (or the
    ;; user resizes the frame).

Then `vtable--ensure-cache` is called, which returns either an existing
cache or a newly computed one, and its return value is bound and used
when inserting the table's header line and when inserting each object's
representation.  So it seems that the rendering of objects always uses
the cached values.

>> 2. In the process of doing that, it calls vtable--cache-key, which
>> returns a cons cell containing the frame-terminal and
>> window-width.
>
> If working with an object needs its window-width, does it mean
> objects are tightly coupled to their windows?  If so, what happens
> when the user or Emacs resizes the window? are the cached objects
> recomputed to reflect that?
AFAICT, if a vtable's buffer's window is resized, nothing happens
automatically.  But if the vtable is reverted (causing it to be
completely re-rendered), the window's width is taken into account
compared to the width of the table, affecting the vtable's header line,
and possibly the widths of some columns, depending on the table's
specification (see functions `vtable--compute-width` and
`vtable--compute-widths`).  And if an individual object is updated, it
fails if the window width has changed, since the cache key misses.

I suppose the cache serves to sometimes help avoid re-rendering all the
objects in the table, and it uses the window-width as part of the cache
key because the table's buffer could be shown in multiple windows, each
of which could have a different width, requiring a different rendering
of the table (though I can't say I understand exactly how that could
work, since ISTM that rendering it for one window would display the same
rendering in the other window).

>> 3. So if the selected frame is on the same terminal, and the
>> selected window has the same width, as the ones when the vtable was
>> last generated, the cache key will match.  This will allow
>> vtable-update-object to access the cached values and look for the
>> object in them.
>
> This again seems to imply that an object is tightly coupled to its
> window, and changes affecting the window must recompute the cached
> value.  Right?
I think the way it works is roughly like this:

- An object's value(s) affect the width of the columns in its
representation's row.

- A row's columns' widths (or their total width) is compared with the
width of the window to determine how to display the last column (or the
last one visible in the window?).

- So the inverse holds as well: the window's width affects the display
of (at least one of) an object's representation's columns.

So if an object is being re-rendered, and the window's width is the
same, the cache can be used (which also seems to record the line number
at which the object is rendered, which saves from having to iterate
through the table to find the buffer line).  Otherwise, if the window's
width has changed, the cache misses; and since `vtable-update-object`
does not handle this case (it just causes an error), the object is not
updated.  This would require the application to handle the error and
revert the whole table instead.

I would suppose that, in that case, the last-used window width (if it
were known) could be used to update the object, assuming that the width
is either the same or "good enough," to avoid having to re-render the
whole table; if the worst that happened is that a column at the edge of
the table were a bit too wide or narrow compared to the ideal, that
would often be preferable to re-rendering the whole table.  (In my case,
I could have 1,000-2,000 rows in the table, in which case I would prefer
to avoid re-rendering the whole thing; leaving the columns at their
existing width is no problem, especially if some of them are already
past the width of the window--and in my case, the table is always wider
than the window.)

> Are these keys (window-width, terminal, etc.) only used to look up
> the object, or are they needed for processing the objects as well?
The window's width appears to be integral to the rendering of the
objects, both individually and when inserting the whole table at once.
The terminal is used as part of the cache key, probably because a
different display could have a different resolution (DPI), which would
make calculations on one invalid on another (I'm sure you know more
about how that works than I do).

>>> If not, can you show a recipe, starting from "emacs -Q", that
>>> reproduces the problem, so we could study it in more detail?
>>
>> I had hoped to avoid writing that much code to demonstrate it.  But
>> if the explanation above doesn't suffice, let me know and I will.
>
> It doesn't have to be code, it could be a (possibly long) list of
> instructions what to type.
In my case, I'm using vtable in my `listen` package on ELPA.  If I, e.g.
add all the tracks in my music library to a `listen-queue`, which is
rendered with vtable, it's helpful to be able to update just one track's
representation at a time (so that the currently playing track can have
an arrow next to it, and the previously playing one can have its arrow
removed), because rendering the whole table can take a few seconds.
(Smaller tables, e.g. a hundred tracks or so, render very quickly.)

If a concrete demo is still needed, let me know and I can hack something up.

Thanks,
Adam

Message #20 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Adam Porter <adam <at> alphapapa.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 69837 <at> debbugs.gnu.org
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows
Date: Thu, 21 Mar 2024 03:36:02 -0500

[Message part 1 (text/plain, inline)]

Hi Eli,

FWIW, I did some hacking on listen.el attempting to further understand 
and work around this problem, and I've found what seems to be a usable 
workaround (for my purposes, anyway).

The attached code defines a macro within which I call 
`listen-queue--vtable-update-object' (which merely incorporates the fix 
to `vtable-update-object' from bug#69664).  As well, I wrap 
`make-vtable' in a function that also sets two buffer-local variables to 
the values of `frame-terminal' and `window-width' at the time of the 
vtable's creation.  The macro locally overrides the functions 
`frame-terminal' and `window-width' to return those saved values.

This allows the cache key to match unconditionally, which allows the 
vtable's objects to be updated even when its buffer is not visible.

In my limited testing, it seems to work fine.  In my estimation, the 
consequences of doing this in the worst case would be that the rows for 
the updated objects might be drawn with some columns at a slightly 
incorrect width, which is easily rectified by reverting the table 
(usually bound to "g").  As well, that worst case (e.g. imagining a 
vtable whose buffer might be initially displayed on one terminal/monitor 
and later on another with different characteristics) would seem to be 
relatively rare (so for my project, it seems like an obviously good 
thing to do).

For Emacs itself, I'm not sure what the best fix would be.  I suppose a 
workaround like this could be implemented as a fallback in case the 
cache key misses; it would seem better to update the object potentially 
sub-optimally than to error and not update it at all.

Another possibility would be to ignore the frame-terminal and 
window-width in the cache key altogether (i.e. so they would always be 
assumed to be the same), but I'm sure that Lars did it this way for a 
reason, so that would seem unwise.

Let me know how you'd like me to proceed.

Thanks,
Adam

[example.el (text/x-emacs-lisp, attachment)]

Message #23 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Adam Porter <adam <at> alphapapa.net>
Cc: 69837 <at> debbugs.gnu.org
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows
Date: Sat, 06 Apr 2024 14:22:53 +0300

> Date: Thu, 21 Mar 2024 03:36:02 -0500
> Cc: 69837 <at> debbugs.gnu.org
> From: Adam Porter <adam <at> alphapapa.net>
> 
> FWIW, I did some hacking on listen.el attempting to further understand 
> and work around this problem, and I've found what seems to be a usable 
> workaround (for my purposes, anyway).
> 
> The attached code defines a macro within which I call 
> `listen-queue--vtable-update-object' (which merely incorporates the fix 
> to `vtable-update-object' from bug#69664).  As well, I wrap 
> `make-vtable' in a function that also sets two buffer-local variables to 
> the values of `frame-terminal' and `window-width' at the time of the 
> vtable's creation.  The macro locally overrides the functions 
> `frame-terminal' and `window-width' to return those saved values.
> 
> This allows the cache key to match unconditionally, which allows the 
> vtable's objects to be updated even when its buffer is not visible.
> 
> In my limited testing, it seems to work fine.  In my estimation, the 
> consequences of doing this in the worst case would be that the rows for 
> the updated objects might be drawn with some columns at a slightly 
> incorrect width, which is easily rectified by reverting the table 
> (usually bound to "g").  As well, that worst case (e.g. imagining a 
> vtable whose buffer might be initially displayed on one terminal/monitor 
> and later on another with different characteristics) would seem to be 
> relatively rare (so for my project, it seems like an obviously good 
> thing to do).
> 
> For Emacs itself, I'm not sure what the best fix would be.  I suppose a 
> workaround like this could be implemented as a fallback in case the 
> cache key misses; it would seem better to update the object potentially 
> sub-optimally than to error and not update it at all.
> 
> Another possibility would be to ignore the frame-terminal and 
> window-width in the cache key altogether (i.e. so they would always be 
> assumed to be the same), but I'm sure that Lars did it this way for a 
> reason, so that would seem unwise.
> 
> Let me know how you'd like me to proceed.

I think you should install your workaround.  I don't see how it could
be worse than what we have now.

P.S. And sorry for a long silence.

Message #26 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org
Subject: Re: bug#69837: 29.2;
 vtable-update-object only works in visible windows
Date: Fri, 28 Feb 2025 19:09:29 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Thu, 21 Mar 2024 03:36:02 -0500
>> Cc: 69837 <at> debbugs.gnu.org
>> From: Adam Porter <adam <at> alphapapa.net>
>>
>> FWIW, I did some hacking on listen.el attempting to further understand
>> and work around this problem, and I've found what seems to be a usable
>> workaround (for my purposes, anyway).
>>
>> The attached code defines a macro within which I call
>> `listen-queue--vtable-update-object' (which merely incorporates the fix
>> to `vtable-update-object' from bug#69664).  As well, I wrap
>> `make-vtable' in a function that also sets two buffer-local variables to
>> the values of `frame-terminal' and `window-width' at the time of the
>> vtable's creation.  The macro locally overrides the functions
>> `frame-terminal' and `window-width' to return those saved values.
>>
>> This allows the cache key to match unconditionally, which allows the
>> vtable's objects to be updated even when its buffer is not visible.
>>
>> In my limited testing, it seems to work fine.  In my estimation, the
>> consequences of doing this in the worst case would be that the rows for
>> the updated objects might be drawn with some columns at a slightly
>> incorrect width, which is easily rectified by reverting the table
>> (usually bound to "g").  As well, that worst case (e.g. imagining a
>> vtable whose buffer might be initially displayed on one terminal/monitor
>> and later on another with different characteristics) would seem to be
>> relatively rare (so for my project, it seems like an obviously good
>> thing to do).
>>
>> For Emacs itself, I'm not sure what the best fix would be.  I suppose a
>> workaround like this could be implemented as a fallback in case the
>> cache key misses; it would seem better to update the object potentially
>> sub-optimally than to error and not update it at all.
>>
>> Another possibility would be to ignore the frame-terminal and
>> window-width in the cache key altogether (i.e. so they would always be
>> assumed to be the same), but I'm sure that Lars did it this way for a
>> reason, so that would seem unwise.
>>
>> Let me know how you'd like me to proceed.
>
> I think you should install your workaround.  I don't see how it could
> be worse than what we have now.
>
> P.S. And sorry for a long silence.

Adam, could you please install the workaround?  Or maybe you did
already?

Message #31 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows
Date: Thu, 19 Jun 2025 17:36:36 -0400

[Message part 1 (text/plain, inline)]

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> Date: Thu, 21 Mar 2024 03:36:02 -0500
>>> Cc: 69837 <at> debbugs.gnu.org
>>> From: Adam Porter <adam <at> alphapapa.net>
>>>
>>> FWIW, I did some hacking on listen.el attempting to further understand
>>> and work around this problem, and I've found what seems to be a usable
>>> workaround (for my purposes, anyway).
>>>
>>> The attached code defines a macro within which I call
>>> `listen-queue--vtable-update-object' (which merely incorporates the fix
>>> to `vtable-update-object' from bug#69664).  As well, I wrap
>>> `make-vtable' in a function that also sets two buffer-local variables to
>>> the values of `frame-terminal' and `window-width' at the time of the
>>> vtable's creation.  The macro locally overrides the functions
>>> `frame-terminal' and `window-width' to return those saved values.
>>>
>>> This allows the cache key to match unconditionally, which allows the
>>> vtable's objects to be updated even when its buffer is not visible.
>>>
>>> In my limited testing, it seems to work fine.  In my estimation, the
>>> consequences of doing this in the worst case would be that the rows for
>>> the updated objects might be drawn with some columns at a slightly
>>> incorrect width, which is easily rectified by reverting the table
>>> (usually bound to "g").  As well, that worst case (e.g. imagining a
>>> vtable whose buffer might be initially displayed on one terminal/monitor
>>> and later on another with different characteristics) would seem to be
>>> relatively rare (so for my project, it seems like an obviously good
>>> thing to do).
>>>
>>> For Emacs itself, I'm not sure what the best fix would be.  I suppose a
>>> workaround like this could be implemented as a fallback in case the
>>> cache key misses; it would seem better to update the object potentially
>>> sub-optimally than to error and not update it at all.
>>>
>>> Another possibility would be to ignore the frame-terminal and
>>> window-width in the cache key altogether (i.e. so they would always be
>>> assumed to be the same), but I'm sure that Lars did it this way for a
>>> reason, so that would seem unwise.
>>>
>>> Let me know how you'd like me to proceed.
>>
>> I think you should install your workaround.  I don't see how it could
>> be worse than what we have now.
>>
>> P.S. And sorry for a long silence.
>
> Adam, could you please install the workaround?  Or maybe you did
> already?

I ran into this same problem.  I think Adam's workaround is on the right
track, but the issue is present in a number of other functions in
vtable.el, not just vtable-update-object.

The root of the issue is that when we're interacting with the text of an
inserted vtable, we should use the "cache" object that was last used to
insert that vtable, rather than one based on the current selected window
and terminal.  Otherwise we will experience various inconsistencies,
e.g. inserting a new line that has different column widths from the rest
of the vtable text.

My attached patch solves this by saving the "cache" object in a text
property on the vtable text, and updating all the relevant vtable
functions to access the cache via that text property rather than by
computing a cache key from the current selected window/terminal.

Adam, could you check that this solves the problem for your use case?

[0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch (text/x-patch, inline)]

From cdde849bf78c20dbc3f5c3abba0e86171c8de188 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Thu, 19 Jun 2025 17:20:12 -0400
Subject: [PATCH] Fix implicit usage of the current window-width in vtable.el

Previously, many functions in vtable.el called vtable--cache,
which computed vtable--cache-key based on the current selected
window and frame; this could cause vtable functions to fail or
misbehave if they were not called from the selected window and
frame that vtable-insert was last called in.

Now, the vtable cache is stored with the text of the vtable, so
that functions which need to interact with some vtable text can
do so reliably without having to use the same selected window
and frame.

Also, vtable-update-object has always required TABLE to be
present at point in the current buffer; now its docstring states
this.

* lisp/emacs-lisp/vtable.el (vtable--current-cache)
(vtable--cache-widths, vtable--cache-lines): Add.
(vtable-insert): Save cache in 'vtable-cache.
(vtable--ensure-cache, vtable--recompute-cache): Inline into
vtable-insert.
(vtable--widths, vtable--cache): Delete.
(vtable-update-object): Use vtable--current-cache and update
docstring.  (bug#69837)
(vtable-remove-object, vtable-insert-object): Use
vtable--current-cache and save cache in 'vtable-cache.
(vtable--sort, vtable--alter-column-width)
(vtable-previous-column, vtable-next-column): Use
vtable--current-cache.
---
 lisp/emacs-lisp/vtable.el | 111 ++++++++++++++++++++------------------
 1 file changed, 59 insertions(+), 52 deletions(-)

diff --git a/lisp/emacs-lisp/vtable.el b/lisp/emacs-lisp/vtable.el
index 00785113edb..991570b9799 100644
--- a/lisp/emacs-lisp/vtable.el
+++ b/lisp/emacs-lisp/vtable.el
@@ -282,10 +282,9 @@ vtable-update-object
   "Update OBJECT's representation in TABLE.
 If OLD-OBJECT is non-nil, replace OLD-OBJECT with OBJECT and display it.
 In either case, if the existing object is not found in the table (being
-compared with `equal'), signal an error.  Note a limitation: if TABLE's
-buffer is not in a visible window, or if its window has changed width
-since it was updated, updating the TABLE is not possible, and an error
-is signaled."
+compared with `equal'), signal an error.
+
+TABLE must be at point in the current buffer."
   (unless old-object
     (setq old-object object))
   (let* ((objects (vtable-objects table))
@@ -302,14 +301,13 @@ vtable-update-object
       (unless objects
         (error "Can't find the old object"))
       (setcar (cdr objects) object))
-    ;; Then update the cache...
-    ;; FIXME: If the table's buffer has no visible window, or if its
-    ;; width has changed since the table was updated, the cache key will
-    ;; not match and the object can't be updated.  (Bug #69837).
-    (if-let* ((line-number (seq-position (car (vtable--cache table)) old-object
-                                         (lambda (a b)
-                                           (equal (car a) b))))
-              (line (elt (car (vtable--cache table)) line-number)))
+    ;; Then update the rendered vtable in the current buffer.
+    (if-let* ((cache (vtable--current-cache))
+             (line-number (seq-position (vtable--cache-lines cache)
+                                        old-object
+                                        (lambda (a b)
+                                          (equal (car a) b))))
+             (line (elt (vtable--cache-widths cache) line-number)))
         (progn
           (setcar line object)
           (setcdr line (vtable--compute-cached-line table object))
@@ -320,10 +318,11 @@ vtable-update-object
                   (start (point)))
               (delete-line)
               (vtable--insert-line table line line-number
-                                   (nth 1 (vtable--cache table))
+                                   (vtable--cache-lines cache)
                                    (vtable--spacer table))
               (add-text-properties start (point) (list 'keymap keymap
-                                                       'vtable table))))
+                                                       'vtable table
+                                                       'vtable-cache cache))))
           ;; We may have inserted a non-numerical value into a previously
           ;; all-numerical table, so recompute.
           (vtable--recompute-numerical table (cdr line)))
@@ -335,11 +334,12 @@ vtable-remove-object
   ;; First remove from the objects.
   (setf (vtable-objects table) (delq object (vtable-objects table)))
   ;; Then adjust the cache and display.
-  (let ((cache (vtable--cache table))
-        (inhibit-read-only t))
-    (setcar cache (delq (assq object (car cache)) (car cache)))
-    (save-excursion
-      (vtable-goto-table table)
+  (save-excursion
+    (vtable-goto-table table)
+    (let ((cache (vtable--current-cache))
+          (inhibit-read-only t))
+      (setcar cache (delq (assq object (vtable--cache-lines cache))
+                          (vtable--cache-lines cache)))
       (when (vtable-goto-object object)
         (delete-line)))))
 
@@ -400,7 +400,7 @@ vtable-insert-object
     ;; Then adjust the cache and display.
     (save-excursion
       (vtable-goto-table table)
-      (let* ((cache (vtable--cache table))
+      (let* ((cache (vtable--current-cache))
              (inhibit-read-only t)
              (keymap (get-text-property (point) 'keymap))
              (ellipsis (if (vtable-ellipsis table)
@@ -408,13 +408,14 @@ vtable-insert-object
                                        'face (vtable-face table))
                          ""))
              (ellipsis-width (string-pixel-width ellipsis))
-             (elem (if location  ; This binding mirrors the binding of `pos' above.
+             (lines (vtable--cache-lines cache))
+             (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)))
+                           (nth location lines)
+                         (or (assq location lines)
+                             (and before (car lines))))
+                     (if before (car lines))))
+             (pos (memq elem lines))
              (line (cons object (vtable--compute-cached-line table object))))
         (if (or before
                 (and pos (integerp location)))
@@ -433,13 +434,13 @@ vtable-insert-object
                     (forward-line 1)  ; Insert *after*.
                   (vtable-end-of-table)))
             ;; Otherwise, append the object.
-            (setcar cache (nconc (car cache) (list line)))
+            (setcar cache (nconc (vtable--cache-lines 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)
+                               (vtable--cache-widths cache) (vtable--spacer table)
                                ellipsis ellipsis-width)
           (add-text-properties start (point) (list 'keymap keymap
                                                    'vtable table)))
@@ -512,15 +513,11 @@ vtable--compute-columns
 (defun vtable--spacer (table)
   (vtable--compute-width table (vtable-separator-width table)))
 
-(defun vtable--recompute-cache (table)
-  (let* ((data (vtable--compute-cache table))
-         (widths (vtable--compute-widths table data)))
-    (setf (gethash (vtable--cache-key) (slot-value table '-cache))
-          (list data widths))))
+(defun vtable--cache-widths (cache)
+  (nth 1 cache))
 
-(defun vtable--ensure-cache (table)
-  (or (vtable--cache table)
-      (vtable--recompute-cache table)))
+(defun vtable--cache-lines (cache)
+  (car cache))
 
 (defun vtable-insert (table)
   (let* ((spacer (vtable--spacer table))
@@ -533,7 +530,12 @@ vtable-insert
          ;; We maintain a cache per screen/window width, so that we render
          ;; correctly if Emacs is open on two different screens (or the
          ;; user resizes the frame).
-         (widths (nth 1 (vtable--ensure-cache table))))
+         (cache (or (gethash (vtable--cache-key) (slot-value table '-cache))
+                    (let* ((data (vtable--compute-cache table))
+                           (widths (vtable--compute-widths table data)))
+                      (setf (gethash (vtable--cache-key) (slot-value table '-cache))
+                            (list data widths)))))
+         (widths (vtable--cache-widths cache)))
     ;; Don't insert any header or header line if the user hasn't
     ;; specified the columns.
     (when (slot-value table '-has-column-spec)
@@ -546,18 +548,20 @@ vtable-insert
         (add-text-properties start (point)
                              (list 'keymap vtable-header-line-map
                                    'rear-nonsticky t
-                                   'vtable table))
+                                   'vtable table
+                                   'vtable-cache cache))
         (setq start (point))))
-    (vtable--sort table)
+    (vtable--sort table cache)
     ;; Insert the data.
     (let ((line-number 0))
-      (dolist (line (car (vtable--cache table)))
+      (dolist (line (vtable--cache-lines cache))
         (vtable--insert-line table line line-number widths spacer
                              ellipsis ellipsis-width)
         (setq line-number (1+ line-number))))
     (add-text-properties start (point)
                          (list 'rear-nonsticky t
-                               'vtable table))
+                               'vtable table
+                               'vtable-cache cache))
     (goto-char start)))
 
 (defun vtable--insert-line (table line line-number widths spacer
@@ -659,16 +663,22 @@ vtable--insert-line
 (defun vtable--cache-key ()
   (cons (frame-terminal) (window-width)))
 
-(defun vtable--cache (table)
-  (gethash (vtable--cache-key) (slot-value table '-cache)))
+(defun vtable--current-cache ()
+  "Return the current cache for the table at point.
+
+In `vtable-insert', the lines and widths of the vtable text are computed
+based on the current selected frame and window and stored in a cache.
+Subsequent interaction with the text of the vtable should use that cache
+via this function rather than by calling `vtable--cache-key' to look up
+the cache."
+  (get-text-property (point) 'vtable-cache))
 
 (defun vtable--clear-cache (table)
   (setf (gethash (vtable--cache-key) (slot-value table '-cache)) nil))
 
-(defun vtable--sort (table)
+(defun vtable--sort (table cache)
   (pcase-dolist (`(,index . ,direction) (vtable-sort-by table))
-    (let ((cache (vtable--cache table))
-          (numerical (vtable-column--numerical
+    (let ((numerical (vtable-column--numerical
                       (elt (vtable-columns table) index)))
           (numcomp (if (eq direction 'descend)
                        #'> #'<))
@@ -971,9 +981,6 @@ vtable-revert
     (when column
       (vtable-goto-column column))))
 
-(defun vtable--widths (table)
-  (nth 1 (vtable--ensure-cache table)))
-
 ;;; Commands.
 
 (defvar-keymap vtable-header-mode-map
@@ -998,7 +1005,7 @@ vtable-narrow-current-column
                                 (- (* (vtable--char-width table) (or n 1))))))
 
 (defun vtable--alter-column-width (table column delta)
-  (let ((widths (vtable--widths table)))
+  (let ((widths (vtable--cache-widths (vtable--current-cache))))
     (setf (aref widths column)
           (max (* (vtable--char-width table) 2)
                (+ (aref widths column) delta)))
@@ -1020,14 +1027,14 @@ vtable-previous-column
   (interactive)
   (vtable-goto-column
    (max 0 (1- (or (vtable-current-column)
-                  (length (vtable--widths (vtable-current-table))))))))
+                  (length (vtable--cache-widths (vtable--current-cache))))))))
 
 (defun vtable-next-column ()
   "Go to the next column."
   (interactive)
   (when (vtable-current-column)
     (vtable-goto-column
-     (min (1- (length (vtable--widths (vtable-current-table))))
+     (min (1- (length (vtable--cache-widths (vtable--current-cache))))
           (1+ (vtable-current-column))))))
 
 (defun vtable-revert-command ()
-- 
2.39.3

Message #34 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2;
 vtable-update-object only works in visible windows
Date: Thu, 19 Jun 2025 17:41:03 -0400

[Message part 1 (text/plain, inline)]

On Thu, Jun 19, 2025 at 5:37 PM Spencer Baugh via Bug reports for GNU
Emacs, the Swiss army knife of text editors <bug-gnu-emacs <at> gnu.org> wrote:

> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> >>> Date: Thu, 21 Mar 2024 03:36:02 -0500
> >>> Cc: 69837 <at> debbugs.gnu.org
> >>> From: Adam Porter <adam <at> alphapapa.net>
> >>>
> >>> FWIW, I did some hacking on listen.el attempting to further understand
> >>> and work around this problem, and I've found what seems to be a usable
> >>> workaround (for my purposes, anyway).
> >>>
> >>> The attached code defines a macro within which I call
> >>> `listen-queue--vtable-update-object' (which merely incorporates the fix
> >>> to `vtable-update-object' from bug#69664).  As well, I wrap
> >>> `make-vtable' in a function that also sets two buffer-local variables
> to
> >>> the values of `frame-terminal' and `window-width' at the time of the
> >>> vtable's creation.  The macro locally overrides the functions
> >>> `frame-terminal' and `window-width' to return those saved values.
> >>>
> >>> This allows the cache key to match unconditionally, which allows the
> >>> vtable's objects to be updated even when its buffer is not visible.
> >>>
> >>> In my limited testing, it seems to work fine.  In my estimation, the
> >>> consequences of doing this in the worst case would be that the rows for
> >>> the updated objects might be drawn with some columns at a slightly
> >>> incorrect width, which is easily rectified by reverting the table
> >>> (usually bound to "g").  As well, that worst case (e.g. imagining a
> >>> vtable whose buffer might be initially displayed on one
> terminal/monitor
> >>> and later on another with different characteristics) would seem to be
> >>> relatively rare (so for my project, it seems like an obviously good
> >>> thing to do).
> >>>
> >>> For Emacs itself, I'm not sure what the best fix would be.  I suppose a
> >>> workaround like this could be implemented as a fallback in case the
> >>> cache key misses; it would seem better to update the object potentially
> >>> sub-optimally than to error and not update it at all.
> >>>
> >>> Another possibility would be to ignore the frame-terminal and
> >>> window-width in the cache key altogether (i.e. so they would always be
> >>> assumed to be the same), but I'm sure that Lars did it this way for a
> >>> reason, so that would seem unwise.
> >>>
> >>> Let me know how you'd like me to proceed.
> >>
> >> I think you should install your workaround.  I don't see how it could
> >> be worse than what we have now.
> >>
> >> P.S. And sorry for a long silence.
> >
> > Adam, could you please install the workaround?  Or maybe you did
> > already?
>
> I ran into this same problem.  I think Adam's workaround is on the right
> track, but the issue is present in a number of other functions in
> vtable.el, not just vtable-update-object.
>
> The root of the issue is that when we're interacting with the text of an
> inserted vtable, we should use the "cache" object that was last used to
> insert that vtable, rather than one based on the current selected window
> and terminal.  Otherwise we will experience various inconsistencies,
> e.g. inserting a new line that has different column widths from the rest
> of the vtable text.
>
> My attached patch solves this by saving the "cache" object in a text
> property on the vtable text, and updating all the relevant vtable
> functions to access the cache via that text property rather than by
> computing a cache key from the current selected window/terminal.
>
> Adam, could you check that this solves the problem for your use case?
>

I have a very big patch coming for vtable under
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=78843 which addresses this
issue, and a ton of other bugs (and features).  That's coming in the next
day or so, if you can wait?

-Stéphane

[Message part 2 (text/html, inline)]

Message #37 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows
Date: Thu, 19 Jun 2025 18:02:59 -0400

[Message part 1 (text/plain, inline)]

Stéphane Marks <shipmints <at> gmail.com> writes:
> On Thu, Jun 19, 2025 at 5:37 PM Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
> <bug-gnu-emacs <at> gnu.org> wrote:
>
>  Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>  > Eli Zaretskii <eliz <at> gnu.org> writes:
>  >
>  >>> Date: Thu, 21 Mar 2024 03:36:02 -0500
>  >>> Cc: 69837 <at> debbugs.gnu.org
>  >>> From: Adam Porter <adam <at> alphapapa.net>
>  >>>
>  >>> FWIW, I did some hacking on listen.el attempting to further understand
>  >>> and work around this problem, and I've found what seems to be a usable
>  >>> workaround (for my purposes, anyway).
>  >>>
>  >>> The attached code defines a macro within which I call
>  >>> `listen-queue--vtable-update-object' (which merely incorporates the fix
>  >>> to `vtable-update-object' from bug#69664).  As well, I wrap
>  >>> `make-vtable' in a function that also sets two buffer-local variables to
>  >>> the values of `frame-terminal' and `window-width' at the time of the
>  >>> vtable's creation.  The macro locally overrides the functions
>  >>> `frame-terminal' and `window-width' to return those saved values.
>  >>>
>  >>> This allows the cache key to match unconditionally, which allows the
>  >>> vtable's objects to be updated even when its buffer is not visible.
>  >>>
>  >>> In my limited testing, it seems to work fine.  In my estimation, the
>  >>> consequences of doing this in the worst case would be that the rows for
>  >>> the updated objects might be drawn with some columns at a slightly
>  >>> incorrect width, which is easily rectified by reverting the table
>  >>> (usually bound to "g").  As well, that worst case (e.g. imagining a
>  >>> vtable whose buffer might be initially displayed on one terminal/monitor
>  >>> and later on another with different characteristics) would seem to be
>  >>> relatively rare (so for my project, it seems like an obviously good
>  >>> thing to do).
>  >>>
>  >>> For Emacs itself, I'm not sure what the best fix would be.  I suppose a
>  >>> workaround like this could be implemented as a fallback in case the
>  >>> cache key misses; it would seem better to update the object potentially
>  >>> sub-optimally than to error and not update it at all.
>  >>>
>  >>> Another possibility would be to ignore the frame-terminal and
>  >>> window-width in the cache key altogether (i.e. so they would always be
>  >>> assumed to be the same), but I'm sure that Lars did it this way for a
>  >>> reason, so that would seem unwise.
>  >>>
>  >>> Let me know how you'd like me to proceed.
>  >>
>  >> I think you should install your workaround.  I don't see how it could
>  >> be worse than what we have now.
>  >>
>  >> P.S. And sorry for a long silence.
>  >
>  > Adam, could you please install the workaround?  Or maybe you did
>  > already?
>
>  I ran into this same problem.  I think Adam's workaround is on the right
>  track, but the issue is present in a number of other functions in
>  vtable.el, not just vtable-update-object.
>
>  The root of the issue is that when we're interacting with the text of an
>  inserted vtable, we should use the "cache" object that was last used to
>  insert that vtable, rather than one based on the current selected window
>  and terminal.  Otherwise we will experience various inconsistencies,
>  e.g. inserting a new line that has different column widths from the rest
>  of the vtable text.
>
>  My attached patch solves this by saving the "cache" object in a text
>  property on the vtable text, and updating all the relevant vtable
>  functions to access the cache via that text property rather than by
>  computing a cache key from the current selected window/terminal.
>
>  Adam, could you check that this solves the problem for your use case?
>
> I have a very big patch coming for vtable under https://debbugs.gnu.org/cgi/bugreport.cgi?bug=78843 which addresses this issue,
> and a ton of other bugs (and features).  That's coming in the next day or so, if you can wait?

Certainly I can wait.  Cc me when you send the patch.

Though if your patch is very large, maybe this relatively small patch
should land first.

(Also, I hope that you're adding tests in your very large patch?)

BTW, an updated version of my patch is attached, just fixing a silly bug
that was in the first version.

[0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch (text/x-patch, inline)]

From 5607bb42202e050edc5e33b8fa9c1c96d65d8746 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Thu, 19 Jun 2025 17:20:12 -0400
Subject: [PATCH] Fix implicit usage of the current window-width in vtable.el

Previously, many functions in vtable.el called vtable--cache,
which computed vtable--cache-key based on the current selected
window and frame; this could cause vtable functions to fail or
misbehave if they were not called from the selected window and
frame that vtable-insert was last called in.

Now, the vtable cache is stored with the text of the vtable, so
that functions which need to interact with some vtable text can
do so reliably without having to use the same selected window
and frame.

Also, vtable-update-object has always required TABLE to be
present at point in the current buffer; now its docstring states
this.

* lisp/emacs-lisp/vtable.el (vtable--current-cache)
(vtable--cache-widths, vtable--cache-lines): Add.
(vtable-insert): Save cache in 'vtable-cache.
(vtable--ensure-cache, vtable--recompute-cache): Inline into
vtable-insert.
(vtable--widths, vtable--cache): Delete.
(vtable-update-object): Use vtable--current-cache and update
docstring.  (bug#69837)
(vtable-remove-object, vtable-insert-object): Use
vtable--current-cache and save cache in 'vtable-cache.
(vtable--sort, vtable--alter-column-width)
(vtable-previous-column, vtable-next-column): Use
vtable--current-cache.
---
 lisp/emacs-lisp/vtable.el | 111 ++++++++++++++++++++------------------
 1 file changed, 59 insertions(+), 52 deletions(-)

diff --git a/lisp/emacs-lisp/vtable.el b/lisp/emacs-lisp/vtable.el
index 00785113edb..d6be6d361f7 100644
--- a/lisp/emacs-lisp/vtable.el
+++ b/lisp/emacs-lisp/vtable.el
@@ -282,10 +282,9 @@ vtable-update-object
   "Update OBJECT's representation in TABLE.
 If OLD-OBJECT is non-nil, replace OLD-OBJECT with OBJECT and display it.
 In either case, if the existing object is not found in the table (being
-compared with `equal'), signal an error.  Note a limitation: if TABLE's
-buffer is not in a visible window, or if its window has changed width
-since it was updated, updating the TABLE is not possible, and an error
-is signaled."
+compared with `equal'), signal an error.
+
+TABLE must be at point in the current buffer."
   (unless old-object
     (setq old-object object))
   (let* ((objects (vtable-objects table))
@@ -302,14 +301,13 @@ vtable-update-object
       (unless objects
         (error "Can't find the old object"))
       (setcar (cdr objects) object))
-    ;; Then update the cache...
-    ;; FIXME: If the table's buffer has no visible window, or if its
-    ;; width has changed since the table was updated, the cache key will
-    ;; not match and the object can't be updated.  (Bug #69837).
-    (if-let* ((line-number (seq-position (car (vtable--cache table)) old-object
-                                         (lambda (a b)
-                                           (equal (car a) b))))
-              (line (elt (car (vtable--cache table)) line-number)))
+    ;; Then update the rendered vtable in the current buffer.
+    (if-let* ((cache (vtable--current-cache))
+             (line-number (seq-position (vtable--cache-lines cache)
+                                        old-object
+                                        (lambda (a b)
+                                          (equal (car a) b))))
+             (line (elt (vtable--cache-lines cache) line-number)))
         (progn
           (setcar line object)
           (setcdr line (vtable--compute-cached-line table object))
@@ -320,10 +318,11 @@ vtable-update-object
                   (start (point)))
               (delete-line)
               (vtable--insert-line table line line-number
-                                   (nth 1 (vtable--cache table))
+                                   (vtable--cache-widths cache)
                                    (vtable--spacer table))
               (add-text-properties start (point) (list 'keymap keymap
-                                                       'vtable table))))
+                                                       'vtable table
+                                                       'vtable-cache cache))))
           ;; We may have inserted a non-numerical value into a previously
           ;; all-numerical table, so recompute.
           (vtable--recompute-numerical table (cdr line)))
@@ -335,11 +334,12 @@ vtable-remove-object
   ;; First remove from the objects.
   (setf (vtable-objects table) (delq object (vtable-objects table)))
   ;; Then adjust the cache and display.
-  (let ((cache (vtable--cache table))
-        (inhibit-read-only t))
-    (setcar cache (delq (assq object (car cache)) (car cache)))
-    (save-excursion
-      (vtable-goto-table table)
+  (save-excursion
+    (vtable-goto-table table)
+    (let ((cache (vtable--current-cache))
+          (inhibit-read-only t))
+      (setcar cache (delq (assq object (vtable--cache-lines cache))
+                          (vtable--cache-lines cache)))
       (when (vtable-goto-object object)
         (delete-line)))))
 
@@ -400,7 +400,7 @@ vtable-insert-object
     ;; Then adjust the cache and display.
     (save-excursion
       (vtable-goto-table table)
-      (let* ((cache (vtable--cache table))
+      (let* ((cache (vtable--current-cache))
              (inhibit-read-only t)
              (keymap (get-text-property (point) 'keymap))
              (ellipsis (if (vtable-ellipsis table)
@@ -408,13 +408,14 @@ vtable-insert-object
                                        'face (vtable-face table))
                          ""))
              (ellipsis-width (string-pixel-width ellipsis))
-             (elem (if location  ; This binding mirrors the binding of `pos' above.
+             (lines (vtable--cache-lines cache))
+             (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)))
+                           (nth location lines)
+                         (or (assq location lines)
+                             (and before (car lines))))
+                     (if before (car lines))))
+             (pos (memq elem lines))
              (line (cons object (vtable--compute-cached-line table object))))
         (if (or before
                 (and pos (integerp location)))
@@ -433,13 +434,13 @@ vtable-insert-object
                     (forward-line 1)  ; Insert *after*.
                   (vtable-end-of-table)))
             ;; Otherwise, append the object.
-            (setcar cache (nconc (car cache) (list line)))
+            (setcar cache (nconc (vtable--cache-lines 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)
+                               (vtable--cache-widths cache) (vtable--spacer table)
                                ellipsis ellipsis-width)
           (add-text-properties start (point) (list 'keymap keymap
                                                    'vtable table)))
@@ -512,15 +513,11 @@ vtable--compute-columns
 (defun vtable--spacer (table)
   (vtable--compute-width table (vtable-separator-width table)))
 
-(defun vtable--recompute-cache (table)
-  (let* ((data (vtable--compute-cache table))
-         (widths (vtable--compute-widths table data)))
-    (setf (gethash (vtable--cache-key) (slot-value table '-cache))
-          (list data widths))))
+(defun vtable--cache-widths (cache)
+  (nth 1 cache))
 
-(defun vtable--ensure-cache (table)
-  (or (vtable--cache table)
-      (vtable--recompute-cache table)))
+(defun vtable--cache-lines (cache)
+  (car cache))
 
 (defun vtable-insert (table)
   (let* ((spacer (vtable--spacer table))
@@ -533,7 +530,12 @@ vtable-insert
          ;; We maintain a cache per screen/window width, so that we render
          ;; correctly if Emacs is open on two different screens (or the
          ;; user resizes the frame).
-         (widths (nth 1 (vtable--ensure-cache table))))
+         (cache (or (gethash (vtable--cache-key) (slot-value table '-cache))
+                    (let* ((data (vtable--compute-cache table))
+                           (widths (vtable--compute-widths table data)))
+                      (setf (gethash (vtable--cache-key) (slot-value table '-cache))
+                            (list data widths)))))
+         (widths (vtable--cache-widths cache)))
     ;; Don't insert any header or header line if the user hasn't
     ;; specified the columns.
     (when (slot-value table '-has-column-spec)
@@ -546,18 +548,20 @@ vtable-insert
         (add-text-properties start (point)
                              (list 'keymap vtable-header-line-map
                                    'rear-nonsticky t
-                                   'vtable table))
+                                   'vtable table
+                                   'vtable-cache cache))
         (setq start (point))))
-    (vtable--sort table)
+    (vtable--sort table cache)
     ;; Insert the data.
     (let ((line-number 0))
-      (dolist (line (car (vtable--cache table)))
+      (dolist (line (vtable--cache-lines cache))
         (vtable--insert-line table line line-number widths spacer
                              ellipsis ellipsis-width)
         (setq line-number (1+ line-number))))
     (add-text-properties start (point)
                          (list 'rear-nonsticky t
-                               'vtable table))
+                               'vtable table
+                               'vtable-cache cache))
     (goto-char start)))
 
 (defun vtable--insert-line (table line line-number widths spacer
@@ -659,16 +663,22 @@ vtable--insert-line
 (defun vtable--cache-key ()
   (cons (frame-terminal) (window-width)))
 
-(defun vtable--cache (table)
-  (gethash (vtable--cache-key) (slot-value table '-cache)))
+(defun vtable--current-cache ()
+  "Return the current cache for the table at point.
+
+In `vtable-insert', the lines and widths of the vtable text are computed
+based on the current selected frame and window and stored in a cache.
+Subsequent interaction with the text of the vtable should use that cache
+via this function rather than by calling `vtable--cache-key' to look up
+the cache."
+  (get-text-property (point) 'vtable-cache))
 
 (defun vtable--clear-cache (table)
   (setf (gethash (vtable--cache-key) (slot-value table '-cache)) nil))
 
-(defun vtable--sort (table)
+(defun vtable--sort (table cache)
   (pcase-dolist (`(,index . ,direction) (vtable-sort-by table))
-    (let ((cache (vtable--cache table))
-          (numerical (vtable-column--numerical
+    (let ((numerical (vtable-column--numerical
                       (elt (vtable-columns table) index)))
           (numcomp (if (eq direction 'descend)
                        #'> #'<))
@@ -971,9 +981,6 @@ vtable-revert
     (when column
       (vtable-goto-column column))))
 
-(defun vtable--widths (table)
-  (nth 1 (vtable--ensure-cache table)))
-
 ;;; Commands.
 
 (defvar-keymap vtable-header-mode-map
@@ -998,7 +1005,7 @@ vtable-narrow-current-column
                                 (- (* (vtable--char-width table) (or n 1))))))
 
 (defun vtable--alter-column-width (table column delta)
-  (let ((widths (vtable--widths table)))
+  (let ((widths (vtable--cache-widths (vtable--current-cache))))
     (setf (aref widths column)
           (max (* (vtable--char-width table) 2)
                (+ (aref widths column) delta)))
@@ -1020,14 +1027,14 @@ vtable-previous-column
   (interactive)
   (vtable-goto-column
    (max 0 (1- (or (vtable-current-column)
-                  (length (vtable--widths (vtable-current-table))))))))
+                  (length (vtable--cache-widths (vtable--current-cache))))))))
 
 (defun vtable-next-column ()
   "Go to the next column."
   (interactive)
   (when (vtable-current-column)
     (vtable-goto-column
-     (min (1- (length (vtable--widths (vtable-current-table))))
+     (min (1- (length (vtable--cache-widths (vtable--current-cache))))
           (1+ (vtable-current-column))))))
 
 (defun vtable-revert-command ()
-- 
2.39.3

Message #40 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2;
 vtable-update-object only works in visible windows
Date: Thu, 19 Jun 2025 18:06:40 -0400

[Message part 1 (text/plain, inline)]

On Thu, Jun 19, 2025 at 6:02 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:

> Stéphane Marks <shipmints <at> gmail.com> writes:
> > On Thu, Jun 19, 2025 at 5:37 PM Spencer Baugh via Bug reports for GNU
> Emacs, the Swiss army knife of text editors
> > <bug-gnu-emacs <at> gnu.org> wrote:
> >
> >  Stefan Kangas <stefankangas <at> gmail.com> writes:
> >
> >  > Eli Zaretskii <eliz <at> gnu.org> writes:
> >  >
> >  >>> Date: Thu, 21 Mar 2024 03:36:02 -0500
> >  >>> Cc: 69837 <at> debbugs.gnu.org
> >  >>> From: Adam Porter <adam <at> alphapapa.net>
> >  >>>
> >  >>> FWIW, I did some hacking on listen.el attempting to further
> understand
> >  >>> and work around this problem, and I've found what seems to be a
> usable
> >  >>> workaround (for my purposes, anyway).
> >  >>>
> >  >>> The attached code defines a macro within which I call
> >  >>> `listen-queue--vtable-update-object' (which merely incorporates the
> fix
> >  >>> to `vtable-update-object' from bug#69664).  As well, I wrap
> >  >>> `make-vtable' in a function that also sets two buffer-local
> variables to
> >  >>> the values of `frame-terminal' and `window-width' at the time of the
> >  >>> vtable's creation.  The macro locally overrides the functions
> >  >>> `frame-terminal' and `window-width' to return those saved values.
> >  >>>
> >  >>> This allows the cache key to match unconditionally, which allows the
> >  >>> vtable's objects to be updated even when its buffer is not visible.
> >  >>>
> >  >>> In my limited testing, it seems to work fine.  In my estimation, the
> >  >>> consequences of doing this in the worst case would be that the rows
> for
> >  >>> the updated objects might be drawn with some columns at a slightly
> >  >>> incorrect width, which is easily rectified by reverting the table
> >  >>> (usually bound to "g").  As well, that worst case (e.g. imagining a
> >  >>> vtable whose buffer might be initially displayed on one
> terminal/monitor
> >  >>> and later on another with different characteristics) would seem to
> be
> >  >>> relatively rare (so for my project, it seems like an obviously good
> >  >>> thing to do).
> >  >>>
> >  >>> For Emacs itself, I'm not sure what the best fix would be.  I
> suppose a
> >  >>> workaround like this could be implemented as a fallback in case the
> >  >>> cache key misses; it would seem better to update the object
> potentially
> >  >>> sub-optimally than to error and not update it at all.
> >  >>>
> >  >>> Another possibility would be to ignore the frame-terminal and
> >  >>> window-width in the cache key altogether (i.e. so they would always
> be
> >  >>> assumed to be the same), but I'm sure that Lars did it this way for
> a
> >  >>> reason, so that would seem unwise.
> >  >>>
> >  >>> Let me know how you'd like me to proceed.
> >  >>
> >  >> I think you should install your workaround.  I don't see how it could
> >  >> be worse than what we have now.
> >  >>
> >  >> P.S. And sorry for a long silence.
> >  >
> >  > Adam, could you please install the workaround?  Or maybe you did
> >  > already?
> >
> >  I ran into this same problem.  I think Adam's workaround is on the right
> >  track, but the issue is present in a number of other functions in
> >  vtable.el, not just vtable-update-object.
> >
> >  The root of the issue is that when we're interacting with the text of an
> >  inserted vtable, we should use the "cache" object that was last used to
> >  insert that vtable, rather than one based on the current selected window
> >  and terminal.  Otherwise we will experience various inconsistencies,
> >  e.g. inserting a new line that has different column widths from the rest
> >  of the vtable text.
> >
> >  My attached patch solves this by saving the "cache" object in a text
> >  property on the vtable text, and updating all the relevant vtable
> >  functions to access the cache via that text property rather than by
> >  computing a cache key from the current selected window/terminal.
> >
> >  Adam, could you check that this solves the problem for your use case?
> >
> > I have a very big patch coming for vtable under
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=78843 which addresses this
> issue,
> > and a ton of other bugs (and features).  That's coming in the next day
> or so, if you can wait?
>
> Certainly I can wait.  Cc me when you send the patch.
>
> Though if your patch is very large, maybe this relatively small patch
> should land first.
>
> (Also, I hope that you're adding tests in your very large patch?)
>
> BTW, an updated version of my patch is attached, just fixing a silly bug
> that was in the first version.
>

Your patch doesn't address several other intertwined issues that my bigger
patch addresses.  I've been testing it for ages now and with other vtable
users.  I can send a new vtable.el and vtable.texi to you off line if you'd
like a look before I finish writing the long changelog.

-Stéphane

[Message part 2 (text/html, inline)]

Message #43 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in
 vtable.el
Date: Wed, 03 Sep 2025 12:30:23 -0400

Spencer Baugh <sbaugh <at> janestreet.com> writes:
> Stéphane Marks <shipmints <at> gmail.com> writes:
>> On Thu, Jun 19, 2025 at 5:37 PM Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
>> <bug-gnu-emacs <at> gnu.org> wrote:
>>
>>  Stefan Kangas <stefankangas <at> gmail.com> writes:
>>
>>  > Eli Zaretskii <eliz <at> gnu.org> writes:
>>  >
>>  >>> Date: Thu, 21 Mar 2024 03:36:02 -0500
>>  >>> Cc: 69837 <at> debbugs.gnu.org
>>  >>> From: Adam Porter <adam <at> alphapapa.net>
>>  >>>
>>  >>> FWIW, I did some hacking on listen.el attempting to further understand
>>  >>> and work around this problem, and I've found what seems to be a usable
>>  >>> workaround (for my purposes, anyway).
>>  >>>
>>  >>> The attached code defines a macro within which I call
>>  >>> `listen-queue--vtable-update-object' (which merely incorporates the fix
>>  >>> to `vtable-update-object' from bug#69664).  As well, I wrap
>>  >>> `make-vtable' in a function that also sets two buffer-local variables to
>>  >>> the values of `frame-terminal' and `window-width' at the time of the
>>  >>> vtable's creation.  The macro locally overrides the functions
>>  >>> `frame-terminal' and `window-width' to return those saved values.
>>  >>>
>>  >>> This allows the cache key to match unconditionally, which allows the
>>  >>> vtable's objects to be updated even when its buffer is not visible.
>>  >>>
>>  >>> In my limited testing, it seems to work fine.  In my estimation, the
>>  >>> consequences of doing this in the worst case would be that the rows for
>>  >>> the updated objects might be drawn with some columns at a slightly
>>  >>> incorrect width, which is easily rectified by reverting the table
>>  >>> (usually bound to "g").  As well, that worst case (e.g. imagining a
>>  >>> vtable whose buffer might be initially displayed on one terminal/monitor
>>  >>> and later on another with different characteristics) would seem to be
>>  >>> relatively rare (so for my project, it seems like an obviously good
>>  >>> thing to do).
>>  >>>
>>  >>> For Emacs itself, I'm not sure what the best fix would be.  I suppose a
>>  >>> workaround like this could be implemented as a fallback in case the
>>  >>> cache key misses; it would seem better to update the object potentially
>>  >>> sub-optimally than to error and not update it at all.
>>  >>>
>>  >>> Another possibility would be to ignore the frame-terminal and
>>  >>> window-width in the cache key altogether (i.e. so they would always be
>>  >>> assumed to be the same), but I'm sure that Lars did it this way for a
>>  >>> reason, so that would seem unwise.
>>  >>>
>>  >>> Let me know how you'd like me to proceed.
>>  >>
>>  >> I think you should install your workaround.  I don't see how it could
>>  >> be worse than what we have now.
>>  >>
>>  >> P.S. And sorry for a long silence.
>>  >
>>  > Adam, could you please install the workaround?  Or maybe you did
>>  > already?
>>
>>  I ran into this same problem.  I think Adam's workaround is on the right
>>  track, but the issue is present in a number of other functions in
>>  vtable.el, not just vtable-update-object.
>>
>>  The root of the issue is that when we're interacting with the text of an
>>  inserted vtable, we should use the "cache" object that was last used to
>>  insert that vtable, rather than one based on the current selected window
>>  and terminal.  Otherwise we will experience various inconsistencies,
>>  e.g. inserting a new line that has different column widths from the rest
>>  of the vtable text.
>>
>>  My attached patch solves this by saving the "cache" object in a text
>>  property on the vtable text, and updating all the relevant vtable
>>  functions to access the cache via that text property rather than by
>>  computing a cache key from the current selected window/terminal.
>>
>>  Adam, could you check that this solves the problem for your use case?
>>
>> I have a very big patch coming for vtable under https://debbugs.gnu.org/cgi/bugreport.cgi?bug=78843 which addresses this issue,
>> and a ton of other bugs (and features).  That's coming in the next day or so, if you can wait?
>
> Certainly I can wait.  Cc me when you send the patch.
>
> Though if your patch is very large, maybe this relatively small patch
> should land first.

Since it's been a while and this large patch has not landed, I suggest
we should install my patch now to close this bug.  Stéphane's patch can
simply be rebased on top later.

Message #46 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el
Date: Thu, 4 Sep 2025 15:05:11 -0400

[Message part 1 (text/plain, inline)]

On Wed, Sep 3, 2025 at 12:30 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:

> Spencer Baugh <sbaugh <at> janestreet.com> writes:
> > Stéphane Marks <shipmints <at> gmail.com> writes:
> >> On Thu, Jun 19, 2025 at 5:37 PM Spencer Baugh via Bug reports for GNU
> Emacs, the Swiss army knife of text editors
> >> <bug-gnu-emacs <at> gnu.org> wrote:
> >>
> >>  Stefan Kangas <stefankangas <at> gmail.com> writes:
> >>
> >>  > Eli Zaretskii <eliz <at> gnu.org> writes:
> >>  >
> >>  >>> Date: Thu, 21 Mar 2024 03:36:02 -0500
> >>  >>> Cc: 69837 <at> debbugs.gnu.org
> >>  >>> From: Adam Porter <adam <at> alphapapa.net>
> >>  >>>
> >>  >>> FWIW, I did some hacking on listen.el attempting to further
> understand
> >>  >>> and work around this problem, and I've found what seems to be a
> usable
> >>  >>> workaround (for my purposes, anyway).
> >>  >>>
> >>  >>> The attached code defines a macro within which I call
> >>  >>> `listen-queue--vtable-update-object' (which merely incorporates
> the fix
> >>  >>> to `vtable-update-object' from bug#69664).  As well, I wrap
> >>  >>> `make-vtable' in a function that also sets two buffer-local
> variables to
> >>  >>> the values of `frame-terminal' and `window-width' at the time of
> the
> >>  >>> vtable's creation.  The macro locally overrides the functions
> >>  >>> `frame-terminal' and `window-width' to return those saved values.
> >>  >>>
> >>  >>> This allows the cache key to match unconditionally, which allows
> the
> >>  >>> vtable's objects to be updated even when its buffer is not visible.
> >>  >>>
> >>  >>> In my limited testing, it seems to work fine.  In my estimation,
> the
> >>  >>> consequences of doing this in the worst case would be that the
> rows for
> >>  >>> the updated objects might be drawn with some columns at a slightly
> >>  >>> incorrect width, which is easily rectified by reverting the table
> >>  >>> (usually bound to "g").  As well, that worst case (e.g. imagining a
> >>  >>> vtable whose buffer might be initially displayed on one
> terminal/monitor
> >>  >>> and later on another with different characteristics) would seem to
> be
> >>  >>> relatively rare (so for my project, it seems like an obviously good
> >>  >>> thing to do).
> >>  >>>
> >>  >>> For Emacs itself, I'm not sure what the best fix would be.  I
> suppose a
> >>  >>> workaround like this could be implemented as a fallback in case the
> >>  >>> cache key misses; it would seem better to update the object
> potentially
> >>  >>> sub-optimally than to error and not update it at all.
> >>  >>>
> >>  >>> Another possibility would be to ignore the frame-terminal and
> >>  >>> window-width in the cache key altogether (i.e. so they would
> always be
> >>  >>> assumed to be the same), but I'm sure that Lars did it this way
> for a
> >>  >>> reason, so that would seem unwise.
> >>  >>>
> >>  >>> Let me know how you'd like me to proceed.
> >>  >>
> >>  >> I think you should install your workaround.  I don't see how it
> could
> >>  >> be worse than what we have now.
> >>  >>
> >>  >> P.S. And sorry for a long silence.
> >>  >
> >>  > Adam, could you please install the workaround?  Or maybe you did
> >>  > already?
> >>
> >>  I ran into this same problem.  I think Adam's workaround is on the
> right
> >>  track, but the issue is present in a number of other functions in
> >>  vtable.el, not just vtable-update-object.
> >>
> >>  The root of the issue is that when we're interacting with the text of
> an
> >>  inserted vtable, we should use the "cache" object that was last used to
> >>  insert that vtable, rather than one based on the current selected
> window
> >>  and terminal.  Otherwise we will experience various inconsistencies,
> >>  e.g. inserting a new line that has different column widths from the
> rest
> >>  of the vtable text.
> >>
> >>  My attached patch solves this by saving the "cache" object in a text
> >>  property on the vtable text, and updating all the relevant vtable
> >>  functions to access the cache via that text property rather than by
> >>  computing a cache key from the current selected window/terminal.
> >>
> >>  Adam, could you check that this solves the problem for your use case?
> >>
> >> I have a very big patch coming for vtable under
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=78843 which addresses this
> issue,
> >> and a ton of other bugs (and features).  That's coming in the next day
> or so, if you can wait?
> >
> > Certainly I can wait.  Cc me when you send the patch.
> >
> > Though if your patch is very large, maybe this relatively small patch
> > should land first.
>
> Since it's been a while and this large patch has not landed, I suggest
> we should install my patch now to close this bug.  Stéphane's patch can
> simply be rebased on top later.
>

I'll try to get back into vtable this weekend and take a fresh look.

[Message part 2 (text/html, inline)]

Message #49 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el
Date: Wed, 10 Sep 2025 10:47:59 -0400

[Message part 1 (text/plain, inline)]

On Thu, Sep 4, 2025 at 3:05 PM Stéphane Marks <shipmints <at> gmail.com> wrote:

> On Wed, Sep 3, 2025 at 12:30 PM Spencer Baugh <sbaugh <at> janestreet.com>
> wrote:
>
>> Spencer Baugh <sbaugh <at> janestreet.com> writes:
>> > Stéphane Marks <shipmints <at> gmail.com> writes:
>> >> On Thu, Jun 19, 2025 at 5:37 PM Spencer Baugh via Bug reports for GNU
>> Emacs, the Swiss army knife of text editors
>> >> <bug-gnu-emacs <at> gnu.org> wrote:
>> >>
>> >>  Stefan Kangas <stefankangas <at> gmail.com> writes:
>> >>
>> >>  > Eli Zaretskii <eliz <at> gnu.org> writes:
>> >>  >
>> >>  >>> Date: Thu, 21 Mar 2024 03:36:02 -0500
>> >>  >>> Cc: 69837 <at> debbugs.gnu.org
>> >>  >>> From: Adam Porter <adam <at> alphapapa.net>
>> >>  >>>
>> >>  >>> FWIW, I did some hacking on listen.el attempting to further
>> understand
>> >>  >>> and work around this problem, and I've found what seems to be a
>> usable
>> >>  >>> workaround (for my purposes, anyway).
>> >>  >>>
>> >>  >>> The attached code defines a macro within which I call
>> >>  >>> `listen-queue--vtable-update-object' (which merely incorporates
>> the fix
>> >>  >>> to `vtable-update-object' from bug#69664).  As well, I wrap
>> >>  >>> `make-vtable' in a function that also sets two buffer-local
>> variables to
>> >>  >>> the values of `frame-terminal' and `window-width' at the time of
>> the
>> >>  >>> vtable's creation.  The macro locally overrides the functions
>> >>  >>> `frame-terminal' and `window-width' to return those saved values.
>> >>  >>>
>> >>  >>> This allows the cache key to match unconditionally, which allows
>> the
>> >>  >>> vtable's objects to be updated even when its buffer is not
>> visible.
>> >>  >>>
>> >>  >>> In my limited testing, it seems to work fine.  In my estimation,
>> the
>> >>  >>> consequences of doing this in the worst case would be that the
>> rows for
>> >>  >>> the updated objects might be drawn with some columns at a slightly
>> >>  >>> incorrect width, which is easily rectified by reverting the table
>> >>  >>> (usually bound to "g").  As well, that worst case (e.g. imagining
>> a
>> >>  >>> vtable whose buffer might be initially displayed on one
>> terminal/monitor
>> >>  >>> and later on another with different characteristics) would seem
>> to be
>> >>  >>> relatively rare (so for my project, it seems like an obviously
>> good
>> >>  >>> thing to do).
>> >>  >>>
>> >>  >>> For Emacs itself, I'm not sure what the best fix would be.  I
>> suppose a
>> >>  >>> workaround like this could be implemented as a fallback in case
>> the
>> >>  >>> cache key misses; it would seem better to update the object
>> potentially
>> >>  >>> sub-optimally than to error and not update it at all.
>> >>  >>>
>> >>  >>> Another possibility would be to ignore the frame-terminal and
>> >>  >>> window-width in the cache key altogether (i.e. so they would
>> always be
>> >>  >>> assumed to be the same), but I'm sure that Lars did it this way
>> for a
>> >>  >>> reason, so that would seem unwise.
>> >>  >>>
>> >>  >>> Let me know how you'd like me to proceed.
>> >>  >>
>> >>  >> I think you should install your workaround.  I don't see how it
>> could
>> >>  >> be worse than what we have now.
>> >>  >>
>> >>  >> P.S. And sorry for a long silence.
>> >>  >
>> >>  > Adam, could you please install the workaround?  Or maybe you did
>> >>  > already?
>> >>
>> >>  I ran into this same problem.  I think Adam's workaround is on the
>> right
>> >>  track, but the issue is present in a number of other functions in
>> >>  vtable.el, not just vtable-update-object.
>> >>
>> >>  The root of the issue is that when we're interacting with the text of
>> an
>> >>  inserted vtable, we should use the "cache" object that was last used
>> to
>> >>  insert that vtable, rather than one based on the current selected
>> window
>> >>  and terminal.  Otherwise we will experience various inconsistencies,
>> >>  e.g. inserting a new line that has different column widths from the
>> rest
>> >>  of the vtable text.
>> >>
>> >>  My attached patch solves this by saving the "cache" object in a text
>> >>  property on the vtable text, and updating all the relevant vtable
>> >>  functions to access the cache via that text property rather than by
>> >>  computing a cache key from the current selected window/terminal.
>> >>
>> >>  Adam, could you check that this solves the problem for your use case?
>> >>
>> >> I have a very big patch coming for vtable under
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=78843 which addresses this
>> issue,
>> >> and a ton of other bugs (and features).  That's coming in the next day
>> or so, if you can wait?
>> >
>> > Certainly I can wait.  Cc me when you send the patch.
>> >
>> > Though if your patch is very large, maybe this relatively small patch
>> > should land first.
>>
>> Since it's been a while and this large patch has not landed, I suggest
>> we should install my patch now to close this bug.  Stéphane's patch can
>> simply be rebased on top later.
>>
>
> I'll try to get back into vtable this weekend and take a fresh look.
>

I'll have some time tomorrow to test and react. I have one amendment that
enhances the patch in the case where a vtable is inserted in a window-less
buffer. My larger patch has such an accommodation.

[Message part 2 (text/html, inline)]

Message #52 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el
Date: Thu, 18 Sep 2025 14:49:36 -0400

[Message part 1 (text/plain, inline)]

On Wed, Sep 10, 2025 at 10:47 AM Stéphane Marks <shipmints <at> gmail.com> wrote:

> On Thu, Sep 4, 2025 at 3:05 PM Stéphane Marks <shipmints <at> gmail.com> wrote:
>
>> On Wed, Sep 3, 2025 at 12:30 PM Spencer Baugh <sbaugh <at> janestreet.com>
>> wrote:
>>
>>> Spencer Baugh <sbaugh <at> janestreet.com> writes:
>>> > Stéphane Marks <shipmints <at> gmail.com> writes:
>>> >> On Thu, Jun 19, 2025 at 5:37 PM Spencer Baugh via Bug reports for GNU
>>> Emacs, the Swiss army knife of text editors
>>> >> <bug-gnu-emacs <at> gnu.org> wrote:
>>> >>
>>> >>  Stefan Kangas <stefankangas <at> gmail.com> writes:
>>> >>
>>> >>  > Eli Zaretskii <eliz <at> gnu.org> writes:
>>> >>  >
>>> >>  >>> Date: Thu, 21 Mar 2024 03:36:02 -0500
>>> >>  >>> Cc: 69837 <at> debbugs.gnu.org
>>> >>  >>> From: Adam Porter <adam <at> alphapapa.net>
>>> >>  >>>
>>> >>  >>> FWIW, I did some hacking on listen.el attempting to further
>>> understand
>>> >>  >>> and work around this problem, and I've found what seems to be a
>>> usable
>>> >>  >>> workaround (for my purposes, anyway).
>>> >>  >>>
>>> >>  >>> The attached code defines a macro within which I call
>>> >>  >>> `listen-queue--vtable-update-object' (which merely incorporates
>>> the fix
>>> >>  >>> to `vtable-update-object' from bug#69664).  As well, I wrap
>>> >>  >>> `make-vtable' in a function that also sets two buffer-local
>>> variables to
>>> >>  >>> the values of `frame-terminal' and `window-width' at the time of
>>> the
>>> >>  >>> vtable's creation.  The macro locally overrides the functions
>>> >>  >>> `frame-terminal' and `window-width' to return those saved values.
>>> >>  >>>
>>> >>  >>> This allows the cache key to match unconditionally, which allows
>>> the
>>> >>  >>> vtable's objects to be updated even when its buffer is not
>>> visible.
>>> >>  >>>
>>> >>  >>> In my limited testing, it seems to work fine.  In my estimation,
>>> the
>>> >>  >>> consequences of doing this in the worst case would be that the
>>> rows for
>>> >>  >>> the updated objects might be drawn with some columns at a
>>> slightly
>>> >>  >>> incorrect width, which is easily rectified by reverting the table
>>> >>  >>> (usually bound to "g").  As well, that worst case (e.g.
>>> imagining a
>>> >>  >>> vtable whose buffer might be initially displayed on one
>>> terminal/monitor
>>> >>  >>> and later on another with different characteristics) would seem
>>> to be
>>> >>  >>> relatively rare (so for my project, it seems like an obviously
>>> good
>>> >>  >>> thing to do).
>>> >>  >>>
>>> >>  >>> For Emacs itself, I'm not sure what the best fix would be.  I
>>> suppose a
>>> >>  >>> workaround like this could be implemented as a fallback in case
>>> the
>>> >>  >>> cache key misses; it would seem better to update the object
>>> potentially
>>> >>  >>> sub-optimally than to error and not update it at all.
>>> >>  >>>
>>> >>  >>> Another possibility would be to ignore the frame-terminal and
>>> >>  >>> window-width in the cache key altogether (i.e. so they would
>>> always be
>>> >>  >>> assumed to be the same), but I'm sure that Lars did it this way
>>> for a
>>> >>  >>> reason, so that would seem unwise.
>>> >>  >>>
>>> >>  >>> Let me know how you'd like me to proceed.
>>> >>  >>
>>> >>  >> I think you should install your workaround.  I don't see how it
>>> could
>>> >>  >> be worse than what we have now.
>>> >>  >>
>>> >>  >> P.S. And sorry for a long silence.
>>> >>  >
>>> >>  > Adam, could you please install the workaround?  Or maybe you did
>>> >>  > already?
>>> >>
>>> >>  I ran into this same problem.  I think Adam's workaround is on the
>>> right
>>> >>  track, but the issue is present in a number of other functions in
>>> >>  vtable.el, not just vtable-update-object.
>>> >>
>>> >>  The root of the issue is that when we're interacting with the text
>>> of an
>>> >>  inserted vtable, we should use the "cache" object that was last used
>>> to
>>> >>  insert that vtable, rather than one based on the current selected
>>> window
>>> >>  and terminal.  Otherwise we will experience various inconsistencies,
>>> >>  e.g. inserting a new line that has different column widths from the
>>> rest
>>> >>  of the vtable text.
>>> >>
>>> >>  My attached patch solves this by saving the "cache" object in a text
>>> >>  property on the vtable text, and updating all the relevant vtable
>>> >>  functions to access the cache via that text property rather than by
>>> >>  computing a cache key from the current selected window/terminal.
>>> >>
>>> >>  Adam, could you check that this solves the problem for your use case?
>>> >>
>>> >> I have a very big patch coming for vtable under
>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=78843 which addresses
>>> this issue,
>>> >> and a ton of other bugs (and features).  That's coming in the next
>>> day or so, if you can wait?
>>> >
>>> > Certainly I can wait.  Cc me when you send the patch.
>>> >
>>> > Though if your patch is very large, maybe this relatively small patch
>>> > should land first.
>>>
>>> Since it's been a while and this large patch has not landed, I suggest
>>> we should install my patch now to close this bug.  Stéphane's patch can
>>> simply be rebased on top later.
>>>
>>
>> I'll try to get back into vtable this weekend and take a fresh look.
>>
>
> I'll have some time tomorrow to test and react. I have one amendment that
> enhances the patch in the case where a vtable is inserted in a window-less
> buffer. My larger patch has such an accommodation.
>

Thank you for being patient (family, life, and work priorities vs.
volunteer time).  I've put a lot of work into vtable bug fixes and
improvements, and I'd like to see how much momentum we can foster.

The proposed patch will mutate only the "current cache" on
insert/update/remove objects leaving the other caches invalid, and when
called from a window that doesn't share the cache key's width, the user
will need to manually revert to repopulate a cache of their window's width
and reset the "current cache" to avoid cache corruption runtime errors on
mutation (I've addressed these use cases in the larger patch.)

I think we should come to an agreement on supporting multiple window-width
caches at all or just a single cache.  The proposed patch would be better
suited to a single-cache case (regenerated on a full data revert).

As a buffer can contain only a single-sized vtable, even when displayed in
multiple windows of different widths, perhaps a single cache isn't so bad?

People might be relying on the current multi-width cache implementation, at
least for predominantly read-only vtables (or they would have reported the
issues we're aware of).  One simple concession could be to support multiple
caches for read-only tables, and simply invalidate other caches on
mutations (this is the policy I implemented in the larger patch).

If we decide on a single cache, then the simpler approach in the proposed
patch could work at the cost of making it harder to restore multi-cache
later (and, if we go with the cache as text property on the table, we
should rename it `vtable--cache` to avoid people using its value in their
own code).

If we decide to retain multi-cache, I'd prefer if we make the smallest
possible change to deal with the case you need now.  I think this can be
done with a simple change to `vtable--cache-key` to produce a uniform
window-less key.  I want to add the option to record the buffer into which
a vtable instance has been inserted to precisely test whether the current
window's buffer is the vtable buffer (also addressed in the larger patch)
and which addresses Augusto's `comint-mime` use case.

-Stéphane

[Message part 2 (text/html, inline)]

Message #55 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in
 vtable.el
Date: Thu, 18 Sep 2025 15:20:09 -0400

Stéphane Marks <shipmints <at> gmail.com> writes:
> Thank you for being patient (family, life, and work priorities vs. volunteer time). I've put a lot of work into vtable bug fixes and
> improvements, and I'd like to see how much momentum we can foster.
>
> The proposed patch will mutate only the "current cache" on insert/update/remove objects leaving the other caches invalid, and
> when called from a window that doesn't share the cache key's width, the user will need to manually revert to repopulate a cache
> of their window's width and reset the "current cache" to avoid cache corruption runtime errors on mutation (I've addressed these
> use cases in the larger patch.)

That's not true.  No corruption can happen with my patch.  If you
believe otherwise, please give some code which demonstrates it, and we
can convert that into a test.

> I think we should come to an agreement on supporting multiple window-width caches at all or just a single cache. The proposed
> patch would be better suited to a single-cache case (regenerated on a full data revert).
>
> As a buffer can contain only a single-sized vtable, even when displayed in multiple windows of different widths, perhaps a single
> cache isn't so bad?

Yes.  That's what my patch does, it has a single "cache" stored in the
text of the vtable, no longer keyed by anything.

But really "cache" is the wrong word for this.  It's just some mutable
data stored by the vtable.

> People might be relying on the current multi-width cache implementation, at least for predominantly read-only vtables (or they
> would have reported the issues we're aware of).  One simple concession could be to support multiple caches for read-only tables,
> and simply invalidate other caches on mutations (this is the policy I implemented in the larger patch).
>
> If we decide on a single cache, then the simpler approach in the proposed patch could work at the cost of making it harder to
> restore multi-cache later (and, if we go with the cache as text property on the table, we should rename it `vtable--cache` to avoid
> people using its value in their own code).

The multi-width cache is completely useless and broken with the current
state of the code, so no-one can be relying on it.  And anyway it
provides no benefit, because vtables can't be displayed differently in
different windows.

> If we decide to retain multi-cache, I'd prefer if we make the smallest possible change to deal with the case you need now.  I think
> this can be done with a simple change to `vtable--cache-key` to produce a uniform window-less key. I want to add the option to
> record the buffer into which a vtable instance has been inserted to precisely test whether the current window's buffer is the vtable
> buffer (also addressed in the larger patch) and which addresses Augusto's `comint-mime` use case. 

Changing vtable--cache-key to produce a key which doesn't mention the
window or the frame would mean changing to have a single cache, because
those are the only two things in the cache key.

Message #58 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el
Date: Fri, 26 Sep 2025 10:09:19 -0400

[Message part 1 (text/plain, inline)]

I kind of badly injured my left hand (this is right hand typing) so will be
back to this when I can function a bit better.

On Thu, Sep 18, 2025 at 3:20 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:

> Stéphane Marks <shipmints <at> gmail.com> writes:
> > Thank you for being patient (family, life, and work priorities vs.
> volunteer time). I've put a lot of work into vtable bug fixes and
> > improvements, and I'd like to see how much momentum we can foster.
> >
> > The proposed patch will mutate only the "current cache" on
> insert/update/remove objects leaving the other caches invalid, and
> > when called from a window that doesn't share the cache key's width, the
> user will need to manually revert to repopulate a cache
> > of their window's width and reset the "current cache" to avoid cache
> corruption runtime errors on mutation (I've addressed these
> > use cases in the larger patch.)
>
> That's not true.  No corruption can happen with my patch.  If you
> believe otherwise, please give some code which demonstrates it, and we
> can convert that into a test.
>
> > I think we should come to an agreement on supporting multiple
> window-width caches at all or just a single cache. The proposed
> > patch would be better suited to a single-cache case (regenerated on a
> full data revert).
> >
> > As a buffer can contain only a single-sized vtable, even when displayed
> in multiple windows of different widths, perhaps a single
> > cache isn't so bad?
>
> Yes.  That's what my patch does, it has a single "cache" stored in the
> text of the vtable, no longer keyed by anything.
>
> But really "cache" is the wrong word for this.  It's just some mutable
> data stored by the vtable.
>
> > People might be relying on the current multi-width cache implementation,
> at least for predominantly read-only vtables (or they
> > would have reported the issues we're aware of).  One simple concession
> could be to support multiple caches for read-only tables,
> > and simply invalidate other caches on mutations (this is the policy I
> implemented in the larger patch).
> >
> > If we decide on a single cache, then the simpler approach in the
> proposed patch could work at the cost of making it harder to
> > restore multi-cache later (and, if we go with the cache as text property
> on the table, we should rename it `vtable--cache` to avoid
> > people using its value in their own code).
>
> The multi-width cache is completely useless and broken with the current
> state of the code, so no-one can be relying on it.  And anyway it
> provides no benefit, because vtables can't be displayed differently in
> different windows.
>
> > If we decide to retain multi-cache, I'd prefer if we make the smallest
> possible change to deal with the case you need now.  I think
> > this can be done with a simple change to `vtable--cache-key` to produce
> a uniform window-less key. I want to add the option to
> > record the buffer into which a vtable instance has been inserted to
> precisely test whether the current window's buffer is the vtable
> > buffer (also addressed in the larger patch) and which addresses
> Augusto's `comint-mime` use case.
>
> Changing vtable--cache-key to produce a key which doesn't mention the
> window or the frame would mean changing to have a single cache, because
> those are the only two things in the cache key.
>

[Message part 2 (text/html, inline)]

Message #61 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el
Date: Thu, 30 Oct 2025 17:41:48 -0400

[Message part 1 (text/plain, inline)]

On Fri, Sep 26, 2025 at 10:09 AM Stéphane Marks <shipmints <at> gmail.com> wrote:

> I kind of badly injured my left hand (this is right hand typing) so will
> be back to this when I can function a bit better.
>
> On Thu, Sep 18, 2025 at 3:20 PM Spencer Baugh <sbaugh <at> janestreet.com>
> wrote:
>
>> Stéphane Marks <shipmints <at> gmail.com> writes:
>> > Thank you for being patient (family, life, and work priorities vs.
>> volunteer time). I've put a lot of work into vtable bug fixes and
>> > improvements, and I'd like to see how much momentum we can foster.
>> >
>> > The proposed patch will mutate only the "current cache" on
>> insert/update/remove objects leaving the other caches invalid, and
>> > when called from a window that doesn't share the cache key's width, the
>> user will need to manually revert to repopulate a cache
>> > of their window's width and reset the "current cache" to avoid cache
>> corruption runtime errors on mutation (I've addressed these
>> > use cases in the larger patch.)
>>
>> That's not true.  No corruption can happen with my patch.  If you
>> believe otherwise, please give some code which demonstrates it, and we
>> can convert that into a test.
>>
>> > I think we should come to an agreement on supporting multiple
>> window-width caches at all or just a single cache. The proposed
>> > patch would be better suited to a single-cache case (regenerated on a
>> full data revert).
>> >
>> > As a buffer can contain only a single-sized vtable, even when displayed
>> in multiple windows of different widths, perhaps a single
>> > cache isn't so bad?
>>
>> Yes.  That's what my patch does, it has a single "cache" stored in the
>> text of the vtable, no longer keyed by anything.
>>
>> But really "cache" is the wrong word for this.  It's just some mutable
>> data stored by the vtable.
>>
>> > People might be relying on the current multi-width cache
>> implementation, at least for predominantly read-only vtables (or they
>> > would have reported the issues we're aware of).  One simple concession
>> could be to support multiple caches for read-only tables,
>> > and simply invalidate other caches on mutations (this is the policy I
>> implemented in the larger patch).
>> >
>> > If we decide on a single cache, then the simpler approach in the
>> proposed patch could work at the cost of making it harder to
>> > restore multi-cache later (and, if we go with the cache as text
>> property on the table, we should rename it `vtable--cache` to avoid
>> > people using its value in their own code).
>>
>> The multi-width cache is completely useless and broken with the current
>> state of the code, so no-one can be relying on it.  And anyway it
>> provides no benefit, because vtables can't be displayed differently in
>> different windows.
>>
>> > If we decide to retain multi-cache, I'd prefer if we make the smallest
>> possible change to deal with the case you need now.  I think
>> > this can be done with a simple change to `vtable--cache-key` to produce
>> a uniform window-less key. I want to add the option to
>> > record the buffer into which a vtable instance has been inserted to
>> precisely test whether the current window's buffer is the vtable
>> > buffer (also addressed in the larger patch) and which addresses
>> Augusto's `comint-mime` use case.
>>
>> Changing vtable--cache-key to produce a key which doesn't mention the
>> window or the frame would mean changing to have a single cache, because
>> those are the only two things in the cache key.
>>
>
Alright! Hand becoming sufficiently usable! Sorry for the delay.

Spencer, can you take a quick look at the patch you provided
via 0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch it does
not pass the vtable tests (which we will beef up, and get rid of the typo
in the first test name).

Selector: test-vtable-insert-object
Passed:  0
Failed:  1 (1 unexpected)
Skipped: 0
Total:   1/1

Started at:   2025-10-30 14:16:26-0400
Finished.
Finished at:  2025-10-30 14:16:27-0400

F

F test-vtable-insert-object
    (wrong-type-argument consp nil)

Thank you,

-Stéphane (two handed!)

[Message part 2 (text/html, inline)]

Message #64 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in
 vtable.el
Date: Mon, 10 Nov 2025 18:00:45 -0500

[Message part 1 (text/plain, inline)]

Stéphane Marks <shipmints <at> gmail.com> writes:
> Alright! Hand becoming sufficiently usable! Sorry for the delay.
>
> Spencer, can you take a quick look at the patch you provided via 0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch
> it does not pass the vtable tests (which we will beef up, and get rid of the typo in the first test name).

Thanks for checking, apologies about that, it was just an error
introduced when I ported the patch forward from emacs-30 (which is what
we use at my site).

Fixed it, this version should be correct.

[0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch (text/x-patch, inline)]

From e8a90f0942fa09f8168c4fb9619144c52b0676eb Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Thu, 19 Jun 2025 17:20:12 -0400
Subject: [PATCH] Fix implicit usage of the current window-width in vtable.el

Previously, many functions in vtable.el called vtable--cache,
which computed vtable--cache-key based on the current selected
window and frame; this could cause vtable functions to fail or
misbehave if they were not called from the selected window and
frame that vtable-insert was last called in.

Now, the vtable cache is stored with the text of the vtable, so
that functions which need to interact with some vtable text can
do so reliably without having to use the same selected window
and frame.

Also, vtable-update-object has always required TABLE to be
present at point in the current buffer; now its docstring states
this.

* lisp/emacs-lisp/vtable.el (vtable--current-cache)
(vtable--cache-widths, vtable--cache-lines): Add.
(vtable-insert): Save cache in 'vtable-cache.
(vtable--ensure-cache, vtable--recompute-cache): Inline into
vtable-insert.
(vtable--widths, vtable--cache): Delete.
(vtable-update-object): Use vtable--current-cache and update
docstring.  (bug#69837)
(vtable-remove-object, vtable-insert-object): Use
vtable--current-cache and save cache in 'vtable-cache.
(vtable--sort, vtable--alter-column-width)
(vtable-previous-column, vtable-next-column): Use
vtable--current-cache.
---
 lisp/emacs-lisp/vtable.el | 114 ++++++++++++++++++++------------------
 1 file changed, 61 insertions(+), 53 deletions(-)

diff --git a/lisp/emacs-lisp/vtable.el b/lisp/emacs-lisp/vtable.el
index 00785113edb..bcdd280fb92 100644
--- a/lisp/emacs-lisp/vtable.el
+++ b/lisp/emacs-lisp/vtable.el
@@ -282,10 +282,9 @@ vtable-update-object
   "Update OBJECT's representation in TABLE.
 If OLD-OBJECT is non-nil, replace OLD-OBJECT with OBJECT and display it.
 In either case, if the existing object is not found in the table (being
-compared with `equal'), signal an error.  Note a limitation: if TABLE's
-buffer is not in a visible window, or if its window has changed width
-since it was updated, updating the TABLE is not possible, and an error
-is signaled."
+compared with `equal'), signal an error.
+
+TABLE must be at point in the current buffer."
   (unless old-object
     (setq old-object object))
   (let* ((objects (vtable-objects table))
@@ -299,17 +298,16 @@ vtable-update-object
       (while (and (cdr objects)
                   (not (eq (cadr objects) old-object)))
         (setq objects (cdr objects)))
-      (unless objects
+      (unless (cdr objects)
         (error "Can't find the old object"))
       (setcar (cdr objects) object))
-    ;; Then update the cache...
-    ;; FIXME: If the table's buffer has no visible window, or if its
-    ;; width has changed since the table was updated, the cache key will
-    ;; not match and the object can't be updated.  (Bug #69837).
-    (if-let* ((line-number (seq-position (car (vtable--cache table)) old-object
-                                         (lambda (a b)
-                                           (equal (car a) b))))
-              (line (elt (car (vtable--cache table)) line-number)))
+    ;; Then update the rendered vtable in the current buffer.
+    (if-let* ((cache (vtable--current-cache))
+             (line-number (seq-position (vtable--cache-lines cache)
+                                        old-object
+                                        (lambda (a b)
+                                          (equal (car a) b))))
+             (line (elt (vtable--cache-lines cache) line-number)))
         (progn
           (setcar line object)
           (setcdr line (vtable--compute-cached-line table object))
@@ -320,10 +318,11 @@ vtable-update-object
                   (start (point)))
               (delete-line)
               (vtable--insert-line table line line-number
-                                   (nth 1 (vtable--cache table))
+                                   (vtable--cache-widths cache)
                                    (vtable--spacer table))
               (add-text-properties start (point) (list 'keymap keymap
-                                                       'vtable table))))
+                                                       'vtable table
+                                                       'vtable-cache cache))))
           ;; We may have inserted a non-numerical value into a previously
           ;; all-numerical table, so recompute.
           (vtable--recompute-numerical table (cdr line)))
@@ -335,11 +334,12 @@ vtable-remove-object
   ;; First remove from the objects.
   (setf (vtable-objects table) (delq object (vtable-objects table)))
   ;; Then adjust the cache and display.
-  (let ((cache (vtable--cache table))
-        (inhibit-read-only t))
-    (setcar cache (delq (assq object (car cache)) (car cache)))
-    (save-excursion
-      (vtable-goto-table table)
+  (save-excursion
+    (vtable-goto-table table)
+    (let ((cache (vtable--current-cache))
+          (inhibit-read-only t))
+      (setcar cache (delq (assq object (vtable--cache-lines cache))
+                          (vtable--cache-lines cache)))
       (when (vtable-goto-object object)
         (delete-line)))))
 
@@ -400,7 +400,7 @@ vtable-insert-object
     ;; Then adjust the cache and display.
     (save-excursion
       (vtable-goto-table table)
-      (let* ((cache (vtable--cache table))
+      (let* ((cache (vtable--current-cache))
              (inhibit-read-only t)
              (keymap (get-text-property (point) 'keymap))
              (ellipsis (if (vtable-ellipsis table)
@@ -408,13 +408,14 @@ vtable-insert-object
                                        'face (vtable-face table))
                          ""))
              (ellipsis-width (string-pixel-width ellipsis))
+             (lines (vtable--cache-lines cache))
              (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)))
+                           (nth location lines)
+                         (or (assq location lines)
+                             (and before (car lines))))
+                     (if before (car lines))))
+             (pos (memq elem lines))
              (line (cons object (vtable--compute-cached-line table object))))
         (if (or before
                 (and pos (integerp location)))
@@ -433,16 +434,17 @@ vtable-insert-object
                     (forward-line 1)  ; Insert *after*.
                   (vtable-end-of-table)))
             ;; Otherwise, append the object.
-            (setcar cache (nconc (car cache) (list line)))
+            (setcar cache (nconc lines (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)
+                               (vtable--cache-widths cache) (vtable--spacer table)
                                ellipsis ellipsis-width)
           (add-text-properties start (point) (list 'keymap keymap
-                                                   'vtable table)))
+                                                   'vtable table
+                                                   'vtable-cache cache)))
         ;; We may have inserted a non-numerical value into a previously
         ;; all-numerical table, so recompute.
         (vtable--recompute-numerical table (cdr line))))))
@@ -512,15 +514,11 @@ vtable--compute-columns
 (defun vtable--spacer (table)
   (vtable--compute-width table (vtable-separator-width table)))
 
-(defun vtable--recompute-cache (table)
-  (let* ((data (vtable--compute-cache table))
-         (widths (vtable--compute-widths table data)))
-    (setf (gethash (vtable--cache-key) (slot-value table '-cache))
-          (list data widths))))
+(defun vtable--cache-widths (cache)
+  (nth 1 cache))
 
-(defun vtable--ensure-cache (table)
-  (or (vtable--cache table)
-      (vtable--recompute-cache table)))
+(defun vtable--cache-lines (cache)
+  (car cache))
 
 (defun vtable-insert (table)
   (let* ((spacer (vtable--spacer table))
@@ -533,7 +531,12 @@ vtable-insert
          ;; We maintain a cache per screen/window width, so that we render
          ;; correctly if Emacs is open on two different screens (or the
          ;; user resizes the frame).
-         (widths (nth 1 (vtable--ensure-cache table))))
+         (cache (or (gethash (vtable--cache-key) (slot-value table '-cache))
+                    (let* ((data (vtable--compute-cache table))
+                           (widths (vtable--compute-widths table data)))
+                      (setf (gethash (vtable--cache-key) (slot-value table '-cache))
+                            (list data widths)))))
+         (widths (vtable--cache-widths cache)))
     ;; Don't insert any header or header line if the user hasn't
     ;; specified the columns.
     (when (slot-value table '-has-column-spec)
@@ -546,18 +549,20 @@ vtable-insert
         (add-text-properties start (point)
                              (list 'keymap vtable-header-line-map
                                    'rear-nonsticky t
-                                   'vtable table))
+                                   'vtable table
+                                   'vtable-cache cache))
         (setq start (point))))
-    (vtable--sort table)
+    (vtable--sort table cache)
     ;; Insert the data.
     (let ((line-number 0))
-      (dolist (line (car (vtable--cache table)))
+      (dolist (line (vtable--cache-lines cache))
         (vtable--insert-line table line line-number widths spacer
                              ellipsis ellipsis-width)
         (setq line-number (1+ line-number))))
     (add-text-properties start (point)
                          (list 'rear-nonsticky t
-                               'vtable table))
+                               'vtable table
+                               'vtable-cache cache))
     (goto-char start)))
 
 (defun vtable--insert-line (table line line-number widths spacer
@@ -659,16 +664,22 @@ vtable--insert-line
 (defun vtable--cache-key ()
   (cons (frame-terminal) (window-width)))
 
-(defun vtable--cache (table)
-  (gethash (vtable--cache-key) (slot-value table '-cache)))
+(defun vtable--current-cache ()
+  "Return the current cache for the table at point.
+
+In `vtable-insert', the lines and widths of the vtable text are computed
+based on the current selected frame and window and stored in a cache.
+Subsequent interaction with the text of the vtable should use that cache
+via this function rather than by calling `vtable--cache-key' to look up
+the cache."
+  (get-text-property (point) 'vtable-cache))
 
 (defun vtable--clear-cache (table)
   (setf (gethash (vtable--cache-key) (slot-value table '-cache)) nil))
 
-(defun vtable--sort (table)
+(defun vtable--sort (table cache)
   (pcase-dolist (`(,index . ,direction) (vtable-sort-by table))
-    (let ((cache (vtable--cache table))
-          (numerical (vtable-column--numerical
+    (let ((numerical (vtable-column--numerical
                       (elt (vtable-columns table) index)))
           (numcomp (if (eq direction 'descend)
                        #'> #'<))
@@ -971,9 +982,6 @@ vtable-revert
     (when column
       (vtable-goto-column column))))
 
-(defun vtable--widths (table)
-  (nth 1 (vtable--ensure-cache table)))
-
 ;;; Commands.
 
 (defvar-keymap vtable-header-mode-map
@@ -998,7 +1006,7 @@ vtable-narrow-current-column
                                 (- (* (vtable--char-width table) (or n 1))))))
 
 (defun vtable--alter-column-width (table column delta)
-  (let ((widths (vtable--widths table)))
+  (let ((widths (vtable--cache-widths (vtable--current-cache))))
     (setf (aref widths column)
           (max (* (vtable--char-width table) 2)
                (+ (aref widths column) delta)))
@@ -1020,14 +1028,14 @@ vtable-previous-column
   (interactive)
   (vtable-goto-column
    (max 0 (1- (or (vtable-current-column)
-                  (length (vtable--widths (vtable-current-table))))))))
+                  (length (vtable--cache-widths (vtable--current-cache))))))))
 
 (defun vtable-next-column ()
   "Go to the next column."
   (interactive)
   (when (vtable-current-column)
     (vtable-goto-column
-     (min (1- (length (vtable--widths (vtable-current-table))))
+     (min (1- (length (vtable--cache-widths (vtable--current-cache))))
           (1+ (vtable-current-column))))))
 
 (defun vtable-revert-command ()
-- 
2.43.7

Message #67 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el
Date: Tue, 18 Nov 2025 15:28:51 -0500

[Message part 1 (text/plain, inline)]

On Mon, Nov 10, 2025 at 6:00 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:

> Stéphane Marks <shipmints <at> gmail.com> writes:
> > Alright! Hand becoming sufficiently usable! Sorry for the delay.
> >
> > Spencer, can you take a quick look at the patch you provided via
> 0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch
> > it does not pass the vtable tests (which we will beef up, and get rid of
> the typo in the first test name).
>
> Thanks for checking, apologies about that, it was just an error
> introduced when I ported the patch forward from emacs-30 (which is what
> we use at my site).
>
> Fixed it, this version should be correct.


Thank you.  Tests pass.

I think we should eliminate the slot -cache and get rid of the cache key
entirely (the slot being superseded by the vtable-cache text property).
Each time the cache needs to be refreshed, the user runs vtable-revert
which repopulates the now sole cache.  Leaving the remnants of the old
mechanism in place will confuse people.  I also think we should rename
current-cache to just cache.  If agreed, I'll finish this patch by removing
the remnants.

[Message part 2 (text/html, inline)]

Message #70 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in
 vtable.el
Date: Tue, 18 Nov 2025 15:40:23 -0500

Stéphane Marks <shipmints <at> gmail.com> writes:

> On Mon, Nov 10, 2025 at 6:00 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>
>  Stéphane Marks <shipmints <at> gmail.com> writes:
>  > Alright! Hand becoming sufficiently usable! Sorry for the delay.
>  >
>  > Spencer, can you take a quick look at the patch you provided via
>  0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch
>  > it does not pass the vtable tests (which we will beef up, and get rid of the typo in the first test name).
>
>  Thanks for checking, apologies about that, it was just an error
>  introduced when I ported the patch forward from emacs-30 (which is what
>  we use at my site).
>
>  Fixed it, this version should be correct.
>
> Thank you.  Tests pass.
>
> I think we should eliminate the slot -cache and get rid of the cache key entirely (the slot being superseded by the vtable-cache
> text property).  Each time the cache needs to be refreshed, the user runs vtable-revert which repopulates the now sole cache. 
> Leaving the remnants of the old mechanism in place will confuse people.  I also think we should rename current-cache to just
> cache.  If agreed, I'll finish this patch by removing the remnants.

Feel free to try it; please send your patch as a diff on top of mine so
I can review it.

Note that we still need something like the cache key just to know when
we need to invalidate the cache.  And when we revert the vtable we
currently grab the cache out of the slot -cache; we'll need to change
the code to grab the cache out of the text properties before we erase
the previously inserted table.

Message #73 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el
Date: Tue, 18 Nov 2025 15:43:40 -0500

[Message part 1 (text/plain, inline)]

On Tue, Nov 18, 2025 at 3:40 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:

> Stéphane Marks <shipmints <at> gmail.com> writes:
>
> > On Mon, Nov 10, 2025 at 6:00 PM Spencer Baugh <sbaugh <at> janestreet.com>
> wrote:
> >
> >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >  > Alright! Hand becoming sufficiently usable! Sorry for the delay.
> >  >
> >  > Spencer, can you take a quick look at the patch you provided via
> >  0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch
> >  > it does not pass the vtable tests (which we will beef up, and get rid
> of the typo in the first test name).
> >
> >  Thanks for checking, apologies about that, it was just an error
> >  introduced when I ported the patch forward from emacs-30 (which is what
> >  we use at my site).
> >
> >  Fixed it, this version should be correct.
> >
> > Thank you.  Tests pass.
> >
> > I think we should eliminate the slot -cache and get rid of the cache key
> entirely (the slot being superseded by the vtable-cache
> > text property).  Each time the cache needs to be refreshed, the user
> runs vtable-revert which repopulates the now sole cache.
> > Leaving the remnants of the old mechanism in place will confuse people.
> I also think we should rename current-cache to just
> > cache.  If agreed, I'll finish this patch by removing the remnants.
>
> Feel free to try it; please send your patch as a diff on top of mine so
> I can review it.
>
> Note that we still need something like the cache key just to know when
> we need to invalidate the cache.  And when we revert the vtable we
> currently grab the cache out of the slot -cache; we'll need to change
> the code to grab the cache out of the text properties before we erase
> the previously inserted table.
>

I don't think explicit cache invalidation is needed.  If the displays a
vtable in another window of a different size, or resizes the current
window, they have to vtable-revert anyway, right?

[Message part 2 (text/html, inline)]

Message #76 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in
 vtable.el
Date: Tue, 18 Nov 2025 15:47:27 -0500

Stéphane Marks <shipmints <at> gmail.com> writes:

> On Tue, Nov 18, 2025 at 3:40 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>
>  Stéphane Marks <shipmints <at> gmail.com> writes:
>
>  > On Mon, Nov 10, 2025 at 6:00 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>  >
>  >  Stéphane Marks <shipmints <at> gmail.com> writes:
>  >  > Alright! Hand becoming sufficiently usable! Sorry for the delay.
>  >  >
>  >  > Spencer, can you take a quick look at the patch you provided via
>  >  0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch
>  >  > it does not pass the vtable tests (which we will beef up, and get rid of the typo in the first test name).
>  >
>  >  Thanks for checking, apologies about that, it was just an error
>  >  introduced when I ported the patch forward from emacs-30 (which is what
>  >  we use at my site).
>  >
>  >  Fixed it, this version should be correct.
>  >
>  > Thank you.  Tests pass.
>  >
>  > I think we should eliminate the slot -cache and get rid of the cache key entirely (the slot being superseded by the
>  vtable-cache
>  > text property).  Each time the cache needs to be refreshed, the user runs vtable-revert which repopulates the now sole
>  cache. 
>  > Leaving the remnants of the old mechanism in place will confuse people.  I also think we should rename current-cache to
>  just
>  > cache.  If agreed, I'll finish this patch by removing the remnants.
>
>  Feel free to try it; please send your patch as a diff on top of mine so
>  I can review it.
>
>  Note that we still need something like the cache key just to know when
>  we need to invalidate the cache.  And when we revert the vtable we
>  currently grab the cache out of the slot -cache; we'll need to change
>  the code to grab the cache out of the text properties before we erase
>  the previously inserted table.
>
> I don't think explicit cache invalidation is needed.  If the displays a vtable in another window of a different size, or resizes the
> current window, they have to vtable-revert anyway, right?

Yes.  vtable-revert calls vtable-insert which doesn't clear the cache if
the cache key is the same, so it doesn't recompute the cached data.

Message #79 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el
Date: Tue, 18 Nov 2025 16:25:20 -0500

[Message part 1 (text/plain, inline)]

On Tue, Nov 18, 2025 at 3:47 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:

> Stéphane Marks <shipmints <at> gmail.com> writes:
>
> > On Tue, Nov 18, 2025 at 3:40 PM Spencer Baugh <sbaugh <at> janestreet.com>
> wrote:
> >
> >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >
> >  > On Mon, Nov 10, 2025 at 6:00 PM Spencer Baugh <sbaugh <at> janestreet.com>
> wrote:
> >  >
> >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >  >  > Alright! Hand becoming sufficiently usable! Sorry for the delay.
> >  >  >
> >  >  > Spencer, can you take a quick look at the patch you provided via
> >  >  0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch
> >  >  > it does not pass the vtable tests (which we will beef up, and get
> rid of the typo in the first test name).
> >  >
> >  >  Thanks for checking, apologies about that, it was just an error
> >  >  introduced when I ported the patch forward from emacs-30 (which is
> what
> >  >  we use at my site).
> >  >
> >  >  Fixed it, this version should be correct.
> >  >
> >  > Thank you.  Tests pass.
> >  >
> >  > I think we should eliminate the slot -cache and get rid of the cache
> key entirely (the slot being superseded by the
> >  vtable-cache
> >  > text property).  Each time the cache needs to be refreshed, the user
> runs vtable-revert which repopulates the now sole
> >  cache.
> >  > Leaving the remnants of the old mechanism in place will confuse
> people.  I also think we should rename current-cache to
> >  just
> >  > cache.  If agreed, I'll finish this patch by removing the remnants.
> >
> >  Feel free to try it; please send your patch as a diff on top of mine so
> >  I can review it.
> >
> >  Note that we still need something like the cache key just to know when
> >  we need to invalidate the cache.  And when we revert the vtable we
> >  currently grab the cache out of the slot -cache; we'll need to change
> >  the code to grab the cache out of the text properties before we erase
> >  the previously inserted table.
> >
> > I don't think explicit cache invalidation is needed.  If the displays a
> vtable in another window of a different size, or resizes the
> > current window, they have to vtable-revert anyway, right?
>
> Yes.  vtable-revert calls vtable-insert which doesn't clear the cache if
> the cache key is the same, so it doesn't recompute the cached data.
>

Here's a simplified cache patch layered on top of yours.  If you agree, I
think we should squash them.

[Message part 2 (text/html, inline)]

[0001-Simplify-vtable-cache-handling.patch (application/octet-stream, attachment)]

Message #82 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in
 vtable.el, [PATCH] Simplify vtable cache handling
Date: Tue, 18 Nov 2025 17:27:16 -0500

Stéphane Marks <shipmints <at> gmail.com> writes:

> On Tue, Nov 18, 2025 at 3:47 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>
>  Stéphane Marks <shipmints <at> gmail.com> writes:
>
>  > On Tue, Nov 18, 2025 at 3:40 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>  >
>  >  Stéphane Marks <shipmints <at> gmail.com> writes:
>  >
>  >  > On Mon, Nov 10, 2025 at 6:00 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>  >  >
>  >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
>  >  >  > Alright! Hand becoming sufficiently usable! Sorry for the delay.
>  >  >  >
>  >  >  > Spencer, can you take a quick look at the patch you provided via
>  >  >  0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch
>  >  >  > it does not pass the vtable tests (which we will beef up, and get rid of the typo in the first test name).
>  >  >
>  >  >  Thanks for checking, apologies about that, it was just an error
>  >  >  introduced when I ported the patch forward from emacs-30 (which is what
>  >  >  we use at my site).
>  >  >
>  >  >  Fixed it, this version should be correct.
>  >  >
>  >  > Thank you.  Tests pass.
>  >  >
>  >  > I think we should eliminate the slot -cache and get rid of the cache key entirely (the slot being superseded by the
>  >  vtable-cache
>  >  > text property).  Each time the cache needs to be refreshed, the user runs vtable-revert which repopulates the now sole
>  >  cache. 
>  >  > Leaving the remnants of the old mechanism in place will confuse people.  I also think we should rename current-cache to
>  >  just
>  >  > cache.  If agreed, I'll finish this patch by removing the remnants.
>  >
>  >  Feel free to try it; please send your patch as a diff on top of mine so
>  >  I can review it.
>  >
>  >  Note that we still need something like the cache key just to know when
>  >  we need to invalidate the cache.  And when we revert the vtable we
>  >  currently grab the cache out of the slot -cache; we'll need to change
>  >  the code to grab the cache out of the text properties before we erase
>  >  the previously inserted table.
>  >
>  > I don't think explicit cache invalidation is needed.  If the displays a vtable in another window of a different size, or resizes
>  the
>  > current window, they have to vtable-revert anyway, right?
>
>  Yes.  vtable-revert calls vtable-insert which doesn't clear the cache if
>  the cache key is the same, so it doesn't recompute the cached data.
>
> Here's a simplified cache patch layered on top of yours.  If you agree, I think we should squash them.

This causes us to recompute the cache every time we call vtable-revert,
instead of reusing it.  Isn't that expensive?

Message #85 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el, 
 [PATCH] Simplify vtable cache handling
Date: Tue, 18 Nov 2025 18:01:38 -0500

[Message part 1 (text/plain, inline)]

On Tue, Nov 18, 2025 at 5:27 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:

> Stéphane Marks <shipmints <at> gmail.com> writes:
>
> > On Tue, Nov 18, 2025 at 3:47 PM Spencer Baugh <sbaugh <at> janestreet.com>
> wrote:
> >
> >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >
> >  > On Tue, Nov 18, 2025 at 3:40 PM Spencer Baugh <sbaugh <at> janestreet.com>
> wrote:
> >  >
> >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >  >
> >  >  > On Mon, Nov 10, 2025 at 6:00 PM Spencer Baugh <
> sbaugh <at> janestreet.com> wrote:
> >  >  >
> >  >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >  >  >  > Alright! Hand becoming sufficiently usable! Sorry for the delay.
> >  >  >  >
> >  >  >  > Spencer, can you take a quick look at the patch you provided via
> >  >  >  0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch
> >  >  >  > it does not pass the vtable tests (which we will beef up, and
> get rid of the typo in the first test name).
> >  >  >
> >  >  >  Thanks for checking, apologies about that, it was just an error
> >  >  >  introduced when I ported the patch forward from emacs-30 (which
> is what
> >  >  >  we use at my site).
> >  >  >
> >  >  >  Fixed it, this version should be correct.
> >  >  >
> >  >  > Thank you.  Tests pass.
> >  >  >
> >  >  > I think we should eliminate the slot -cache and get rid of the
> cache key entirely (the slot being superseded by the
> >  >  vtable-cache
> >  >  > text property).  Each time the cache needs to be refreshed, the
> user runs vtable-revert which repopulates the now sole
> >  >  cache.
> >  >  > Leaving the remnants of the old mechanism in place will confuse
> people.  I also think we should rename current-cache to
> >  >  just
> >  >  > cache.  If agreed, I'll finish this patch by removing the remnants.
> >  >
> >  >  Feel free to try it; please send your patch as a diff on top of mine
> so
> >  >  I can review it.
> >  >
> >  >  Note that we still need something like the cache key just to know
> when
> >  >  we need to invalidate the cache.  And when we revert the vtable we
> >  >  currently grab the cache out of the slot -cache; we'll need to change
> >  >  the code to grab the cache out of the text properties before we erase
> >  >  the previously inserted table.
> >  >
> >  > I don't think explicit cache invalidation is needed.  If the displays
> a vtable in another window of a different size, or resizes
> >  the
> >  > current window, they have to vtable-revert anyway, right?
> >
> >  Yes.  vtable-revert calls vtable-insert which doesn't clear the cache if
> >  the cache key is the same, so it doesn't recompute the cached data.
> >
> > Here's a simplified cache patch layered on top of yours.  If you agree,
> I think we should squash them.
>
> This causes us to recompute the cache every time we call vtable-revert,
> instead of reusing it.  Isn't that expensive?
>

What I'd like to do is introduce more thoughtful cache invalidation logic
as I'd proposed in the larger patch.  The logic there was complicated by
the multi-cache infrastructure which is now gone.  I'll introduce that
logic in another patch as I go through all the other stuff I have
accumulated to add.

If we can agree on this patch, let's get it going and continue along?

[Message part 2 (text/html, inline)]

Message #88 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in
 vtable.el,  [PATCH] Simplify vtable cache handling
Date: Tue, 18 Nov 2025 18:05:53 -0500

Stéphane Marks <shipmints <at> gmail.com> writes:

> On Tue, Nov 18, 2025 at 5:27 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>
>  Stéphane Marks <shipmints <at> gmail.com> writes:
>
>  > On Tue, Nov 18, 2025 at 3:47 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>  >
>  >  Stéphane Marks <shipmints <at> gmail.com> writes:
>  >
>  >  > On Tue, Nov 18, 2025 at 3:40 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>  >  >
>  >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
>  >  >
>  >  >  > On Mon, Nov 10, 2025 at 6:00 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>  >  >  >
>  >  >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
>  >  >  >  > Alright! Hand becoming sufficiently usable! Sorry for the delay.
>  >  >  >  >
>  >  >  >  > Spencer, can you take a quick look at the patch you provided via
>  >  >  >  0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch
>  >  >  >  > it does not pass the vtable tests (which we will beef up, and get rid of the typo in the first test name).
>  >  >  >
>  >  >  >  Thanks for checking, apologies about that, it was just an error
>  >  >  >  introduced when I ported the patch forward from emacs-30 (which is what
>  >  >  >  we use at my site).
>  >  >  >
>  >  >  >  Fixed it, this version should be correct.
>  >  >  >
>  >  >  > Thank you.  Tests pass.
>  >  >  >
>  >  >  > I think we should eliminate the slot -cache and get rid of the cache key entirely (the slot being superseded by the
>  >  >  vtable-cache
>  >  >  > text property).  Each time the cache needs to be refreshed, the user runs vtable-revert which repopulates the now sole
>  >  >  cache. 
>  >  >  > Leaving the remnants of the old mechanism in place will confuse people.  I also think we should rename current-cache
>  to
>  >  >  just
>  >  >  > cache.  If agreed, I'll finish this patch by removing the remnants.
>  >  >
>  >  >  Feel free to try it; please send your patch as a diff on top of mine so
>  >  >  I can review it.
>  >  >
>  >  >  Note that we still need something like the cache key just to know when
>  >  >  we need to invalidate the cache.  And when we revert the vtable we
>  >  >  currently grab the cache out of the slot -cache; we'll need to change
>  >  >  the code to grab the cache out of the text properties before we erase
>  >  >  the previously inserted table.
>  >  >
>  >  > I don't think explicit cache invalidation is needed.  If the displays a vtable in another window of a different size, or resizes
>  >  the
>  >  > current window, they have to vtable-revert anyway, right?
>  >
>  >  Yes.  vtable-revert calls vtable-insert which doesn't clear the cache if
>  >  the cache key is the same, so it doesn't recompute the cached data.
>  >
>  > Here's a simplified cache patch layered on top of yours.  If you agree, I think we should squash them.
>
>  This causes us to recompute the cache every time we call vtable-revert,
>  instead of reusing it.  Isn't that expensive?
>
> What I'd like to do is introduce more thoughtful cache invalidation logic as I'd proposed in the larger patch.  The logic there was
> complicated by the multi-cache infrastructure which is now gone.  I'll introduce that logic in another patch as I go through all the
> other stuff I have accumulated to add.
>
> If we can agree on this patch, let's get it going and continue along?

If you're going to be introducing more thoughtful cache invalidation
logic, then please just post a patch that contains that.

As it stands, the patch you just posted will cause a performance
regression and so therefore should not be installed.

Message #91 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el, 
 [PATCH] Simplify vtable cache handling
Date: Tue, 18 Nov 2025 18:40:00 -0500

[Message part 1 (text/plain, inline)]

On Tue, Nov 18, 2025 at 18:05 Spencer Baugh <sbaugh <at> janestreet.com> wrote:

> Stéphane Marks <shipmints <at> gmail.com> writes:
>
> > On Tue, Nov 18, 2025 at 5:27 PM Spencer Baugh <sbaugh <at> janestreet.com>
> wrote:
> >
> >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >
> >  > On Tue, Nov 18, 2025 at 3:47 PM Spencer Baugh <sbaugh <at> janestreet.com>
> wrote:
> >  >
> >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >  >
> >  >  > On Tue, Nov 18, 2025 at 3:40 PM Spencer Baugh <
> sbaugh <at> janestreet.com> wrote:
> >  >  >
> >  >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >  >  >
> >  >  >  > On Mon, Nov 10, 2025 at 6:00 PM Spencer Baugh <
> sbaugh <at> janestreet.com> wrote:
> >  >  >  >
> >  >  >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >  >  >  >  > Alright! Hand becoming sufficiently usable! Sorry for the
> delay.
> >  >  >  >  >
> >  >  >  >  > Spencer, can you take a quick look at the patch you provided
> via
> >  >  >  >  0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch
> >  >  >  >  > it does not pass the vtable tests (which we will beef up,
> and get rid of the typo in the first test name).
> >  >  >  >
> >  >  >  >  Thanks for checking, apologies about that, it was just an error
> >  >  >  >  introduced when I ported the patch forward from emacs-30
> (which is what
> >  >  >  >  we use at my site).
> >  >  >  >
> >  >  >  >  Fixed it, this version should be correct.
> >  >  >  >
> >  >  >  > Thank you.  Tests pass.
> >  >  >  >
> >  >  >  > I think we should eliminate the slot -cache and get rid of the
> cache key entirely (the slot being superseded by the
> >  >  >  vtable-cache
> >  >  >  > text property).  Each time the cache needs to be refreshed, the
> user runs vtable-revert which repopulates the now sole
> >  >  >  cache.
> >  >  >  > Leaving the remnants of the old mechanism in place will confuse
> people.  I also think we should rename current-cache
> >  to
> >  >  >  just
> >  >  >  > cache.  If agreed, I'll finish this patch by removing the
> remnants.
> >  >  >
> >  >  >  Feel free to try it; please send your patch as a diff on top of
> mine so
> >  >  >  I can review it.
> >  >  >
> >  >  >  Note that we still need something like the cache key just to know
> when
> >  >  >  we need to invalidate the cache.  And when we revert the vtable we
> >  >  >  currently grab the cache out of the slot -cache; we'll need to
> change
> >  >  >  the code to grab the cache out of the text properties before we
> erase
> >  >  >  the previously inserted table.
> >  >  >
> >  >  > I don't think explicit cache invalidation is needed.  If the
> displays a vtable in another window of a different size, or resizes
> >  >  the
> >  >  > current window, they have to vtable-revert anyway, right?
> >  >
> >  >  Yes.  vtable-revert calls vtable-insert which doesn't clear the
> cache if
> >  >  the cache key is the same, so it doesn't recompute the cached data.
> >  >
> >  > Here's a simplified cache patch layered on top of yours.  If you
> agree, I think we should squash them.
> >
> >  This causes us to recompute the cache every time we call vtable-revert,
> >  instead of reusing it.  Isn't that expensive?
> >
> > What I'd like to do is introduce more thoughtful cache invalidation
> logic as I'd proposed in the larger patch.  The logic there was
> > complicated by the multi-cache infrastructure which is now gone.  I'll
> introduce that logic in another patch as I go through all the
> > other stuff I have accumulated to add.
> >
> > If we can agree on this patch, let's get it going and continue along?
>
> If you're going to be introducing more thoughtful cache invalidation
> logic, then please just post a patch that contains that.
>
> As it stands, the patch you just posted will cause a performance
> regression and so therefore should not be installed.


Alright let's install yours and keep going then.

>
>

[Message part 2 (text/html, inline)]

Message #94 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in
 vtable.el,  [PATCH] Simplify vtable cache handling
Date: Tue, 18 Nov 2025 18:42:15 -0500

Stéphane Marks <shipmints <at> gmail.com> writes:

> On Tue, Nov 18, 2025 at 18:05 Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>
>  Stéphane Marks <shipmints <at> gmail.com> writes:
>
>  > On Tue, Nov 18, 2025 at 5:27 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>  >
>  >  Stéphane Marks <shipmints <at> gmail.com> writes:
>  >
>  >  > On Tue, Nov 18, 2025 at 3:47 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>  >  >
>  >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
>  >  >
>  >  >  > On Tue, Nov 18, 2025 at 3:40 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>  >  >  >
>  >  >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
>  >  >  >
>  >  >  >  > On Mon, Nov 10, 2025 at 6:00 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:
>  >  >  >  >
>  >  >  >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
>  >  >  >  >  > Alright! Hand becoming sufficiently usable! Sorry for the delay.
>  >  >  >  >  >
>  >  >  >  >  > Spencer, can you take a quick look at the patch you provided via
>  >  >  >  >  0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch
>  >  >  >  >  > it does not pass the vtable tests (which we will beef up, and get rid of the typo in the first test name).
>  >  >  >  >
>  >  >  >  >  Thanks for checking, apologies about that, it was just an error
>  >  >  >  >  introduced when I ported the patch forward from emacs-30 (which is what
>  >  >  >  >  we use at my site).
>  >  >  >  >
>  >  >  >  >  Fixed it, this version should be correct.
>  >  >  >  >
>  >  >  >  > Thank you.  Tests pass.
>  >  >  >  >
>  >  >  >  > I think we should eliminate the slot -cache and get rid of the cache key entirely (the slot being superseded by the
>  >  >  >  vtable-cache
>  >  >  >  > text property).  Each time the cache needs to be refreshed, the user runs vtable-revert which repopulates the now
>  sole
>  >  >  >  cache. 
>  >  >  >  > Leaving the remnants of the old mechanism in place will confuse people.  I also think we should rename
>  current-cache
>  >  to
>  >  >  >  just
>  >  >  >  > cache.  If agreed, I'll finish this patch by removing the remnants.
>  >  >  >
>  >  >  >  Feel free to try it; please send your patch as a diff on top of mine so
>  >  >  >  I can review it.
>  >  >  >
>  >  >  >  Note that we still need something like the cache key just to know when
>  >  >  >  we need to invalidate the cache.  And when we revert the vtable we
>  >  >  >  currently grab the cache out of the slot -cache; we'll need to change
>  >  >  >  the code to grab the cache out of the text properties before we erase
>  >  >  >  the previously inserted table.
>  >  >  >
>  >  >  > I don't think explicit cache invalidation is needed.  If the displays a vtable in another window of a different size, or
>  resizes
>  >  >  the
>  >  >  > current window, they have to vtable-revert anyway, right?
>  >  >
>  >  >  Yes.  vtable-revert calls vtable-insert which doesn't clear the cache if
>  >  >  the cache key is the same, so it doesn't recompute the cached data.
>  >  >
>  >  > Here's a simplified cache patch layered on top of yours.  If you agree, I think we should squash them.
>  >
>  >  This causes us to recompute the cache every time we call vtable-revert,
>  >  instead of reusing it.  Isn't that expensive?
>  >
>  > What I'd like to do is introduce more thoughtful cache invalidation logic as I'd proposed in the larger patch.  The logic there
>  was
>  > complicated by the multi-cache infrastructure which is now gone.  I'll introduce that logic in another patch as I go through
>  all the
>  > other stuff I have accumulated to add.
>  >
>  > If we can agree on this patch, let's get it going and continue along?
>
>  If you're going to be introducing more thoughtful cache invalidation
>  logic, then please just post a patch that contains that.
>
>  As it stands, the patch you just posted will cause a performance
>  regression and so therefore should not be installed.
>
> Alright let's install yours and keep going then. 

I would if I could :)  I think we still need to find someone else who
will kindly install this patch.

Message #97 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el, 
 [PATCH] Simplify vtable cache handling
Date: Tue, 18 Nov 2025 19:30:33 -0500

[Message part 1 (text/plain, inline)]

On Tue, Nov 18, 2025 at 18:42 Spencer Baugh <sbaugh <at> janestreet.com> wrote:

> Stéphane Marks <shipmints <at> gmail.com> writes:
>
> > On Tue, Nov 18, 2025 at 18:05 Spencer Baugh <sbaugh <at> janestreet.com>
> wrote:
> >
> >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >
> >  > On Tue, Nov 18, 2025 at 5:27 PM Spencer Baugh <sbaugh <at> janestreet.com>
> wrote:
> >  >
> >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >  >
> >  >  > On Tue, Nov 18, 2025 at 3:47 PM Spencer Baugh <
> sbaugh <at> janestreet.com> wrote:
> >  >  >
> >  >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >  >  >
> >  >  >  > On Tue, Nov 18, 2025 at 3:40 PM Spencer Baugh <
> sbaugh <at> janestreet.com> wrote:
> >  >  >  >
> >  >  >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >  >  >  >
> >  >  >  >  > On Mon, Nov 10, 2025 at 6:00 PM Spencer Baugh <
> sbaugh <at> janestreet.com> wrote:
> >  >  >  >  >
> >  >  >  >  >  Stéphane Marks <shipmints <at> gmail.com> writes:
> >  >  >  >  >  > Alright! Hand becoming sufficiently usable! Sorry for the
> delay.
> >  >  >  >  >  >
> >  >  >  >  >  > Spencer, can you take a quick look at the patch you
> provided via
> >  >  >  >  >
> 0001-Fix-implicit-usage-of-the-current-window-width-in-vt.patch
> >  >  >  >  >  > it does not pass the vtable tests (which we will beef up,
> and get rid of the typo in the first test name).
> >  >  >  >  >
> >  >  >  >  >  Thanks for checking, apologies about that, it was just an
> error
> >  >  >  >  >  introduced when I ported the patch forward from emacs-30
> (which is what
> >  >  >  >  >  we use at my site).
> >  >  >  >  >
> >  >  >  >  >  Fixed it, this version should be correct.
> >  >  >  >  >
> >  >  >  >  > Thank you.  Tests pass.
> >  >  >  >  >
> >  >  >  >  > I think we should eliminate the slot -cache and get rid of
> the cache key entirely (the slot being superseded by the
> >  >  >  >  vtable-cache
> >  >  >  >  > text property).  Each time the cache needs to be refreshed,
> the user runs vtable-revert which repopulates the now
> >  sole
> >  >  >  >  cache.
> >  >  >  >  > Leaving the remnants of the old mechanism in place will
> confuse people.  I also think we should rename
> >  current-cache
> >  >  to
> >  >  >  >  just
> >  >  >  >  > cache.  If agreed, I'll finish this patch by removing the
> remnants.
> >  >  >  >
> >  >  >  >  Feel free to try it; please send your patch as a diff on top
> of mine so
> >  >  >  >  I can review it.
> >  >  >  >
> >  >  >  >  Note that we still need something like the cache key just to
> know when
> >  >  >  >  we need to invalidate the cache.  And when we revert the
> vtable we
> >  >  >  >  currently grab the cache out of the slot -cache; we'll need to
> change
> >  >  >  >  the code to grab the cache out of the text properties before
> we erase
> >  >  >  >  the previously inserted table.
> >  >  >  >
> >  >  >  > I don't think explicit cache invalidation is needed.  If the
> displays a vtable in another window of a different size, or
> >  resizes
> >  >  >  the
> >  >  >  > current window, they have to vtable-revert anyway, right?
> >  >  >
> >  >  >  Yes.  vtable-revert calls vtable-insert which doesn't clear the
> cache if
> >  >  >  the cache key is the same, so it doesn't recompute the cached
> data.
> >  >  >
> >  >  > Here's a simplified cache patch layered on top of yours.  If you
> agree, I think we should squash them.
> >  >
> >  >  This causes us to recompute the cache every time we call
> vtable-revert,
> >  >  instead of reusing it.  Isn't that expensive?
> >  >
> >  > What I'd like to do is introduce more thoughtful cache invalidation
> logic as I'd proposed in the larger patch.  The logic there
> >  was
> >  > complicated by the multi-cache infrastructure which is now gone.
> I'll introduce that logic in another patch as I go through
> >  all the
> >  > other stuff I have accumulated to add.
> >  >
> >  > If we can agree on this patch, let's get it going and continue along?
> >
> >  If you're going to be introducing more thoughtful cache invalidation
> >  logic, then please just post a patch that contains that.
> >
> >  As it stands, the patch you just posted will cause a performance
> >  regression and so therefore should not be installed.
> >
> > Alright let's install yours and keep going then.
>
> I would if I could :)  I think we still need to find someone else who
> will kindly install this patch.


I was hoping as part of volunteering to be the vtable maintainer that I'd
be able to and agree to touch only that file (and documentation).

>
>

[Message part 2 (text/html, inline)]

Message #100 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: adam <at> alphapapa.net, 69837 <at> debbugs.gnu.org, shipmints <at> gmail.com,
 stefankangas <at> gmail.com
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in
 vtable.el,  [PATCH] Simplify vtable cache handling
Date: Wed, 19 Nov 2025 15:20:52 +0200

> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Cc: Adam Porter <adam <at> alphapapa.net>,  69837 <at> debbugs.gnu.org,  Eli Zaretskii
>   <eliz <at> gnu.org>,  Stefan Kangas <stefankangas <at> gmail.com>
> Date: Tue, 18 Nov 2025 18:42:15 -0500
> 
> Stéphane Marks <shipmints <at> gmail.com> writes:
> 
> > Alright let's install yours and keep going then. 
> 
> I would if I could :)  I think we still need to find someone else who
> will kindly install this patch.

Is this the patch in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=69837#64 ?

If so, I can install it.

Message #103 received at 69837 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org,
 Ship Mints <shipmints <at> gmail.com>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el, 
 [PATCH] Simplify vtable cache handling
Date: Wed, 19 Nov 2025 08:32:09 -0500

[Message part 1 (text/plain, inline)]

On Wed, Nov 19, 2025, 8:20 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Spencer Baugh <sbaugh <at> janestreet.com>
> > Cc: Adam Porter <adam <at> alphapapa.net>,  69837 <at> debbugs.gnu.org,  Eli
> Zaretskii
> >   <eliz <at> gnu.org>,  Stefan Kangas <stefankangas <at> gmail.com>
> > Date: Tue, 18 Nov 2025 18:42:15 -0500
> >
> > Stéphane Marks <shipmints <at> gmail.com> writes:
> >
> > > Alright let's install yours and keep going then.
> >
> > I would if I could :)  I think we still need to find someone else who
> > will kindly install this patch.
>
> Is this the patch in
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=69837#64 ?
>
> If so, I can install it.
>

Yes

>

[Message part 2 (text/html, inline)]

Message #108 received at 69837-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: adam <at> alphapapa.net, 69837-done <at> debbugs.gnu.org, shipmints <at> gmail.com,
 stefankangas <at> gmail.com
Subject: Re: bug#69837: 29.2; vtable-update-object only works in visible
 windows, [PATCH] Fix implicit usage of the current window-width in vtable.el, 
 [PATCH] Simplify vtable cache handling
Date: Wed, 19 Nov 2025 17:29:08 +0200

> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Date: Wed, 19 Nov 2025 08:32:09 -0500
> Cc: Ship Mints <shipmints <at> gmail.com>, Adam Porter <adam <at> alphapapa.net>, 69837 <at> debbugs.gnu.org, 
>  	Stefan Kangas <stefankangas <at> gmail.com>
> 
> On Wed, Nov 19, 2025, 8:20 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>  > From: Spencer Baugh <sbaugh <at> janestreet.com>
>  > Cc: Adam Porter <adam <at> alphapapa.net>,  69837 <at> debbugs.gnu.org,  Eli Zaretskii
>  >   <eliz <at> gnu.org>,  Stefan Kangas <stefankangas <at> gmail.com>
>  > Date: Tue, 18 Nov 2025 18:42:15 -0500
>  > 
>  > Stéphane Marks <shipmints <at> gmail.com> writes:
>  > 
>  > > Alright let's install yours and keep going then. 
>  > 
>  > I would if I could :)  I think we still need to find someone else who
>  > will kindly install this patch.
> 
>  Is this the patch in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=69837#64 ?
> 
>  If so, I can install it.
> 
> Yes 

Thanks, installed on master, and closing the bug.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.