GNU bug report logs - #78843
Various vtable bug fixes and enhancements

Previous Next

To reply to this bug, email your comments to 78843 AT debbugs.gnu.org.

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

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

From: Stéphane Marks <shipmints <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Various vtable bug fixes and enhancements
Date: Thu, 19 Jun 2025 16:24:06 -0400

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

Bug for a consolidated patch which is in the process of being prepared.

-Stéphane

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

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

From: Stéphane Marks <shipmints <at> gmail.com>
To: 78843 <at> debbugs.gnu.org
Cc: Adam Porter <adam <at> alphapapa.net>, Spencer Baugh <sbaugh <at> janestreet.com>,
 Lars Ingebrigtsen <larsi <at> gnus.org>, Augusto Stoffel <arstoffel <at> gmail.com>,
 Kristoffer Balintona <krisbalintona <at> gmail.com>
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Sat, 21 Jun 2025 11:41:50 -0400

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

On Thu, Jun 19, 2025 at 4:25 PM Stéphane Marks <shipmints <at> gmail.com> wrote:

> Bug for a consolidated patch which is in the process of being prepared.
>

Large patch attached.  All the changes, aside from bug fixes, should be
backward compatible.  I've cc'd a few of the major vtable users whose code
bases helped inform many bug fixes and enhancements.  Some of you have
helped test and provide feedback along the way.  Please try this out (and
review the updated texinfo documentation if you want to).

Feedback here and/or to me directly is fine.

-Stéphane

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

[0001-vtable-bug-fixes-and-enhancements-bug-78843.patch (application/octet-stream, attachment)]

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

From: Stéphane Marks <shipmints <at> gmail.com>
To: 78843 <at> debbugs.gnu.org
Cc: Adam Porter <adam <at> alphapapa.net>, Spencer Baugh <sbaugh <at> janestreet.com>,
 Lars Ingebrigtsen <larsi <at> gnus.org>, Augusto Stoffel <arstoffel <at> gmail.com>,
 Kristoffer Balintona <krisbalintona <at> gmail.com>
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Sat, 21 Jun 2025 13:40:21 -0400

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

On Sat, Jun 21, 2025 at 11:41 AM Stéphane Marks <shipmints <at> gmail.com> wrote:

> On Thu, Jun 19, 2025 at 4:25 PM Stéphane Marks <shipmints <at> gmail.com>
> wrote:
>
>> Bug for a consolidated patch which is in the process of being prepared.
>>
>
> Large patch attached.  All the changes, aside from bug fixes, should be
> backward compatible.  I've cc'd a few of the major vtable users whose code
> bases helped inform many bug fixes and enhancements.  Some of you have
> helped test and provide feedback along the way.  Please try this out (and
> review the updated texinfo documentation if you want to).
>
> Feedback here and/or to me directly is fine.
>

After double checking comint-mime, it looks like I should add
header-text-properties just as I did row-text-properties so that
comint-mime doesn't need shenanigans.  On the list.  What else?

-Stéphane

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: sbaugh <at> janestreet.com, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org,
 adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Sun, 22 Jun 2025 11:59:21 +0300

> Cc: Adam Porter <adam <at> alphapapa.net>, Spencer Baugh <sbaugh <at> janestreet.com>,
>  Lars Ingebrigtsen <larsi <at> gnus.org>, Augusto Stoffel <arstoffel <at> gmail.com>,
>  Kristoffer Balintona <krisbalintona <at> gmail.com>
> From: Stéphane Marks <shipmints <at> gmail.com>
> Date: Sat, 21 Jun 2025 11:41:50 -0400
> 
> On Thu, Jun 19, 2025 at 4:25 PM Stéphane Marks <shipmints <at> gmail.com> wrote:
> 
>  Bug for a consolidated patch which is in the process of being prepared.
> 
> Large patch attached.  All the changes, aside from bug fixes, should be backward compatible.  I've cc'd a few
> of the major vtable users whose code bases helped inform many bug fixes and enhancements.  Some of you
> have helped test and provide feedback along the way.  Please try this out (and review the updated texinfo
> documentation if you want to).

Thanks.

Is it really not feasible to have separate patches for each bugfix and
each enhancement (or coherent set of enhancements)?  What if we decide
to install just some of these changes -- you'd need to update the
entire large patch, instead of dropping a relatively small one and
leaving the others intact.

For now, just a few comments from superficial first reading, mainly
about the documentation.

> +A vtable can be inserted into a single buffer only.  An error is
> +signaled if a vtable is attempted to be inserted more than once into a
> +single buffer, or into multiple buffers.

The passive voice here is used way too much.  Please rephrase not to
use it.

>  @table @code
> +@item :name
> +The default table name is `*vtable*`.  Specify this if you have more
                             ^^^^^^^^^^
This should be @samp{*vtable*}.

> -In the latter case, if @code{:columns} is non-@code{nil} and there's
> -more elements in the sequence than there is in @code{:columns}, only
> -the @code{:columns} first elements are displayed.
> +In the latter case, if @code{:columns} is non-@code{nil} and there are
> +more elements in the sequence than there are in @code{:columns}, the
> +elements beyond those in @code{:columns} are not displayed.

Since the elements are not in :columns, the modified text is
confusing, IMO.  I suggest

  ... the sequence elements beyond the number of elements in
  @code{:columns} will not be shown.

> +@item :object-equal
> +This function tests for the equality of two table objects.  It defaults
> +to @code{eq}.
> +
> +@defun object-equal object1 object2
> +Return non-nil if @var{object1} and @var{object2} are equal, and non-nil
> +otherwise.
> +@end defun

I don't understand the purpose of this @defun here.  What did you
intend to say by placing it here?

