On Sun, Jun 22, 2025 at 6:35 AM Eli Zaretskii wrote: > > From: Stéphane Marks > > Date: Sun, 22 Jun 2025 06:24:39 -0400 > > Cc: 78843@debbugs.gnu.org, adam@alphapapa.net, sbaugh@janestreet.com, > > larsi@gnus.org, arstoffel@gmail.com, krisbalintona@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.