> +@item :duplicate-objects
> +A vtable assumes all objects inserted into the table are unique among
> +themselves.  If the symbol @code{'allow}, the default, allow duplicate
> +objects (the first object found during table operations has primacy), if

First, please say "If the value is the symbol...", not just "If the
symbol".  And second, the way we typeset symbols in a Texinfo manual
is by using @code{allow}, without the apostrophe.

> +@code{'ignore}, silently ignore duplicates leaving the existing object
> +intact, if @code{'ignore-warn} do the same thing as @code{'ignore} and
> +produce a warning message, or if @code{'error}, signal an error if a
> +duplicate is detected.

Likewise here about the symbol names.  In addition, please use
semi-colons to separate the description of the different values:

  ...if it's |@code{ignore}, silently ignore duplicates leaving the
  existing object intact; if it's @code{ignore-warn}, do the same...

> +This is a list where each element is either a string (the column name),
> +a plist of keyword/values (to make a @code{vtable-column} object), or a
> +@code{vtable-column} object (created by calling the function
> +@code{make-vtable-column}).

When you mention a function or a variable, like make-vtable-column
here, it is usually a good idea to have a cross-reference to where
they are described (unless they are described in the same node).

> +that need special treatment, specify a larger guess increment.  If nil,
                                                                      ^^^
"nil" should have the @code markup.

> +@item comparator
> +This function will be called to compare column values.  Use this when
> +your objects contain data that needs non-numeric or string collation,
> +for example, dates.  Another case would be where you want the equivalent
> +of @code{sort-fold-case}.
> +
> +@defun comparator value1 value2
> +This function is called with two values to compare.  The return value
> +should follow the semantics of @code{<} or, @code{string-lessp}.
>  @end defun

Same question here about this @defun, as above about object-equal.

> +@item :sort-indicator
> +This is a list of two cons cells that specify the sort ascending and
> +descending characters that are shown on the table header to indicate a
> +sorted column.  The first character is the fancier ``graphical''
> +character, and the second a text-only character.  If the first character
> +cannot be displayed on the selected frame, the table will show the
> +text-only character.  The default is
> +@code{vtable-sort-indicator-default}.  For example:
> +
> +@lisp
> +'((?▼ ?v) (?▲ ?^)) ; This is the default.
> +'((?⬇ ?v) (?⬆ ?^)) ; Alternative arrows.
> +@end lisp

Using non-ASCII characters in a manual runs the risk of producing
incorrect output in the printed manual, due to lack of support in the
fonts used by TeX.  It is better to avoid that.  Here, you use them in
the example, which seems unnecessary, so I suggest to rewrite the
example using only ASCII characters (you can mention non-ASCII as a
possibility without actually showing such an example).

> +@table @kbd
> +@findex vtable-next-line
> +@item n
> +@item <down>

Our conventions are to UPCASE arrow key names, so please use DOWN, UP,
etc.

Also, since "DOWN" is a label of the key, you should use @key{DOWN},
not <DOWN>.

> +@findex vtable-goto-next-table
> +@item <forward-paragraph>

What's "<forward-paragraph>"?  Are there keyboards with a key that has
such a label?

> --- a/lisp/emacs-lisp/vtable.el
> +++ b/lisp/emacs-lisp/vtable.el
> @@ -30,77 +30,220 @@
> 
>  (defface vtable
>    '((t :inherit variable-pitch))
> -  "Face used (by default) for vtables."
> +  "Face used (by default) for vtable bodies."
>    :version "29.1"
>    :group 'faces)
> 
> +(defface vtable-header
> +  '((t :inherit (header-line vtable)))
> +  "Face used (by default) for vtable headers."
> +  :version "31.1"
> +  :group 'faces)
> +
> +(defface vtable-marked
> +  '((t :inherit region))
> +  "Face used (by default) for marked vtable objects."
> +  :version "31.1"
> +  :group 'faces)
> +
> +(defface vtable-sort-indicator-ascend
> +  '((t :inherit vtable-header))
> +  "Face used (by default) for vtable ascend sort indicator."
> +  :version "31.1"
> +  :group 'faces)
> +
> +(defface vtable-sort-indicator-descend
> +  '((t :inherit vtable-header))
> +  "Face used (by default) for vtable descend sort indicator."
> +  :version "31.1"
> +  :group 'faces)

Shouldn't these faces be in the vtable group instead?

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

From: Stéphane Marks <shipmints <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sbaugh <at> janestreet.com, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org,
 adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Sun, 22 Jun 2025 05:12:46 -0400

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

On Sun, Jun 22, 2025 at 4:59 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > Cc: Adam Porter <adam <at> alphapapa.net>, Spencer Baugh <
> sbaugh <at> janestreet.com>,
> >  Lars Ingebrigtsen <larsi <at> gnus.org>, Augusto Stoffel <
> arstoffel <at> gmail.com>,
> >  Kristoffer Balintona <krisbalintona <at> gmail.com>
> > From: Stéphane Marks <shipmints <at> gmail.com>
> > Date: Sat, 21 Jun 2025 11:41:50 -0400
> >
> > On Thu, Jun 19, 2025 at 4:25 PM Stéphane Marks <shipmints <at> gmail.com>
> wrote:
> >
> >  Bug for a consolidated patch which is in the process of being prepared.
> >
> > Large patch attached.  All the changes, aside from bug fixes, should be
> backward compatible.  I've cc'd a few
> > of the major vtable users whose code bases helped inform many bug fixes
> and enhancements.  Some of you
> > have helped test and provide feedback along the way.  Please try this
> out (and review the updated texinfo
> > documentation if you want to).
>
> Thanks.
>
> Is it really not feasible to have separate patches for each bugfix and
> each enhancement (or coherent set of enhancements)?  What if we decide
> to install just some of these changes -- you'd need to update the
> entire large patch, instead of dropping a relatively small one and
> leaving the others intact.
>

I hope we elect to adopt them all.  It would be much more work to tease
them all out piecemeal and wouldn't really, in the end, produce a better
vtable.

For now, just a few comments from superficial first reading, mainly
> about the documentation.
>

I'll review the below and adjust.  Much of the style was just carried over
from the original vtable doc.  Happy to improve it.

WRT quoting symbols, there are examples in the texinfo doc collection that
suggest the convention is not enforced:

./misc/srecode.texi:such as the symbol @code{'prototype} for prototype
functions, or

./misc/ses.texi:@code{'ses-prin1}.

./misc/idlwave.texi:the single symbol @code{'everything}, all the copious
shell input is

./misc/auth.texi:Set this variable to @code{'trivia} to see lots of output
in


> > +A vtable can be inserted into a single buffer only.  An error is
> > +signaled if a vtable is attempted to be inserted more than once into a
> > +single buffer, or into multiple buffers.
>
> The passive voice here is used way too much.  Please rephrase not to
> use it.
>
> >  @table @code
> > +@item :name
> > +The default table name is `*vtable*`.  Specify this if you have more
>                              ^^^^^^^^^^
> This should be @samp{*vtable*}.
>
> > -In the latter case, if @code{:columns} is non-@code{nil} and there's
> > -more elements in the sequence than there is in @code{:columns}, only
> > -the @code{:columns} first elements are displayed.
> > +In the latter case, if @code{:columns} is non-@code{nil} and there are
> > +more elements in the sequence than there are in @code{:columns}, the
> > +elements beyond those in @code{:columns} are not displayed.
>
> Since the elements are not in :columns, the modified text is
> confusing, IMO.  I suggest
>
>   ... the sequence elements beyond the number of elements in
>   @code{:columns} will not be shown.
>
> > +@item :object-equal
> > +This function tests for the equality of two table objects.  It defaults
> > +to @code{eq}.
> > +
> > +@defun object-equal object1 object2
> > +Return non-nil if @var{object1} and @var{object2} are equal, and non-nil
> > +otherwise.
> > +@end defun
>
> I don't understand the purpose of this @defun here.  What did you
> intend to say by placing it here?
>
> > +@item :duplicate-objects
> > +A vtable assumes all objects inserted into the table are unique among
> > +themselves.  If the symbol @code{'allow}, the default, allow duplicate
> > +objects (the first object found during table operations has primacy), if
>
> First, please say "If the value is the symbol...", not just "If the
> symbol".  And second, the way we typeset symbols in a Texinfo manual
> is by using @code{allow}, without the apostrophe.
>
> > +@code{'ignore}, silently ignore duplicates leaving the existing object
> > +intact, if @code{'ignore-warn} do the same thing as @code{'ignore} and
> > +produce a warning message, or if @code{'error}, signal an error if a
> > +duplicate is detected.
>
> Likewise here about the symbol names.  In addition, please use
> semi-colons to separate the description of the different values:
>
>   ...if it's |@code{ignore}, silently ignore duplicates leaving the
>   existing object intact; if it's @code{ignore-warn}, do the same...
>
> > +This is a list where each element is either a string (the column name),
> > +a plist of keyword/values (to make a @code{vtable-column} object), or a
> > +@code{vtable-column} object (created by calling the function
> > +@code{make-vtable-column}).
>
> When you mention a function or a variable, like make-vtable-column
> here, it is usually a good idea to have a cross-reference to where
> they are described (unless they are described in the same node).
>
> > +that need special treatment, specify a larger guess increment.  If nil,
>                                                                       ^^^
> "nil" should have the @code markup.
>
> > +@item comparator
> > +This function will be called to compare column values.  Use this when
> > +your objects contain data that needs non-numeric or string collation,
> > +for example, dates.  Another case would be where you want the equivalent
> > +of @code{sort-fold-case}.
> > +
> > +@defun comparator value1 value2
> > +This function is called with two values to compare.  The return value
> > +should follow the semantics of @code{<} or, @code{string-lessp}.
> >  @end defun
>
> Same question here about this @defun, as above about object-equal.
>
> > +@item :sort-indicator
> > +This is a list of two cons cells that specify the sort ascending and
> > +descending characters that are shown on the table header to indicate a
> > +sorted column.  The first character is the fancier ``graphical''
> > +character, and the second a text-only character.  If the first character
> > +cannot be displayed on the selected frame, the table will show the
> > +text-only character.  The default is
> > +@code{vtable-sort-indicator-default}.  For example:
> > +
> > +@lisp
> > +'((?▼ ?v) (?▲ ?^)) ; This is the default.
> > +'((?⬇ ?v) (?⬆ ?^)) ; Alternative arrows.
> > +@end lisp
>
> Using non-ASCII characters in a manual runs the risk of producing
> incorrect output in the printed manual, due to lack of support in the
> fonts used by TeX.  It is better to avoid that.  Here, you use them in
> the example, which seems unnecessary, so I suggest to rewrite the
> example using only ASCII characters (you can mention non-ASCII as a
> possibility without actually showing such an example).
>
> > +@table @kbd
> > +@findex vtable-next-line
> > +@item n
> > +@item <down>
>
> Our conventions are to UPCASE arrow key names, so please use DOWN, UP,
> etc.
>
> Also, since "DOWN" is a label of the key, you should use @key{DOWN},
> not <DOWN>.
>
> > +@findex vtable-goto-next-table
> > +@item <forward-paragraph>
>
> What's "<forward-paragraph>"?  Are there keyboards with a key that has
> such a label?
>
> > --- a/lisp/emacs-lisp/vtable.el
> > +++ b/lisp/emacs-lisp/vtable.el
> > @@ -30,77 +30,220 @@
> >
> >  (defface vtable
> >    '((t :inherit variable-pitch))
> > -  "Face used (by default) for vtables."
> > +  "Face used (by default) for vtable bodies."
> >    :version "29.1"
> >    :group 'faces)
> >
> > +(defface vtable-header
> > +  '((t :inherit (header-line vtable)))
> > +  "Face used (by default) for vtable headers."
> > +  :version "31.1"
> > +  :group 'faces)
> > +
> > +(defface vtable-marked
> > +  '((t :inherit region))
> > +  "Face used (by default) for marked vtable objects."
> > +  :version "31.1"
> > +  :group 'faces)
> > +
> > +(defface vtable-sort-indicator-ascend
> > +  '((t :inherit vtable-header))
> > +  "Face used (by default) for vtable ascend sort indicator."
> > +  :version "31.1"
> > +  :group 'faces)
> > +
> > +(defface vtable-sort-indicator-descend
> > +  '((t :inherit vtable-header))
> > +  "Face used (by default) for vtable descend sort indicator."
> > +  :version "31.1"
> > +  :group 'faces)
>
> Shouldn't these faces be in the vtable group instead?
>

Yessir.

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: sbaugh <at> janestreet.com, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org,
 adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Sun, 22 Jun 2025 12:41:56 +0300

> From: Stéphane Marks <shipmints <at> gmail.com>
> Date: Sun, 22 Jun 2025 05:12:46 -0400
> Cc: 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, sbaugh <at> janestreet.com, 
> 	larsi <at> gnus.org, arstoffel <at> gmail.com, krisbalintona <at> gmail.com
> 
> On Sun, Jun 22, 2025 at 4:59 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>  Is it really not feasible to have separate patches for each bugfix and
>  each enhancement (or coherent set of enhancements)?  What if we decide
>  to install just some of these changes -- you'd need to update the
>  entire large patch, instead of dropping a relatively small one and
>  leaving the others intact.
> 
> I hope we elect to adopt them all.  It would be much more work to tease them all out piecemeal and wouldn't
> really, in the end, produce a better vtable.

Please also consider the inherent difficulties in reviewing such a
large (4K lines!) patch.

> WRT quoting symbols, there are examples in the texinfo doc collection that suggest the convention is not
> enforced:

They are all bugs that need to be fixed.  Patches welcome.  "Not
enforced" would be a wrong conclusion from these omissions.

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

From: Stéphane Marks <shipmints <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sbaugh <at> janestreet.com, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org,
 adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Sun, 22 Jun 2025 05:45:59 -0400

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

On Sun, Jun 22, 2025 at 5:42 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Stéphane Marks <shipmints <at> gmail.com>
> > Date: Sun, 22 Jun 2025 05:12:46 -0400
> > Cc: 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, sbaugh <at> janestreet.com,
> >       larsi <at> gnus.org, arstoffel <at> gmail.com, krisbalintona <at> gmail.com
> >
> > On Sun, Jun 22, 2025 at 4:59 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> >  Is it really not feasible to have separate patches for each bugfix and
> >  each enhancement (or coherent set of enhancements)?  What if we decide
> >  to install just some of these changes -- you'd need to update the
> >  entire large patch, instead of dropping a relatively small one and
> >  leaving the others intact.
> >
> > I hope we elect to adopt them all.  It would be much more work to tease
> them all out piecemeal and wouldn't
> > really, in the end, produce a better vtable.
>
> Please also consider the inherent difficulties in reviewing such a
> large (4K lines!) patch.
>

Let's start with the vtable users I looped in actually using it.  I can
attach a vtable.el without the patch file and make it easier.

> WRT quoting symbols, there are examples in the texinfo doc collection
> that suggest the convention is not
> > enforced:
>
> They are all bugs that need to be fixed.  Patches welcome.  "Not
> enforced" would be a wrong conclusion from these omissions.
>

Cool.  I have a patch or two to ediff coming and ediff doesn't follow ELisp
formatting conventions which I found confusing.

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: sbaugh <at> janestreet.com, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org,
 adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Sun, 22 Jun 2025 13:02:50 +0300

> From: Stéphane Marks <shipmints <at> gmail.com>
> Date: Sun, 22 Jun 2025 05:45:59 -0400
> Cc: 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, sbaugh <at> janestreet.com, 
> 	larsi <at> gnus.org, arstoffel <at> gmail.com, krisbalintona <at> gmail.com
> 
> On Sun, Jun 22, 2025 at 5:42 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>  > From: Stéphane Marks <shipmints <at> gmail.com>
>  > Date: Sun, 22 Jun 2025 05:12:46 -0400
>  > Cc: 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, sbaugh <at> janestreet.com, 
>  >       larsi <at> gnus.org, arstoffel <at> gmail.com, krisbalintona <at> gmail.com
>  > 
>  > On Sun, Jun 22, 2025 at 4:59 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>  > 
>  >  Is it really not feasible to have separate patches for each bugfix and
>  >  each enhancement (or coherent set of enhancements)?  What if we decide
>  >  to install just some of these changes -- you'd need to update the
>  >  entire large patch, instead of dropping a relatively small one and
>  >  leaving the others intact.
>  > 
>  > I hope we elect to adopt them all.  It would be much more work to tease them all out piecemeal and
>  wouldn't
>  > really, in the end, produce a better vtable.
> 
>  Please also consider the inherent difficulties in reviewing such a
>  large (4K lines!) patch.
> 
> Let's start with the vtable users I looped in actually using it.

I don't see how this will help in reviewing the patch. It will allow
those folks to try the patch, but that's a separate job.  Patch review
has to happen regardless, because it might spot issues that no
reasonable amount of testing can.  This is why patch review is a
necessary step in our development process.

> I can attach a vtable.el without the patch file and
> make it easier.

Thanks, but it won't make it easier, no.

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

From: Stéphane Marks <shipmints <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sbaugh <at> janestreet.com, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org,
 adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Sun, 22 Jun 2025 06:24:39 -0400

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

On Sun, Jun 22, 2025 at 6:02 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Stéphane Marks <shipmints <at> gmail.com>
> > Date: Sun, 22 Jun 2025 05:45:59 -0400
> > Cc: 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, sbaugh <at> janestreet.com,
> >       larsi <at> gnus.org, arstoffel <at> gmail.com, krisbalintona <at> gmail.com
> >
> > On Sun, Jun 22, 2025 at 5:42 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> >  > From: Stéphane Marks <shipmints <at> gmail.com>
> >  > Date: Sun, 22 Jun 2025 05:12:46 -0400
> >  > Cc: 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, sbaugh <at> janestreet.com,
>
> >  >       larsi <at> gnus.org, arstoffel <at> gmail.com, krisbalintona <at> gmail.com
> >  >
> >  > On Sun, Jun 22, 2025 at 4:59 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >  >
> >  >  Is it really not feasible to have separate patches for each bugfix
> and
> >  >  each enhancement (or coherent set of enhancements)?  What if we
> decide
> >  >  to install just some of these changes -- you'd need to update the
> >  >  entire large patch, instead of dropping a relatively small one and
> >  >  leaving the others intact.
> >  >
> >  > I hope we elect to adopt them all.  It would be much more work to
> tease them all out piecemeal and
> >  wouldn't
> >  > really, in the end, produce a better vtable.
> >
> >  Please also consider the inherent difficulties in reviewing such a
> >  large (4K lines!) patch.
> >
> > Let's start with the vtable users I looped in actually using it.
>
> I don't see how this will help in reviewing the patch. It will allow
> those folks to try the patch, but that's a separate job.  Patch review
> has to happen regardless, because it might spot issues that no
> reasonable amount of testing can.  This is why patch review is a
> necessary step in our development process.
>
> > I can attach a vtable.el without the patch file and
> > make it easier.
>
> Thanks, but it won't make it easier, no.
>

Of course.  I did say that I would submit a "big-bang" patch a few weeks
ago.  The vtable users who are familiar with the code (having implemented
their workarounds) should have an easier time with the review.
Fortunately, vtable is still young so fewer users.  I've been using it
every day and I enjoy the revised one much more.

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: sbaugh <at> janestreet.com, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org,
 adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Sun, 22 Jun 2025 13:35:21 +0300

> From: Stéphane Marks <shipmints <at> gmail.com>
> Date: Sun, 22 Jun 2025 06:24:39 -0400
> Cc: 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, sbaugh <at> janestreet.com, 
> 	larsi <at> gnus.org, arstoffel <at> gmail.com, krisbalintona <at> gmail.com
> 
>  > Let's start with the vtable users I looped in actually using it.
> 
>  I don't see how this will help in reviewing the patch. It will allow
>  those folks to try the patch, but that's a separate job.  Patch review
>  has to happen regardless, because it might spot issues that no
>  reasonable amount of testing can.  This is why patch review is a
>  necessary step in our development process.
> 
>  > I can attach a vtable.el without the patch file and
>  > make it easier.
> 
>  Thanks, but it won't make it easier, no.
> 
> Of course.  I did say that I would submit a "big-bang" patch a few weeks ago.  The vtable users who are
> familiar with the code (having implemented their workarounds) should have an easier time with the review. 

That will help, but I would like to review it as well.  It's part of
my job.

Your patch lists many changes:

> Bug fixes
> 
> - vtable-update-object works with no visible or changed window
> width cache key (bug=69837)
> - Improve vtable--limit-string performance for long
> strings (bug=77684)
> - Keymap changes:
>   - Keymap spans the whole header column and header
>   dividers (bug=74701)
>   - Missing keymap and 'vtable-column property on body/header
>   dividers and newlines
>   - mouse-1 and close table bindings in vtable-header-line-map
>   keymap
>   - table-drag-resize-column-map keymap for use in body on
>   dividers (bug=74701)
>   - vtable-header-drag-resize-column-map consolidates
>   vtable-header-line-map and vtable-drag-resize-column-map
> - Infer column widths based on formatted elements, not raw
> elements
> - Compute columns works when there are no objects at
> initialization
> - text-scale-mode support (header and body elements are pixel
> aligned)
> - Header line handles display-line-numbers-mode
> - Redraw row colors on lines below an inserted or removed line
> - Cache is coherent with its objects (rather than by chance),
> invalidated where needed
> - All cache references now cache warming calls vs. mere cache
> get
> - Table mutations work when the table buffer is not the current
> buffer and if the table isn't current
> - Resize all tables in a buffer when their window
> resizes (debounced to accommodate frame size dragging)
> - Spacer goes after the column name on the header if right
> aligned
> - Column name defaults to right-aligned when its data are
> right-aligned
> - Improve column placeholder message when no objects and no
> columns are specified
> - Do not display divider after a row's final column
> - Option to display divider on the header (was always on)
> - Treat divider as a part of its preceding column when dragging
> or sorting
> - Append table faces to values and column heading names rather
> than override
> - Truncated string ellipsis text properties match those of the
> formatted column entry
> - Clicking on a header keeps point within table bounds
> - Use vtable-object-equal and do not assume eq (or via assq,
> memq)
> - Header line adjust final column name and sort indicator in a
> narrow window works as intended
> - Set window point where needed
> - Next/previous line remains in the current column
> - Signal an error if a table is inserted in more than one buffer
> or more than once in a buffer
> - vtable--cache-key handle if called when the selected window
> does not contain a table buffer
> - Restore "focus" after vtable-remove-object,
> vtable-update-object, vtable-redisplay-range, vtable-revert if
> table is current
> - vtable-revert works when reverted table is not the current
> table
> - Mutate the correct associated table in
> vtable-header-line-sort, vtable--drag-resize-column
> - vtable--recompute-cache considers the table's sort order
> - vtable--alter-column-width respects min-width and max-width
> - vtable-insert-object if integer location specified, does not
> assume the object list and line cache are in the same order
> - vtable-insert-object signal an error if integer insert
> location specified when the table is sorted
> - vtable-update-object does not assume objects are in the same
> order as the cache, which might be sorted
> - vtable-update-object no longer signal "setcar nil" on failed
> old-object search
> - vtable-update-object goto table before updating
> - vtable-remove-object updates numeric column type as needed
> after removing line
> - vtable-remove-object signals an error if the specified object
> is not found
> - vtable-remove-object leaves point within table bounds if table
> is current (and there are rows)
> - Hide event-handler interactive functions from M-x display (not
> really "commands")
> - Renamed private vtable-header-line-sort to
> vtable--header-line-sort
> - Ensured docstrings for all public functions with added
> clarifications where needed
> 
> Enhancements
> 
> - Custom column comparator for sorting types other than numerics
> and lexical strings
> https://lists.gnu.org/archive/html/emacs-devel/2024-10/msg00043.html
> - Column-width inference can optionally include column-name
> width vs. just data
> - Table name slot, defaults to "*vtable*", which is displayed in
> messages to differentiate vtables
> - Duplicate object ignore, warning or error, defaults to ignore
> duplicates
> - Explicitly identify columns as numeric or non-numeric to avoid
> the cost of inferring numericalness
> - Option to sort table after insert or update object
> - Option to select the newly inserted line if the table is
> current
> - Sort indicator characters specifiable
> - Sort indicator faces for ascending and descending
> - Unsort the table, toggling between unsorted and the sort-by at
> initialization
> - Unsort binding in vtable-map
> - Add tab to vtable-header-line-map to move point into the table
> - Functions to compute programmatic row/column colors
> - Header face independent of body face
> - Header column alignment independent of from body alignment,
> defaults to body alignment
> - Center alignment options
> - Truncation ellipsis specifiable
> - Option to make in-buffer header cursor-intangible
> - Option to make in-buffer "decor" cursor-intangible (spacers,
> dividers, indicator padding)
> - Object/line marking/unmarking functions, marked line face
> - Next/previous line functions that remain in table body bounds
> - Multi-table commands 'vtable-goto-next-table',
> 'vtable-goto-previous-table'
> - Optionally apply row text properties; e.g., for mouse-face,
> cursor-face
> - Optional "navigation" keymap adding several useful bindings
> including "q" to close the table (a la special-mode)
> - Display a message when interactive column resizing min-width
> and max-width limits reached
> - Optionally pulse an updated, inserted, removed line,
> defaulting to 'pulse-momentary-highlight-one-line'
> - Go to table beginning/end of body convenience functions
> - Table and object line-number convenience functions
> - Table close-action which defaults to 'quit-window'
> - Extra-data slots on both the table and columns for
> programmatic reference

It is nigh impossible to review such a large number of changes
together: how am I supposed to find out which design and
implementation decisions did you take, when faced with such a large
number of separate changes?  Without that, the review is not useful,
even impossible.

So please subdivide the patch into smaller, preferably independent
parts, to make the patch review possible, let alone practical.

TIA

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

From: Stéphane Marks <shipmints <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sbaugh <at> janestreet.com, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org,
 adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Sun, 22 Jun 2025 06:38:56 -0400

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

On Sun, Jun 22, 2025 at 6:35 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Stéphane Marks <shipmints <at> gmail.com>
> > Date: Sun, 22 Jun 2025 06:24:39 -0400
> > Cc: 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, sbaugh <at> janestreet.com,
> >       larsi <at> gnus.org, arstoffel <at> gmail.com, krisbalintona <at> gmail.com
> >
> >  > Let's start with the vtable users I looped in actually using it.
> >
> >  I don't see how this will help in reviewing the patch. It will allow
> >  those folks to try the patch, but that's a separate job.  Patch review
> >  has to happen regardless, because it might spot issues that no
> >  reasonable amount of testing can.  This is why patch review is a
> >  necessary step in our development process.
> >
> >  > I can attach a vtable.el without the patch file and
> >  > make it easier.
> >
> >  Thanks, but it won't make it easier, no.
> >
> > Of course.  I did say that I would submit a "big-bang" patch a few weeks
> ago.  The vtable users who are
> > familiar with the code (having implemented their workarounds) should
> have an easier time with the review.
>
> That will help, but I would like to review it as well.  It's part of
> my job.
>
> Your patch lists many changes:
>
> > Bug fixes
> >
> > - vtable-update-object works with no visible or changed window
> > width cache key (bug=69837)
> > - Improve vtable--limit-string performance for long
> > strings (bug=77684)
> > - Keymap changes:
> >   - Keymap spans the whole header column and header
> >   dividers (bug=74701)
> >   - Missing keymap and 'vtable-column property on body/header
> >   dividers and newlines
> >   - mouse-1 and close table bindings in vtable-header-line-map
> >   keymap
> >   - table-drag-resize-column-map keymap for use in body on
> >   dividers (bug=74701)
> >   - vtable-header-drag-resize-column-map consolidates
> >   vtable-header-line-map and vtable-drag-resize-column-map
> > - Infer column widths based on formatted elements, not raw
> > elements
> > - Compute columns works when there are no objects at
> > initialization
> > - text-scale-mode support (header and body elements are pixel
> > aligned)
> > - Header line handles display-line-numbers-mode
> > - Redraw row colors on lines below an inserted or removed line
> > - Cache is coherent with its objects (rather than by chance),
> > invalidated where needed
> > - All cache references now cache warming calls vs. mere cache
> > get
> > - Table mutations work when the table buffer is not the current
> > buffer and if the table isn't current
> > - Resize all tables in a buffer when their window
> > resizes (debounced to accommodate frame size dragging)
> > - Spacer goes after the column name on the header if right
> > aligned
> > - Column name defaults to right-aligned when its data are
> > right-aligned
> > - Improve column placeholder message when no objects and no
> > columns are specified
> > - Do not display divider after a row's final column
> > - Option to display divider on the header (was always on)
> > - Treat divider as a part of its preceding column when dragging
> > or sorting
> > - Append table faces to values and column heading names rather
> > than override
> > - Truncated string ellipsis text properties match those of the
> > formatted column entry
> > - Clicking on a header keeps point within table bounds
> > - Use vtable-object-equal and do not assume eq (or via assq,
> > memq)
> > - Header line adjust final column name and sort indicator in a
> > narrow window works as intended
> > - Set window point where needed
> > - Next/previous line remains in the current column
> > - Signal an error if a table is inserted in more than one buffer
> > or more than once in a buffer
> > - vtable--cache-key handle if called when the selected window
> > does not contain a table buffer
> > - Restore "focus" after vtable-remove-object,
> > vtable-update-object, vtable-redisplay-range, vtable-revert if
> > table is current
> > - vtable-revert works when reverted table is not the current
> > table
> > - Mutate the correct associated table in
> > vtable-header-line-sort, vtable--drag-resize-column
> > - vtable--recompute-cache considers the table's sort order
> > - vtable--alter-column-width respects min-width and max-width
> > - vtable-insert-object if integer location specified, does not
> > assume the object list and line cache are in the same order
> > - vtable-insert-object signal an error if integer insert
> > location specified when the table is sorted
> > - vtable-update-object does not assume objects are in the same
> > order as the cache, which might be sorted
> > - vtable-update-object no longer signal "setcar nil" on failed
> > old-object search
> > - vtable-update-object goto table before updating
> > - vtable-remove-object updates numeric column type as needed
> > after removing line
> > - vtable-remove-object signals an error if the specified object
> > is not found
> > - vtable-remove-object leaves point within table bounds if table
> > is current (and there are rows)
> > - Hide event-handler interactive functions from M-x display (not
> > really "commands")
> > - Renamed private vtable-header-line-sort to
> > vtable--header-line-sort
> > - Ensured docstrings for all public functions with added
> > clarifications where needed
> >
> > Enhancements
> >
> > - Custom column comparator for sorting types other than numerics
> > and lexical strings
> > https://lists.gnu.org/archive/html/emacs-devel/2024-10/msg00043.html
> > - Column-width inference can optionally include column-name
> > width vs. just data
> > - Table name slot, defaults to "*vtable*", which is displayed in
> > messages to differentiate vtables
> > - Duplicate object ignore, warning or error, defaults to ignore
> > duplicates
> > - Explicitly identify columns as numeric or non-numeric to avoid
> > the cost of inferring numericalness
> > - Option to sort table after insert or update object
> > - Option to select the newly inserted line if the table is
> > current
> > - Sort indicator characters specifiable
> > - Sort indicator faces for ascending and descending
> > - Unsort the table, toggling between unsorted and the sort-by at
> > initialization
> > - Unsort binding in vtable-map
> > - Add tab to vtable-header-line-map to move point into the table
> > - Functions to compute programmatic row/column colors
> > - Header face independent of body face
> > - Header column alignment independent of from body alignment,
> > defaults to body alignment
> > - Center alignment options
> > - Truncation ellipsis specifiable
> > - Option to make in-buffer header cursor-intangible
> > - Option to make in-buffer "decor" cursor-intangible (spacers,
> > dividers, indicator padding)
> > - Object/line marking/unmarking functions, marked line face
> > - Next/previous line functions that remain in table body bounds
> > - Multi-table commands 'vtable-goto-next-table',
> > 'vtable-goto-previous-table'
> > - Optionally apply row text properties; e.g., for mouse-face,
> > cursor-face
> > - Optional "navigation" keymap adding several useful bindings
> > including "q" to close the table (a la special-mode)
> > - Display a message when interactive column resizing min-width
> > and max-width limits reached
> > - Optionally pulse an updated, inserted, removed line,
> > defaulting to 'pulse-momentary-highlight-one-line'
> > - Go to table beginning/end of body convenience functions
> > - Table and object line-number convenience functions
> > - Table close-action which defaults to 'quit-window'
> > - Extra-data slots on both the table and columns for
> > programmatic reference
>
> It is nigh impossible to review such a large number of changes
> together: how am I supposed to find out which design and
> implementation decisions did you take, when faced with such a large
> number of separate changes?  Without that, the review is not useful,
> even impossible.
>
> So please subdivide the patch into smaller, preferably independent
> parts, to make the patch review possible, let alone practical.
>

Let's see what the vtable user feedback is first and then I'll think about
it.  It will be a trade-off between your pain and mine.

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: sbaugh <at> janestreet.com, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org,
 adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Sun, 22 Jun 2025 13:59:45 +0300

> From: Stéphane Marks <shipmints <at> gmail.com>
> Date: Sun, 22 Jun 2025 06:38:56 -0400
> Cc: 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, sbaugh <at> janestreet.com, 
> 	larsi <at> gnus.org, arstoffel <at> gmail.com, krisbalintona <at> gmail.com
> 
>  So please subdivide the patch into smaller, preferably independent
>  parts, to make the patch review possible, let alone practical.
> 
> Let's see what the vtable user feedback is first and then I'll think about it.  It will be a trade-off between your
> pain and mine.

Patch review isn't supposed to be pain.  Its purpose is to ensure high
enough quality of our code and its maintainability for years to come.
As such, we should all have the common interest of making the patch
review to be as efficient and seamless as possible.

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

From: Stéphane Marks <shipmints <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sbaugh <at> janestreet.com, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org,
 adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Mon, 23 Jun 2025 10:49:07 -0400

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

On Sun, Jun 22, 2025 at 4:59 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

>
> For now, just a few comments from superficial first reading, mainly
> about the documentation.
>

I'm going through the documentation based on your feedback while I have a
few minutes today.

> +A vtable can be inserted into a single buffer only.  An error is
> > +signaled if a vtable is attempted to be inserted more than once into a
> > +single buffer, or into multiple buffers.
>
> The passive voice here is used way too much.  Please rephrase not to
> use it.
>

Done. I will review the rest of the language.

>  @table @code
> > +@item :name
> > +The default table name is `*vtable*`.  Specify this if you have more
>                              ^^^^^^^^^^
> This should be @samp{*vtable*}.
>

Done.

> -In the latter case, if @code{:columns} is non-@code{nil} and there's
> > -more elements in the sequence than there is in @code{:columns}, only
> > -the @code{:columns} first elements are displayed.
> > +In the latter case, if @code{:columns} is non-@code{nil} and there are
> > +more elements in the sequence than there are in @code{:columns}, the
> > +elements beyond those in @code{:columns} are not displayed.
>
> Since the elements are not in :columns, the modified text is
> confusing, IMO.  I suggest
>
>   ... the sequence elements beyond the number of elements in
>   @code{:columns} will not be shown.
>

Done.

> +@item :object-equal
> > +This function tests for the equality of two table objects.  It defaults
> > +to @code{eq}.
> > +
> > +@defun object-equal object1 object2
> > +Return non-nil if @var{object1} and @var{object2} are equal, and non-nil
> > +otherwise.
> > +@end defun
>
> I don't understand the purpose of this @defun here.  What did you
> intend to say by placing it here?
>

I was following Lars's original format for other "callback" functions.  If
I change this one, I should change them all for consistency.  What is your
preference?

> +@item :duplicate-objects
> > +A vtable assumes all objects inserted into the table are unique among
> > +themselves.  If the symbol @code{'allow}, the default, allow duplicate
> > +objects (the first object found during table operations has primacy), if
>
> First, please say "If the value is the symbol...", not just "If the
> symbol".  And second, the way we typeset symbols in a Texinfo manual
> is by using @code{allow}, without the apostrophe.
>

Done.

> +@code{'ignore}, silently ignore duplicates leaving the existing object
> > +intact, if @code{'ignore-warn} do the same thing as @code{'ignore} and
> > +produce a warning message, or if @code{'error}, signal an error if a
> > +duplicate is detected.
>
> Likewise here about the symbol names.  In addition, please use
> semi-colons to separate the description of the different values:
>
>   ...if it's |@code{ignore}, silently ignore duplicates leaving the
>   existing object intact; if it's @code{ignore-warn}, do the same...
>

Done.

> +This is a list where each element is either a string (the column name),
> > +a plist of keyword/values (to make a @code{vtable-column} object), or a
> > +@code{vtable-column} object (created by calling the function
> > +@code{make-vtable-column}).
>
> When you mention a function or a variable, like make-vtable-column
> here, it is usually a good idea to have a cross-reference to where
> they are described (unless they are described in the same node).
>

I will.

> +that need special treatment, specify a larger guess increment.  If nil,
>                                                                       ^^^
> "nil" should have the @code markup.
>

I corrected all the nil references.  It was copy/pasta from docstrings.

> +@item comparator
> > +This function will be called to compare column values.  Use this when
> > +your objects contain data that needs non-numeric or string collation,
> > +for example, dates.  Another case would be where you want the equivalent
> > +of @code{sort-fold-case}.
> > +
> > +@defun comparator value1 value2
> > +This function is called with two values to compare.  The return value
> > +should follow the semantics of @code{<} or, @code{string-lessp}.
> >  @end defun
>
> Same question here about this @defun, as above about object-equal.
>

Ditto.  Tell me which style you prefer.

> +@item :sort-indicator
> > +This is a list of two cons cells that specify the sort ascending and
> > +descending characters that are shown on the table header to indicate a
> > +sorted column.  The first character is the fancier ``graphical''
> > +character, and the second a text-only character.  If the first character
> > +cannot be displayed on the selected frame, the table will show the
> > +text-only character.  The default is
> > +@code{vtable-sort-indicator-default}.  For example:
> > +
> > +@lisp
> > +'((?▼ ?v) (?▲ ?^)) ; This is the default.
> > +'((?⬇ ?v) (?⬆ ?^)) ; Alternative arrows.
> > +@end lisp
>
> Using non-ASCII characters in a manual runs the risk of producing
> incorrect output in the printed manual, due to lack of support in the
> fonts used by TeX.  It is better to avoid that.  Here, you use them in
> the example, which seems unnecessary, so I suggest to rewrite the
> example using only ASCII characters (you can mention non-ASCII as a
> possibility without actually showing such an example).
>

Done.

> +@table @kbd
> > +@findex vtable-next-line
> > +@item n
> > +@item <down>
>
> Our conventions are to UPCASE arrow key names, so please use DOWN, UP,
> etc.
>
> Also, since "DOWN" is a label of the key, you should use @key{DOWN},
> not <DOWN>.
>

Done.

> +@findex vtable-goto-next-table
> > +@item <forward-paragraph>
>
> What's "<forward-paragraph>"?  Are there keyboards with a key that has
> such a label?
>

This is remapped in the code.  Is there a way you prefer to refer to
remapped keys?

(defvar-keymap vtable-navigation-map
  "<remap> <forward-paragraph>"  #'vtable-goto-next-table
  "<remap> <backward-paragraph>" #'vtable-goto-previous-table

> --- a/lisp/emacs-lisp/vtable.el
> > +++ b/lisp/emacs-lisp/vtable.el
> > @@ -30,77 +30,220 @@
> >
> >  (defface vtable
> >    '((t :inherit variable-pitch))
> > -  "Face used (by default) for vtables."
> > +  "Face used (by default) for vtable bodies."
> >    :version "29.1"
> >    :group 'faces)
> >
> > +(defface vtable-header
> > +  '((t :inherit (header-line vtable)))
> > +  "Face used (by default) for vtable headers."
> > +  :version "31.1"
> > +  :group 'faces)
> > +
> > +(defface vtable-marked
> > +  '((t :inherit region))
> > +  "Face used (by default) for marked vtable objects."
> > +  :version "31.1"
> > +  :group 'faces)
> > +
> > +(defface vtable-sort-indicator-ascend
> > +  '((t :inherit vtable-header))
> > +  "Face used (by default) for vtable ascend sort indicator."
> > +  :version "31.1"
> > +  :group 'faces)
> > +
> > +(defface vtable-sort-indicator-descend
> > +  '((t :inherit vtable-header))
> > +  "Face used (by default) for vtable descend sort indicator."
> > +  :version "31.1"
> > +  :group 'faces)
>
> Shouldn't these faces be in the vtable group instead?
>

Done.  I'd copied the original vtable.

Once we get feedback from users, I will split the patch into several
digestible smaller patches, but probably not too parsimoniously.

-Stéphane

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: sbaugh <at> janestreet.com, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org,
 adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Mon, 23 Jun 2025 19:05:31 +0300

> From: Stéphane Marks <shipmints <at> gmail.com>
> Date: Mon, 23 Jun 2025 10:49:07 -0400
> Cc: 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, sbaugh <at> janestreet.com, 
> 	larsi <at> gnus.org, arstoffel <at> gmail.com, krisbalintona <at> gmail.com
> 
>  > +@item :object-equal
>  > +This function tests for the equality of two table objects.  It defaults
>  > +to @code{eq}.
>  > +
>  > +@defun object-equal object1 object2
>  > +Return non-nil if @var{object1} and @var{object2} are equal, and non-nil
>  > +otherwise.
>  > +@end defun
> 
>  I don't understand the purpose of this @defun here.  What did you
>  intend to say by placing it here?
> 
> I was following Lars's original format for other "callback" functions.  If I change this one, I should change them
> all for consistency.  What is your preference?

Let's leave these alone for now.

>  > +@findex vtable-goto-next-table
>  > +@item <forward-paragraph>
> 
>  What's "<forward-paragraph>"?  Are there keyboards with a key that has
>  such a label?
> 
> This is remapped in the code.  Is there a way you prefer to refer to remapped keys?

We usually simply show the default bindings, M-} etc.

> Once we get feedback from users, I will split the patch into several digestible smaller patches, but probably
> not too parsimoniously.

Thanks!

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net,
 larsi <at> gnus.org, arstoffel <at> gmail.com, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Mon, 23 Jun 2025 13:15:06 -0400

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

> On Sun, Jun 22, 2025 at 6:35 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
>  > From: Stéphane Marks <shipmints <at> gmail.com>
>  > Date: Sun, 22 Jun 2025 06:24:39 -0400
>  > Cc: 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, sbaugh <at> janestreet.com, 
>  >       larsi <at> gnus.org, arstoffel <at> gmail.com, krisbalintona <at> gmail.com
>  > 
>  >  > Let's start with the vtable users I looped in actually using it.
>  > 
>  >  I don't see how this will help in reviewing the patch. It will allow
>  >  those folks to try the patch, but that's a separate job.  Patch review
>  >  has to happen regardless, because it might spot issues that no
>  >  reasonable amount of testing can.  This is why patch review is a
>  >  necessary step in our development process.
>  > 
>  >  > I can attach a vtable.el without the patch file and
>  >  > make it easier.
>  > 
>  >  Thanks, but it won't make it easier, no.
>  > 
>  > Of course.  I did say that I would submit a "big-bang" patch a few weeks ago.  The vtable users who are
>  > familiar with the code (having implemented their workarounds) should have an easier time with the review. 
>
>  That will help, but I would like to review it as well.  It's part of
>  my job.
>
>  Your patch lists many changes:
>
>  > Bug fixes
>  > 
>  > - vtable-update-object works with no visible or changed window
>  > width cache key (bug=69837)
>  > - Improve vtable--limit-string performance for long
>  > strings (bug=77684)
>  > - Keymap changes:
>  >   - Keymap spans the whole header column and header
>  >   dividers (bug=74701)
>  >   - Missing keymap and 'vtable-column property on body/header
>  >   dividers and newlines
>  >   - mouse-1 and close table bindings in vtable-header-line-map
>  >   keymap
>  >   - table-drag-resize-column-map keymap for use in body on
>  >   dividers (bug=74701)
>  >   - vtable-header-drag-resize-column-map consolidates
>  >   vtable-header-line-map and vtable-drag-resize-column-map
>  > - Infer column widths based on formatted elements, not raw
>  > elements
>  > - Compute columns works when there are no objects at
>  > initialization
>  > - text-scale-mode support (header and body elements are pixel
>  > aligned)
>  > - Header line handles display-line-numbers-mode
>  > - Redraw row colors on lines below an inserted or removed line
>  > - Cache is coherent with its objects (rather than by chance),
>  > invalidated where needed
>  > - All cache references now cache warming calls vs. mere cache
>  > get
>  > - Table mutations work when the table buffer is not the current
>  > buffer and if the table isn't current
>  > - Resize all tables in a buffer when their window
>  > resizes (debounced to accommodate frame size dragging)
>  > - Spacer goes after the column name on the header if right
>  > aligned
>  > - Column name defaults to right-aligned when its data are
>  > right-aligned
>  > - Improve column placeholder message when no objects and no
>  > columns are specified
>  > - Do not display divider after a row's final column
>  > - Option to display divider on the header (was always on)
>  > - Treat divider as a part of its preceding column when dragging
>  > or sorting
>  > - Append table faces to values and column heading names rather
>  > than override
>  > - Truncated string ellipsis text properties match those of the
>  > formatted column entry
>  > - Clicking on a header keeps point within table bounds
>  > - Use vtable-object-equal and do not assume eq (or via assq,
>  > memq)
>  > - Header line adjust final column name and sort indicator in a
>  > narrow window works as intended
>  > - Set window point where needed
>  > - Next/previous line remains in the current column
>  > - Signal an error if a table is inserted in more than one buffer
>  > or more than once in a buffer
>  > - vtable--cache-key handle if called when the selected window
>  > does not contain a table buffer
>  > - Restore "focus" after vtable-remove-object,
>  > vtable-update-object, vtable-redisplay-range, vtable-revert if
>  > table is current
>  > - vtable-revert works when reverted table is not the current
>  > table
>  > - Mutate the correct associated table in
>  > vtable-header-line-sort, vtable--drag-resize-column
>  > - vtable--recompute-cache considers the table's sort order
>  > - vtable--alter-column-width respects min-width and max-width
>  > - vtable-insert-object if integer location specified, does not
>  > assume the object list and line cache are in the same order
>  > - vtable-insert-object signal an error if integer insert
>  > location specified when the table is sorted
>  > - vtable-update-object does not assume objects are in the same
>  > order as the cache, which might be sorted
>  > - vtable-update-object no longer signal "setcar nil" on failed
>  > old-object search
>  > - vtable-update-object goto table before updating
>  > - vtable-remove-object updates numeric column type as needed
>  > after removing line
>  > - vtable-remove-object signals an error if the specified object
>  > is not found
>  > - vtable-remove-object leaves point within table bounds if table
>  > is current (and there are rows)
>  > - Hide event-handler interactive functions from M-x display (not
>  > really "commands")
>  > - Renamed private vtable-header-line-sort to
>  > vtable--header-line-sort
>  > - Ensured docstrings for all public functions with added
>  > clarifications where needed
>  > 
>  > Enhancements
>  > 
>  > - Custom column comparator for sorting types other than numerics
>  > and lexical strings
>  > https://lists.gnu.org/archive/html/emacs-devel/2024-10/msg00043.html
>  > - Column-width inference can optionally include column-name
>  > width vs. just data
>  > - Table name slot, defaults to "*vtable*", which is displayed in
>  > messages to differentiate vtables
>  > - Duplicate object ignore, warning or error, defaults to ignore
>  > duplicates
>  > - Explicitly identify columns as numeric or non-numeric to avoid
>  > the cost of inferring numericalness
>  > - Option to sort table after insert or update object
>  > - Option to select the newly inserted line if the table is
>  > current
>  > - Sort indicator characters specifiable
>  > - Sort indicator faces for ascending and descending
>  > - Unsort the table, toggling between unsorted and the sort-by at
>  > initialization
>  > - Unsort binding in vtable-map
>  > - Add tab to vtable-header-line-map to move point into the table
>  > - Functions to compute programmatic row/column colors
>  > - Header face independent of body face
>  > - Header column alignment independent of from body alignment,
>  > defaults to body alignment
>  > - Center alignment options
>  > - Truncation ellipsis specifiable
>  > - Option to make in-buffer header cursor-intangible
>  > - Option to make in-buffer "decor" cursor-intangible (spacers,
>  > dividers, indicator padding)
>  > - Object/line marking/unmarking functions, marked line face
>  > - Next/previous line functions that remain in table body bounds
>  > - Multi-table commands 'vtable-goto-next-table',
>  > 'vtable-goto-previous-table'
>  > - Optionally apply row text properties; e.g., for mouse-face,
>  > cursor-face
>  > - Optional "navigation" keymap adding several useful bindings
>  > including "q" to close the table (a la special-mode)
>  > - Display a message when interactive column resizing min-width
>  > and max-width limits reached
>  > - Optionally pulse an updated, inserted, removed line,
>  > defaulting to 'pulse-momentary-highlight-one-line'
>  > - Go to table beginning/end of body convenience functions
>  > - Table and object line-number convenience functions
>  > - Table close-action which defaults to 'quit-window'
>  > - Extra-data slots on both the table and columns for
>  > programmatic reference
>
>  It is nigh impossible to review such a large number of changes
>  together: how am I supposed to find out which design and
>  implementation decisions did you take, when faced with such a large
>  number of separate changes?  Without that, the review is not useful,
>  even impossible.
>
>  So please subdivide the patch into smaller, preferably independent
>  parts, to make the patch review possible, let alone practical.
>
> Let's see what the vtable user feedback is first and then I'll think
> about it.  It will be a trade-off between your pain and mine.

As a vtable user: Please either split this patch up into smaller
patches, or add many automated tests of vtable (both for new and old
functionality).  Preferably both.

Without doing at least one of those things, I don't believe this patch
should be merged, because it is too likely to break things.

Yes, I know you have tested it manually.  That is not sufficient.  Good
software engineering practice demands more than that.

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

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net,
 larsi <at> gnus.org, arstoffel <at> gmail.com, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Mon, 23 Jun 2025 13:59:27 -0400

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

On Mon, Jun 23, 2025 at 1:15 PM Spencer Baugh <sbaugh <at> janestreet.com> wrote:

> Stéphane Marks <shipmints <at> gmail.com> writes:
>
> > On Sun, Jun 22, 2025 at 6:35 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> >  > From: Stéphane Marks <shipmints <at> gmail.com>
> >  > Date: Sun, 22 Jun 2025 06:24:39 -0400
> >  > Cc: 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, sbaugh <at> janestreet.com,
>
> >  >       larsi <at> gnus.org, arstoffel <at> gmail.com, krisbalintona <at> gmail.com
> >  >
> >  >  > Let's start with the vtable users I looped in actually using it.
> >  >
> >  >  I don't see how this will help in reviewing the patch. It will allow
> >  >  those folks to try the patch, but that's a separate job.  Patch
> review
> >  >  has to happen regardless, because it might spot issues that no
> >  >  reasonable amount of testing can.  This is why patch review is a
> >  >  necessary step in our development process.
> >  >
> >  >  > I can attach a vtable.el without the patch file and
> >  >  > make it easier.
> >  >
> >  >  Thanks, but it won't make it easier, no.
> >  >
> >  > Of course.  I did say that I would submit a "big-bang" patch a few
> weeks ago.  The vtable users who are
> >  > familiar with the code (having implemented their workarounds) should
> have an easier time with the review.
> >
> >  That will help, but I would like to review it as well.  It's part of
> >  my job.
> >
> >  Your patch lists many changes:
> >
> >  > Bug fixes
> >  >
> >  > - vtable-update-object works with no visible or changed window
> >  > width cache key (bug=69837)
> >  > - Improve vtable--limit-string performance for long
> >  > strings (bug=77684)
> >  > - Keymap changes:
> >  >   - Keymap spans the whole header column and header
> >  >   dividers (bug=74701)
> >  >   - Missing keymap and 'vtable-column property on body/header
> >  >   dividers and newlines
> >  >   - mouse-1 and close table bindings in vtable-header-line-map
> >  >   keymap
> >  >   - table-drag-resize-column-map keymap for use in body on
> >  >   dividers (bug=74701)
> >  >   - vtable-header-drag-resize-column-map consolidates
> >  >   vtable-header-line-map and vtable-drag-resize-column-map
> >  > - Infer column widths based on formatted elements, not raw
> >  > elements
> >  > - Compute columns works when there are no objects at
> >  > initialization
> >  > - text-scale-mode support (header and body elements are pixel
> >  > aligned)
> >  > - Header line handles display-line-numbers-mode
> >  > - Redraw row colors on lines below an inserted or removed line
> >  > - Cache is coherent with its objects (rather than by chance),
> >  > invalidated where needed
> >  > - All cache references now cache warming calls vs. mere cache
> >  > get
> >  > - Table mutations work when the table buffer is not the current
> >  > buffer and if the table isn't current
> >  > - Resize all tables in a buffer when their window
> >  > resizes (debounced to accommodate frame size dragging)
> >  > - Spacer goes after the column name on the header if right
> >  > aligned
> >  > - Column name defaults to right-aligned when its data are
> >  > right-aligned
> >  > - Improve column placeholder message when no objects and no
> >  > columns are specified
> >  > - Do not display divider after a row's final column
> >  > - Option to display divider on the header (was always on)
> >  > - Treat divider as a part of its preceding column when dragging
> >  > or sorting
> >  > - Append table faces to values and column heading names rather
> >  > than override
> >  > - Truncated string ellipsis text properties match those of the
> >  > formatted column entry
> >  > - Clicking on a header keeps point within table bounds
> >  > - Use vtable-object-equal and do not assume eq (or via assq,
> >  > memq)
> >  > - Header line adjust final column name and sort indicator in a
> >  > narrow window works as intended
> >  > - Set window point where needed
> >  > - Next/previous line remains in the current column
> >  > - Signal an error if a table is inserted in more than one buffer
> >  > or more than once in a buffer
> >  > - vtable--cache-key handle if called when the selected window
> >  > does not contain a table buffer
> >  > - Restore "focus" after vtable-remove-object,
> >  > vtable-update-object, vtable-redisplay-range, vtable-revert if
> >  > table is current
> >  > - vtable-revert works when reverted table is not the current
> >  > table
> >  > - Mutate the correct associated table in
> >  > vtable-header-line-sort, vtable--drag-resize-column
> >  > - vtable--recompute-cache considers the table's sort order
> >  > - vtable--alter-column-width respects min-width and max-width
> >  > - vtable-insert-object if integer location specified, does not
> >  > assume the object list and line cache are in the same order
> >  > - vtable-insert-object signal an error if integer insert
> >  > location specified when the table is sorted
> >  > - vtable-update-object does not assume objects are in the same
> >  > order as the cache, which might be sorted
> >  > - vtable-update-object no longer signal "setcar nil" on failed
> >  > old-object search
> >  > - vtable-update-object goto table before updating
> >  > - vtable-remove-object updates numeric column type as needed
> >  > after removing line
> >  > - vtable-remove-object signals an error if the specified object
> >  > is not found
> >  > - vtable-remove-object leaves point within table bounds if table
> >  > is current (and there are rows)
> >  > - Hide event-handler interactive functions from M-x display (not
> >  > really "commands")
> >  > - Renamed private vtable-header-line-sort to
> >  > vtable--header-line-sort
> >  > - Ensured docstrings for all public functions with added
> >  > clarifications where needed
> >  >
> >  > Enhancements
> >  >
> >  > - Custom column comparator for sorting types other than numerics
> >  > and lexical strings
> >  > https://lists.gnu.org/archive/html/emacs-devel/2024-10/msg00043.html
> >  > - Column-width inference can optionally include column-name
> >  > width vs. just data
> >  > - Table name slot, defaults to "*vtable*", which is displayed in
> >  > messages to differentiate vtables
> >  > - Duplicate object ignore, warning or error, defaults to ignore
> >  > duplicates
> >  > - Explicitly identify columns as numeric or non-numeric to avoid
> >  > the cost of inferring numericalness
> >  > - Option to sort table after insert or update object
> >  > - Option to select the newly inserted line if the table is
> >  > current
> >  > - Sort indicator characters specifiable
> >  > - Sort indicator faces for ascending and descending
> >  > - Unsort the table, toggling between unsorted and the sort-by at
> >  > initialization
> >  > - Unsort binding in vtable-map
> >  > - Add tab to vtable-header-line-map to move point into the table
> >  > - Functions to compute programmatic row/column colors
> >  > - Header face independent of body face
> >  > - Header column alignment independent of from body alignment,
> >  > defaults to body alignment
> >  > - Center alignment options
> >  > - Truncation ellipsis specifiable
> >  > - Option to make in-buffer header cursor-intangible
> >  > - Option to make in-buffer "decor" cursor-intangible (spacers,
> >  > dividers, indicator padding)
> >  > - Object/line marking/unmarking functions, marked line face
> >  > - Next/previous line functions that remain in table body bounds
> >  > - Multi-table commands 'vtable-goto-next-table',
> >  > 'vtable-goto-previous-table'
> >  > - Optionally apply row text properties; e.g., for mouse-face,
> >  > cursor-face
> >  > - Optional "navigation" keymap adding several useful bindings
> >  > including "q" to close the table (a la special-mode)
> >  > - Display a message when interactive column resizing min-width
> >  > and max-width limits reached
> >  > - Optionally pulse an updated, inserted, removed line,
> >  > defaulting to 'pulse-momentary-highlight-one-line'
> >  > - Go to table beginning/end of body convenience functions
> >  > - Table and object line-number convenience functions
> >  > - Table close-action which defaults to 'quit-window'
> >  > - Extra-data slots on both the table and columns for
> >  > programmatic reference
> >
> >  It is nigh impossible to review such a large number of changes
> >  together: how am I supposed to find out which design and
> >  implementation decisions did you take, when faced with such a large
> >  number of separate changes?  Without that, the review is not useful,
> >  even impossible.
> >
> >  So please subdivide the patch into smaller, preferably independent
> >  parts, to make the patch review possible, let alone practical.
> >
> > Let's see what the vtable user feedback is first and then I'll think
> > about it.  It will be a trade-off between your pain and mine.
>
> As a vtable user: Please either split this patch up into smaller
> patches, or add many automated tests of vtable (both for new and old
> functionality).  Preferably both.
>
> Without doing at least one of those things, I don't believe this patch
> should be merged, because it is too likely to break things.
>

As I volunteered to maintain vtable, aside from the sheer annoyance that a
breakage might bring a user during Emacs 31 development, the burden to
address them will be mine.  I have little incentive to create bugs.

Yes, I know you have tested it manually.  That is not sufficient.  Good
> software engineering practice demands more than that.
>

Surely.  I'm surprised that vtable was adopted without any tests at all.  I
expect it will be a slow evolution to add them.

In the meantime, be a mensch, and try it out to see if it addresses
your immediate issue and provide constructive feedback.  The set of bugs I
fixed and the new features added were informed by other menschen.  I've
always considered your ideas and input well.

Here are the latest two files for ease of testing.  vtable.el, in its
current form, is compatible with Emacs 31 unless and until
`string-pixel-width` is updated in compat.  I doubt Eli will want this in
Emacs 30 but it could be published on ELPA if there is demand?

I greatly appreciate it.  Once feedback is received, I'll start crafting
smaller (but not tiny) patches.

Best,

-Stéphane

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

[vtable.el (application/octet-stream, attachment)]

[vtable.texi (application/octet-stream, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: sbaugh <at> janestreet.com, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org,
 adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Tue, 24 Jun 2025 05:26:31 +0300

> From: Stéphane Marks <shipmints <at> gmail.com>
> Date: Mon, 23 Jun 2025 13:59:27 -0400
> Cc: Eli Zaretskii <eliz <at> gnu.org>, krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org, 
> 	adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com
> 
> Here are the latest two files for ease of testing.  vtable.el, in its current form, is compatible with Emacs 31
> unless and until `string-pixel-width` is updated in compat.  I doubt Eli will want this in Emacs 30 but it could be
> published on ELPA if there is demand?

Indeed, these changes are for Emacs 31.

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

From: Augusto Stoffel <arstoffel <at> gmail.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: Spencer Baugh <sbaugh <at> janestreet.com>,
 Kristoffer Balintona <krisbalintona <at> gmail.com>, 78843 <at> debbugs.gnu.org,
 Adam Porter <adam <at> alphapapa.net>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Augusto Stoffel <arstoffel <at> gmail.com>
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Tue, 24 Jun 2025 10:00:19 +0200

On Sat, 21 Jun 2025 at 13:40, Stéphane Marks wrote:

> On Sat, Jun 21, 2025 at 11:41 AM Stéphane Marks <shipmints <at> gmail.com> wrote:
>
>  On Thu, Jun 19, 2025 at 4:25 PM Stéphane Marks <shipmints <at> gmail.com>
>  wrote:
>
>  Bug for a consolidated patch which is in the process of being prepared.
>
>  Large patch attached.  All the changes, aside from bug fixes, should be backward
>  compatible.  I've cc'd a few of the major vtable users whose code bases helped
>  inform many bug fixes and enhancements.  Some of you have helped test and
>  provide feedback along the way.  Please try this out (and review the updated
>  texinfo documentation if you want to).
>
>  Feedback here and/or to me directly is fine.
>
> After double checking comint-mime, it looks like I should add header-text-properties
> just as I did row-text-properties so that comint-mime doesn't need shenanigans.  On
> the list.  What else?

Part of the comint-mime shenanigan is because I wanted certain text
properties in the vtable area.  How to best achieve this could be a
discussion to be had.

The other reason for the shenanigan is that I insert a vtable in a
buffer with font lock and so I need font lock to not override the vtable
faces.  This is just a bug to be fixed.  Do you address that in your
patch?

Thanks for working on vtable ;-).

> -Stéphane

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

From: Stéphane Marks <shipmints <at> gmail.com>
To: Augusto Stoffel <arstoffel <at> gmail.com>
Cc: Adam Porter <adam <at> alphapapa.net>, Spencer Baugh <sbaugh <at> janestreet.com>,
 Lars Ingebrigtsen <larsi <at> gnus.org>,
 Kristoffer Balintona <krisbalintona <at> gmail.com>, 78843 <at> debbugs.gnu.org
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Tue, 24 Jun 2025 07:25:48 -0400

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

On Tue, Jun 24, 2025 at 4:00 AM Augusto Stoffel <arstoffel <at> gmail.com> wrote:

> On Sat, 21 Jun 2025 at 13:40, Stéphane Marks wrote:
>
> > On Sat, Jun 21, 2025 at 11:41 AM Stéphane Marks <shipmints <at> gmail.com>
> wrote:
> >
> >  On Thu, Jun 19, 2025 at 4:25 PM Stéphane Marks <shipmints <at> gmail.com>
> >  wrote:
> >
> >  Bug for a consolidated patch which is in the process of being prepared.
> >
> >  Large patch attached.  All the changes, aside from bug fixes, should be
> backward
> >  compatible.  I've cc'd a few of the major vtable users whose code bases
> helped
> >  inform many bug fixes and enhancements.  Some of you have helped test
> and
> >  provide feedback along the way.  Please try this out (and review the
> updated
> >  texinfo documentation if you want to).
> >
> >  Feedback here and/or to me directly is fine.
> >
> > After double checking comint-mime, it looks like I should add
> header-text-properties
> > just as I did row-text-properties so that comint-mime doesn't need
> shenanigans.  On
> > the list.  What else?
>
> Part of the comint-mime shenanigan is because I wanted certain text
> properties in the vtable area.  How to best achieve this could be a
> discussion to be had.
>
> The other reason for the shenanigan is that I insert a vtable in a
> buffer with font lock and so I need font lock to not override the vtable
> faces.  This is just a bug to be fixed.  Do you address that in your
> patch?
>

Nope.  I did your other use cases but not this one.  I'll take a look at
that today.

If I "mimecat -t text/html table.html" with old and new vtable, they look
the same, so at least it's backward compatible.

Thanks for working on vtable ;-).
>

It's been fun to improve.

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

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net,
 larsi <at> gnus.org, arstoffel <at> gmail.com, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Mon, 30 Jun 2025 12:27:04 -0400

Stéphane Marks <shipmints <at> gmail.com> writes:
> In the meantime, be a mensch, and try it out to see if it addresses your immediate issue and provide constructive feedback.  The
> set of bugs I fixed and the new features added were informed by other menschen.  I've always considered your ideas and input
> well.
>
> Here are the latest two files for ease of testing.  vtable.el, in its current form, is compatible with Emacs 31 unless and until
> `string-pixel-width` is updated in compat.  I doubt Eli will want this in Emacs 30 but it could be published on ELPA if there is
> demand?

A few issues using the vtable.el you attached:

- Clicking on header lines no longer changes sorting.

- Columns where I specify :displayer now seem to have extra empty
  padding added to the end of them.  I think this may be the result of
  vtable--insert-line passing clipped-value-width (which has space for
  the ellipsis removed) to the displayer function rather than passing
  column-width as it used to.

I do like that the header line is exactly properly aligned with columns
now, both in graphical and terminal Emacs.  I wish you would please make
smaller patches for your individual improvements so that we could have
them without these bugs...

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net,
 larsi <at> gnus.org, arstoffel <at> gmail.com, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Mon, 30 Jun 2025 12:45:55 -0400

Spencer Baugh <sbaugh <at> janestreet.com> writes:

> Stéphane Marks <shipmints <at> gmail.com> writes:
>> In the meantime, be a mensch, and try it out to see if it addresses your immediate issue and provide constructive feedback.  The
>> set of bugs I fixed and the new features added were informed by other menschen.  I've always considered your ideas and input
>> well.
>>
>> Here are the latest two files for ease of testing.  vtable.el, in its current form, is compatible with Emacs 31 unless and until
>> `string-pixel-width` is updated in compat.  I doubt Eli will want this in Emacs 30 but it could be published on ELPA if there is
>> demand?
>
> A few issues using the vtable.el you attached:
>
> - Clicking on header lines no longer changes sorting.
>
> - Columns where I specify :displayer now seem to have extra empty
>   padding added to the end of them.  I think this may be the result of
>   vtable--insert-line passing clipped-value-width (which has space for
>   the ellipsis removed) to the displayer function rather than passing
>   column-width as it used to.
>
> I do like that the header line is exactly properly aligned with columns
> now, both in graphical and terminal Emacs.  I wish you would please make
> smaller patches for your individual improvements so that we could have
> them without these bugs...

(Or at least add tests for these issues once you fix them)

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

From: Stéphane Marks <shipmints <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: krisbalintona <at> gmail.com, 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net,
 larsi <at> gnus.org, arstoffel <at> gmail.com, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Mon, 30 Jun 2025 14:41:09 -0400

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

On Mon, Jun 30, 2025 at 12:27 PM Spencer Baugh <sbaugh <at> janestreet.com>
wrote:

> Stéphane Marks <shipmints <at> gmail.com> writes:
> > In the meantime, be a mensch, and try it out to see if it addresses your
> immediate issue and provide constructive feedback.  The
> > set of bugs I fixed and the new features added were informed by other
> menschen.  I've always considered your ideas and input
> > well.
> >
> > Here are the latest two files for ease of testing.  vtable.el, in its
> current form, is compatible with Emacs 31 unless and until
> > `string-pixel-width` is updated in compat.  I doubt Eli will want this
> in Emacs 30 but it could be published on ELPA if there is
> > demand?
>
> A few issues using the vtable.el you attached:
>
> - Clicking on header lines no longer changes sorting.
>
> - Columns where I specify :displayer now seem to have extra empty
>   padding added to the end of them.  I think this may be the result of
>   vtable--insert-line passing clipped-value-width (which has space for
>   the ellipsis removed) to the displayer function rather than passing
>   column-width as it used to.
>
> I do like that the header line is exactly properly aligned with columns
> now, both in graphical and terminal Emacs.  I wish you would please make
> smaller patches for your individual improvements so that we could have
> them without these bugs...
>

Thank you very much for testing the large change.  If I piecemealed all the
changes it would take weeks/months to get past reviewing all of them.  And
I've restructured some of the code along the way for clarity and
correctness.

I've since repaired those.

I had put in a restriction that enforced that a vtable should be inserted
into a single buffer.  That broke comint-mime which renders a vtable from
HTML in a temp buffer and then inserts the buffer's string into the shell
buffer.  While that could be reworked, I sense that the single-buffer
restriction is too defensive and less Emacsy.  What I will do instead is
add documentation and a suggestion about invoking vtable functions when the
current buffer is not the vtable buffer.

As I find time, I will start working on some tests to add to the two tests
that are there.

I've attached a revised vtable.el that still has the single-buffer
restriction and with your feedback items addressed.

I deeply appreciate everyone's patience and flexibility with these fixes
and improvements.

-Stéphane

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

[vtable.el (application/octet-stream, attachment)]

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

From: Augusto Stoffel <arstoffel <at> gmail.com>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: Spencer Baugh <sbaugh <at> janestreet.com>, krisbalintona <at> gmail.com,
 78843 <at> debbugs.gnu.org, adam <at> alphapapa.net, larsi <at> gnus.org, arstoffel <at> gmail.com,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#78843: Various vtable bug fixes and enhancements
Date: Tue, 01 Jul 2025 12:46:11 +0200

On Mon, 30 Jun 2025 at 14:41, Stéphane Marks wrote:

> I had put in a restriction that enforced that a vtable should be
> inserted into a single buffer.  That broke comint-mime which renders a
> vtable from HTML in a temp buffer and then inserts the buffer's string
> into the shell buffer.  While that could be reworked, I sense that the
> single-buffer restriction is too defensive and less Emacsy.  What I
> will do instead is add documentation and a suggestion about invoking
> vtable functions when the current buffer is not the vtable buffer.

I don't remember why I did it that way (there must have been a reason)
but it does seem a bit eccentric and could be changed.  This is
specially true if there is something meaningful to be gained from the
restriction you thought of enforcing.

Previous Next

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