bug#70664: 29.3; vtable-insert-object cannot insert at top of table

bug#70664: 29.3; vtable-insert-object cannot insert at top of table

[Joost Kremers]

The current implementation of `vtable.el` provides the function
`vtable-insert-object` to add an object to the vtable. Its signature is this:

    vtable-insert-object table object &optional after-object

The doc string says that if AFTER-OBJECT is not provided, the new object is
added to the end of the table. This makes it impossible to add an object as the
first element of the table.

Note that although this e-mail is written from Emacs 29.3, the version of
`vtable.el` in master has the same problem.

[All the diagnostic info that `M-x report-emacs-bug` usually gathers should
appear here, but there was a problem with the buffer being read-only. I'll have
to look into that, but for this report, this info isn't relevant, I think.]

TIA

-- 
Joost Kremers
Life has its moments

bug#70664: 29.3; vtable-insert-object cannot insert at top of table

[Eli Zaretskii]

> From: Joost Kremers <joostkremers@fastmail.fm>
> Date: Tue, 30 Apr 2024 11:30:47 +0200
> 
> The current implementation of `vtable.el` provides the function
> `vtable-insert-object` to add an object to the vtable. Its signature is this:
> 
>     vtable-insert-object table object &optional after-object
> 
> The doc string says that if AFTER-OBJECT is not provided, the new object is
> added to the end of the table. This makes it impossible to add an object as the
> first element of the table.
> 
> Note that although this e-mail is written from Emacs 29.3, the version of
> `vtable.el` in master has the same problem.

Adam, WDYT?  You did something similar with vtable-update-object,
AFAIR.

bug#70664: 29.3; vtable-insert-object cannot insert at top of table

[Joost Kremers]

Hi Adam,

On Tue, Apr 30 2024, Adam Porter wrote:
> Hm, yes, it seems that it's not possible with the current implementation.
>
> My first idea, requiring maybe the smallest change to the code and no change to
> the signature, would be to accept a special value as the AFTER-OBJECT argument
> to indicate that it should be inserted as the first element, e.g.
> `:insert-first'.

My initial thought as well.

> Alternatively, an additional BEFORE-OBJECT argument could be added, which would
> probably require more changes to the code.

I thought about this and a few other options that came to mind, but I don't
think there's any option that's worth the additional effort to implement it.


-- 
Joost Kremers
Life has its moments

bug#70664: 29.3; vtable-insert-object cannot insert at top of table

[Adam Porter]

To be clear, I don't plan to work on this anytime soon.  :)

Having said that, using a special value for AFTER-OBJECT to insert first 
would probably be a good idea, because it seems like it should be 
possible to do that, and it wouldn't require changing the signature.

Beyond that, IMHO it might be good to write a function with a different 
signature that would allow more flexibility, e.g.

(cl-defun vtable-add (object table &key after before at)
   "Add OBJECT to TABLE at specified position.
AFTER may be an object after which to insert it; or BEFORE may be an 
object before which to insert it; or AT may be an integer position at 
which to insert the object, 0 meaning first and -1 meaning last (only 
one of these three arguments should be given).")

Then the old function could be deprecated.

bug#70664: 29.3; vtable-insert-object cannot insert at top of table

[Joost Kremers]

On Thu, May 02 2024, Adam Porter wrote:
> To be clear, I don't plan to work on this anytime soon.  :)

No problem. I would kinda like to give it a try myself, but I'm not going to
make any promises, either. :-)

> Beyond that, IMHO it might be good to write a function with a different
> signature that would allow more flexibility, e.g.
>
> (cl-defun vtable-add (object table &key after before at)
>   "Add OBJECT to TABLE at specified position.
> AFTER may be an object after which to insert it; or BEFORE may be an object
> before which to insert it; or AT may be an integer position at which to insert
> the object, 0 meaning first and -1 meaning last (only one of these three
> arguments should be given).")

I personally don't like the "only one of these three arguments should be given"
part (what happens if more than one are given?), so perhaps a different
suggestion:

(defun vtable-add (object table &optional position before)
   ...)

with POSITION being either an object or an integer. If an object, BEFORE being
non-nil would mean "insert before POSITION", and nil would mean "insert after
POSITION"). If POSITION is an integer, BEFORE is simply ignored. (With this
signature, vtable-insert-object could actually be aliased to vtable-add.)

Though I admit your suggestion has the advantage of explicit keywords.

Do let me know which approach you prefer, in case I do decide to give it a try.

-- 
Joost Kremers
Life has its moments

bug#70664: 29.3; vtable-insert-object cannot insert at top of table

[Adam Porter]

Hi Joost,

On 5/2/24 05:12, Joost Kremers wrote:
> On Thu, May 02 2024, Adam Porter wrote:
>> To be clear, I don't plan to work on this anytime soon.  :)
> 
> No problem. I would kinda like to give it a try myself, but I'm not going to
> make any promises, either. :-)

Of course.  :)

>> Beyond that, IMHO it might be good to write a function with a different
>> signature that would allow more flexibility, e.g.
>>
>> (cl-defun vtable-add (object table &key after before at)
>>    "Add OBJECT to TABLE at specified position.
>> AFTER may be an object after which to insert it; or BEFORE may be an object
>> before which to insert it; or AT may be an integer position at which to insert
>> the object, 0 meaning first and -1 meaning last (only one of these three
>> arguments should be given).")
> 
> I personally don't like the "only one of these three arguments should be given"
> part (what happens if more than one are given?), so perhaps a different
> suggestion:
> 
> (defun vtable-add (object table &optional position before)
>     ...)
> 
> with POSITION being either an object or an integer. If an object, BEFORE being
> non-nil would mean "insert before POSITION", and nil would mean "insert after
> POSITION"). If POSITION is an integer, BEFORE is simply ignored. (With this
> signature, vtable-insert-object could actually be aliased to vtable-add.)
> 
> Though I admit your suggestion has the advantage of explicit keywords.
> 
> Do let me know which approach you prefer, in case I do decide to give it a try.

I generally like to use keywords for clarity when there are more than 
3-4 arguments to a function (also to avoid the "nil nil t" patterns that 
sometimes happens without keywords).

In this case, your idea is slightly less explicit, perhaps requiring 
more careful reading of the docstring, but is more compact, and could 
actually extend the signature of the existing vtable-insert-object 
function, which would seem good.

So, IMHO, I'd suggest applying your idea to the existing 
vtable-insert-object function, i.e. repurposing its existing 
AFTER-OBJECT argument to your new after-object-or-position argument, and 
adding your new before-object-or-position argument (I use those names 
just here for clarity, of course).

(And, BTW, having thought about it further, we should probably keep the 
order of the OBJECT and TABLE arguments as-is in vtable.)

My two cents, anyway.  :)

Thanks,
Adam

bug#70664: 29.3; vtable-insert-object cannot insert at top of table

[Joost Kremers]

[-- Attachment #1: Type: text/plain, Size: 3011 bytes --]

Hi Adam, Eli,

Here's my first attempt at fixing this bug and generally making
`vtable-insert-object` more versatile (see attachment).

A few questions / comments:

- Is this the right place to send this patch? Or should it go to emacs-devel?

- There are quite a few combinations of LOCATION and BEFORE to handle, which may
  make the code a bit hard to follow. A possible alternative would be to define
  some internal helper functions, `vtable--insert-before` and
  `vtable--insert-after`. You guys be the judge. (I added a few comments to
  hopefully make it clearer)

- The patch also updates the documentation. Do check the style, though, my
  writing usually isn't so great.

- I'm sure I made all sorts of mistakes with the commit message.

- I don't know if this change warrants a NEWS entry. It's not really
  user-facing, so I didn't add one.

- The current implementation of vtable-insert-object has a bug: it puts the
  object in the right location in the object list and the cache, but not in the
  buffer. This patch also fixes this bug.

- I don't know how to write a non-interactive test for my changes, because
  `vtable-insert-object` only works if the vtable is being displayed in a
  buffer. So instead I wrote an interactive function to test all possible
  combinations of LOCATION and BEFORE:

```emacs-lisp
(defun test-vtable-insert-object-interactive ()
  "Test `vtable-insert-object'."
  (interactive)
  (let ((buffer (get-buffer-create " *vtable-test*")))
    (pop-to-buffer buffer)
    (erase-buffer)
    (let* ((object1 '("Foo" 3))
           (object2 '("Gazonk" 8))
           (table (make-vtable
                   :columns '("Name" (:name "Rank" :width 5))
                   :objects (list object1 object2))))
      (mapc (lambda (args)
              (pcase-let ((`(,object ,location ,before) args))
                (if (y-or-n-p (format "Update table with location = %s and before = %s?" location before))
                    (vtable-insert-object table object location before))))
            `( ; Some correct inputs.
              ;; object    location        before
              (("Fizz" 4)  ,object1        nil)
              (("Bop"  7)  ,object2        t)
              (("Zat"  5)  2               nil)
              (("Dib"  6)  3               t)
              (("Wup"  9)  nil             nil)
              (("Quam" 2)  nil             t)
              ;; And some faulty inputs.
              (("Yat"  1)  -1              nil) ; non-existing index, `before' is ignored.
              (("Vop"  10) 100             t)   ; non-existing index, `before' is ignored.
              (("Jib"  11) ("Bleh"  0)     nil) ; non-existing object.
              (("Nix"  0)  ("Ugh"   0)     t)   ; non-existing object.
              ))
      (if (y-or-n-p "Regenerate table?")
          (vtable-revert)))))
``` 

The final table should have the "Rank" column sorted in ascending order (0-11).
Regenerating the table should not change it.

-- 
Joost Kremers
Life has its moments



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-vtable-insert-object-more-versatile.patch --]
[-- Type: text/x-patch, Size: 7262 bytes --]

From d48020ff58ff9b2684365772a6161023d4ff22d5 Mon Sep 17 00:00:00 2001
From: Joost Kremers <joostkremers@fastmail.com>
Date: Tue, 7 May 2024 11:52:27 +0200
Subject: [PATCH] Make vtable-insert-object more versatile

Rename argument AFTER-OBJECT to LOCATION; allow use of index to refer to
the insertion position; add argument BEFORE (Bug#70664).
* lisp/emacs-lisp/vtable.el (vtable-insert-object):
* doc/misc/vtable.texi (Interface Functions): Document the change.
---
 doc/misc/vtable.texi      | 18 +++++--
 lisp/emacs-lisp/vtable.el | 98 ++++++++++++++++++++++++++++++---------
 2 files changed, 89 insertions(+), 27 deletions(-)

diff --git a/doc/misc/vtable.texi b/doc/misc/vtable.texi
index dd5b70cf32f..82a12906e7a 100644
--- a/doc/misc/vtable.texi
+++ b/doc/misc/vtable.texi
@@ -548,10 +548,20 @@ Interface Functions
 table.
 @end defun
 
-@defun vtable-insert-object table object &optional after-object
-Insert @var{object} into @var{table}.  If @var{after-object}, insert
-the object after this object; otherwise append to @var{table}.  This
-also updates the displayed table.
+@defun vtable-insert-object table object &optional location before
+Insert @var{object} into @var{table}.  @var{location} should be an
+object in the table, the new object is inserted after this object, or
+before it if @var{before} is non-nil.  If @var{location} is nil,
+@var{object} is appended to @var{table}, or prepended if @var{before} is
+non-nil.
+
+@var{location} can also be an integer, a zero-based index into the
+table.  In this case, @var{object} is inserted at that index.  If the
+index is out of range, @var{object} is prepended to @var{table} if the
+index is too small, or appended if it is too large.  In this case,
+@var{before} is ignored.
+
+This also updates the displayed table.
 @end defun
 
 @defun vtable-update-object table object &optional old-object
diff --git a/lisp/emacs-lisp/vtable.el b/lisp/emacs-lisp/vtable.el
index d8e5136c666..b53df6743ea 100644
--- a/lisp/emacs-lisp/vtable.el
+++ b/lisp/emacs-lisp/vtable.el
@@ -348,19 +348,57 @@ vtable-remove-object
       (when (vtable-goto-object object)
         (delete-line)))))
 
-(defun vtable-insert-object (table object &optional after-object)
-  "Insert OBJECT into TABLE after AFTER-OBJECT.
-If AFTER-OBJECT is nil (or doesn't exist in the table), insert
-OBJECT at the end.
+;; FIXME: The fact that the `location' argument of
+;; `vtable-insert-object' can be an integer and is then interpreted as
+;; an index precludes the use of integers as objects. This seems a very
+;; unlikely use-case, so let's just accept this limitation.
+
+(defun vtable-insert-object (table object &optional location before)
+  "Insert OBJECT into TABLE at LOCATION.
+LOCATION is an object in TABLE.  OBJECT is inserted after LOCATION,
+unless BEFORE is non-nil, in which case it is inserted before LOCATION.
+
+If LOCATION is nil, or does not exist in the table, OBJECT is inserted
+at the end of the table, or at the beginning if BEFORE is non-nil.
+
+LOCATION can also be an integer, a (zero-based) index into the table.
+OBJECT is inserted at this location.  If the index is out of range,
+OBJECT is inserted at the beginning (if the index is less than 0) or
+end (if the index is too large) of the table.  BEFORE is ignored in this
+case.
+
 This also updates the displayed table."
+  ;; FIXME: Inserting an object into an empty vtable currently isn't
+  ;; possible. `nconc' fails silently (twice), and `setcar' on the cache
+  ;; raises an error.
+  (if (null (vtable-objects table))
+      (error "[vtable] Cannot insert object into empty vtable"))
   ;; First insert into the objects.
-  (let (pos)
-    (if (and after-object
-             (setq pos (memq after-object (vtable-objects table))))
-        ;; Splice into list.
-        (setcdr pos (cons object (cdr pos)))
-      ;; Append.
-      (nconc (vtable-objects table) (list object))))
+  (let ((pos (if location
+                 (if (integerp location)
+                     (prog1
+                         (nthcdr location (vtable-objects table))
+                       ;; Do not prepend if index is too large:
+                       (setq before nil))
+                   (or (memq location (vtable-objects table))
+                       ;; Prepend if `location' is not found and
+                       ;; `before' is non-nil:
+                       (and before (vtable-objects table))))
+               ;; If `location' is nil and `before' is non-nil, we
+               ;; prepend the new object.
+               (if before (vtable-objects table)))))
+    (if (or before  ; If `before' is non-nil, `pos' should be, as well.
+            (and pos (integerp location)))
+        ;; Add the new object before.
+        (let ((old-object (car pos)))
+          (setcar pos object)
+          (setcdr pos (cons old-object (cdr pos))))
+      ;; Otherwise, add the object after.
+      (if pos
+          ;; Splice the object into the list.
+          (setcdr pos (cons object (cdr pos)))
+        ;; Otherwise, append the object.
+        (nconc (vtable-objects table) (list object)))))
   ;; Then adjust the cache and display.
   (save-excursion
     (vtable-goto-table table)
@@ -372,19 +410,33 @@ vtable-insert-object
                                      'face (vtable-face table))
                        ""))
            (ellipsis-width (string-pixel-width ellipsis))
-           (elem (and after-object
-                      (assq after-object (car cache))))
+           (elem (if location  ; This binding mirrors the binding of `pos' above.
+                     (if (integerp location)
+                         (nth location (car cache))
+                       (or (assq location (car cache))
+                           (and before (caar cache))))
+                   (if before (caar cache))))
+           (pos (memq elem (car cache)))
            (line (cons object (vtable--compute-cached-line table object))))
-      (if (not elem)
-          ;; Append.
-          (progn
-            (setcar cache (nconc (car cache) (list line)))
-            (vtable-end-of-table))
-        ;; Splice into list.
-        (let ((pos (memq elem (car cache))))
-          (setcdr pos (cons line (cdr pos)))
-          (unless (vtable-goto-object after-object)
-            (vtable-end-of-table))))
+      (if (or before
+              (and pos (integerp location)))
+          ;; Add the new object before:.
+          (let ((old-line (car pos)))
+            (setcar pos line)
+            (setcdr pos (cons old-line (cdr pos)))
+            (unless (vtable-goto-object (car elem))
+              (vtable-beginning-of-table)))
+        ;; Otherwise, add the object after.
+        (if pos
+            ;; Splice the object into the list.
+            (progn
+              (setcdr pos (cons line (cdr pos)))
+              (if (vtable-goto-object location)
+                  (forward-line 1)  ; Insert *after*.
+                (vtable-end-of-table)))
+          ;; Otherwise, append the object.
+          (setcar cache (nconc (car cache) (list line)))
+          (vtable-end-of-table)))
       (let ((start (point)))
         ;; FIXME: We have to adjust colors in lines below this if we
         ;; have :row-colors.
-- 
2.45.0

bug#70664: 29.3; vtable-insert-object cannot insert at top of table

[Eli Zaretskii]

> From: Joost Kremers <joostkremers@fastmail.fm>
> Cc: Eli Zaretskii <eliz@gnu.org>,  70664@debbugs.gnu.org
> Date: Tue, 07 May 2024 12:52:56 +0200
> 
> Here's my first attempt at fixing this bug and generally making
> `vtable-insert-object` more versatile (see attachment).

Thanks.

> A few questions / comments:
> 
> - Is this the right place to send this patch? Or should it go to emacs-devel?

Here is the right place for patches.

> - I don't know if this change warrants a NEWS entry. It's not really
>   user-facing, so I didn't add one.

This changes a public API, so it does need to be called out in NEWS,
just in the section which lists Lisp-level changes.

> - I don't know how to write a non-interactive test for my changes, because
>   `vtable-insert-object` only works if the vtable is being displayed in a
>   buffer. So instead I wrote an interactive function to test all possible
>   combinations of LOCATION and BEFORE:

A test can be interactive (since the test suite can be run
interactively as well), but then please skip the test if it's run in
batch mode.

> +@defun vtable-insert-object table object &optional location before
> +Insert @var{object} into @var{table}.  @var{location} should be an
> +object in the table, the new object is inserted after this object, or
> +before it if @var{before} is non-nil.  If @var{location} is nil,
                                    ^^^                        ^^^
@code{nil}, in both cases.

> +;; FIXME: The fact that the `location' argument of
> +;; `vtable-insert-object' can be an integer and is then interpreted as
> +;; an index precludes the use of integers as objects. This seems a very
                                                       ^^
Two spaces between sentences, please.

bug#70664: 29.3; vtable-insert-object cannot insert at top of table

[Joost Kremers]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

On Thu, May 09 2024, Eli Zaretskii wrote:
> This changes a public API, so it does need to be called out in NEWS,
> just in the section which lists Lisp-level changes.

OK, I added an entry, now contained in the new patch.

> A test can be interactive (since the test suite can be run
> interactively as well), but then please skip the test if it's run in
> batch mode.

Actually, once I took out the 'y-or-n-p' calls, it turned out the test runs fine
non-interactively. I included it in the patch.

> @code{nil}, in both cases.
[...]
> Two spaces between sentences, please.

Done.

Here's the new patch.

-- 
Joost Kremers
Life has its moments


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-vtable-insert-object-more-versatile.patch --]
[-- Type: text/x-patch, Size: 10269 bytes --]

From aacba116ee729663f078e8fb1fee2d0fee01a7a8 Mon Sep 17 00:00:00 2001
From: Joost Kremers <joostkremers@fastmail.com>
Date: Tue, 7 May 2024 11:52:27 +0200
Subject: [PATCH] Make vtable-insert-object more versatile

Rename argument AFTER-OBJECT to LOCATION; allow use of index to refer to
the insertion position; add argument BEFORE (Bug#70664).
* lisp/emacs-lisp/vtable.el (vtable-insert-object):
* doc/misc/vtable.texi (Interface Functions): Document the change.
---
 doc/misc/vtable.texi                 | 18 +++--
 etc/NEWS                             | 13 ++++
 lisp/emacs-lisp/vtable.el            | 98 +++++++++++++++++++++-------
 test/lisp/emacs-lisp/vtable-tests.el | 30 +++++++++
 4 files changed, 132 insertions(+), 27 deletions(-)

diff --git a/doc/misc/vtable.texi b/doc/misc/vtable.texi
index dd5b70cf32f..822b1097cd9 100644
--- a/doc/misc/vtable.texi
+++ b/doc/misc/vtable.texi
@@ -548,10 +548,20 @@ Interface Functions
 table.
 @end defun
 
-@defun vtable-insert-object table object &optional after-object
-Insert @var{object} into @var{table}.  If @var{after-object}, insert
-the object after this object; otherwise append to @var{table}.  This
-also updates the displayed table.
+@defun vtable-insert-object table object &optional location before
+Insert @var{object} into @var{table}.  @var{location} should be an
+object in the table, the new object is inserted after this object, or
+before it if @var{before} is non-nil.  If @var{location} is @code{nil},
+@var{object} is appended to @var{table}, or prepended if @var{before} is
+non-@code{nil}.
+
+@var{location} can also be an integer, a zero-based index into the
+table.  In this case, @var{object} is inserted at that index.  If the
+index is out of range, @var{object} is prepended to @var{table} if the
+index is too small, or appended if it is too large.  In this case,
+@var{before} is ignored.
+
+This also updates the displayed table.
 @end defun
 
 @defun vtable-update-object table object &optional old-object
diff --git a/etc/NEWS b/etc/NEWS
index e2588afeb40..6ed5bf12287 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2563,6 +2563,19 @@ this case, would mean repeating the object in the argument list.)  When
 replacing an object with a different one, passing both the new and old
 objects is still necessary.
 
+** 'vtable-insert-object' can insert "before" or at an index.
+The signature of 'vtable-insert-object' has changed and is now:
+
+(vtable-insert-object table object &optional location before)
+
+'location' corresponds to the old 'after-object' argument; if 'before'
+is non-nil, the new object is inserted before the 'location' object,
+making it possible to insert a new object at the top of the
+table. (Before, this was not possible.)  In addition, 'location' can be
+an integer, a (zero-based) index into the table at which the new object
+is inserted ('before' is ignored in this case).
+
+
 ** JSON
 
 ---
diff --git a/lisp/emacs-lisp/vtable.el b/lisp/emacs-lisp/vtable.el
index d8e5136c666..cb7ea397314 100644
--- a/lisp/emacs-lisp/vtable.el
+++ b/lisp/emacs-lisp/vtable.el
@@ -348,19 +348,57 @@ vtable-remove-object
       (when (vtable-goto-object object)
         (delete-line)))))
 
-(defun vtable-insert-object (table object &optional after-object)
-  "Insert OBJECT into TABLE after AFTER-OBJECT.
-If AFTER-OBJECT is nil (or doesn't exist in the table), insert
-OBJECT at the end.
+;; FIXME: The fact that the `location' argument of
+;; `vtable-insert-object' can be an integer and is then interpreted as
+;; an index precludes the use of integers as objects.  This seems a very
+;; unlikely use-case, so let's just accept this limitation.
+
+(defun vtable-insert-object (table object &optional location before)
+  "Insert OBJECT into TABLE at LOCATION.
+LOCATION is an object in TABLE.  OBJECT is inserted after LOCATION,
+unless BEFORE is non-nil, in which case it is inserted before LOCATION.
+
+If LOCATION is nil, or does not exist in the table, OBJECT is inserted
+at the end of the table, or at the beginning if BEFORE is non-nil.
+
+LOCATION can also be an integer, a (zero-based) index into the table.
+OBJECT is inserted at this location.  If the index is out of range,
+OBJECT is inserted at the beginning (if the index is less than 0) or
+end (if the index is too large) of the table.  BEFORE is ignored in this
+case.
+
 This also updates the displayed table."
+  ;; FIXME: Inserting an object into an empty vtable currently isn't
+  ;; possible. `nconc' fails silently (twice), and `setcar' on the cache
+  ;; raises an error.
+  (if (null (vtable-objects table))
+      (error "[vtable] Cannot insert object into empty vtable"))
   ;; First insert into the objects.
-  (let (pos)
-    (if (and after-object
-             (setq pos (memq after-object (vtable-objects table))))
-        ;; Splice into list.
-        (setcdr pos (cons object (cdr pos)))
-      ;; Append.
-      (nconc (vtable-objects table) (list object))))
+  (let ((pos (if location
+                 (if (integerp location)
+                     (prog1
+                         (nthcdr location (vtable-objects table))
+                       ;; Do not prepend if index is too large:
+                       (setq before nil))
+                   (or (memq location (vtable-objects table))
+                       ;; Prepend if `location' is not found and
+                       ;; `before' is non-nil:
+                       (and before (vtable-objects table))))
+               ;; If `location' is nil and `before' is non-nil, we
+               ;; prepend the new object.
+               (if before (vtable-objects table)))))
+    (if (or before  ; If `before' is non-nil, `pos' should be, as well.
+            (and pos (integerp location)))
+        ;; Add the new object before.
+        (let ((old-object (car pos)))
+          (setcar pos object)
+          (setcdr pos (cons old-object (cdr pos))))
+      ;; Otherwise, add the object after.
+      (if pos
+          ;; Splice the object into the list.
+          (setcdr pos (cons object (cdr pos)))
+        ;; Otherwise, append the object.
+        (nconc (vtable-objects table) (list object)))))
   ;; Then adjust the cache and display.
   (save-excursion
     (vtable-goto-table table)
@@ -372,19 +410,33 @@ vtable-insert-object
                                      'face (vtable-face table))
                        ""))
            (ellipsis-width (string-pixel-width ellipsis))
-           (elem (and after-object
-                      (assq after-object (car cache))))
+           (elem (if location  ; This binding mirrors the binding of `pos' above.
+                     (if (integerp location)
+                         (nth location (car cache))
+                       (or (assq location (car cache))
+                           (and before (caar cache))))
+                   (if before (caar cache))))
+           (pos (memq elem (car cache)))
            (line (cons object (vtable--compute-cached-line table object))))
-      (if (not elem)
-          ;; Append.
-          (progn
-            (setcar cache (nconc (car cache) (list line)))
-            (vtable-end-of-table))
-        ;; Splice into list.
-        (let ((pos (memq elem (car cache))))
-          (setcdr pos (cons line (cdr pos)))
-          (unless (vtable-goto-object after-object)
-            (vtable-end-of-table))))
+      (if (or before
+              (and pos (integerp location)))
+          ;; Add the new object before:.
+          (let ((old-line (car pos)))
+            (setcar pos line)
+            (setcdr pos (cons old-line (cdr pos)))
+            (unless (vtable-goto-object (car elem))
+              (vtable-beginning-of-table)))
+        ;; Otherwise, add the object after.
+        (if pos
+            ;; Splice the object into the list.
+            (progn
+              (setcdr pos (cons line (cdr pos)))
+              (if (vtable-goto-object location)
+                  (forward-line 1)  ; Insert *after*.
+                (vtable-end-of-table)))
+          ;; Otherwise, append the object.
+          (setcar cache (nconc (car cache) (list line)))
+          (vtable-end-of-table)))
       (let ((start (point)))
         ;; FIXME: We have to adjust colors in lines below this if we
         ;; have :row-colors.
diff --git a/test/lisp/emacs-lisp/vtable-tests.el b/test/lisp/emacs-lisp/vtable-tests.el
index 08fdf1594a4..1d4b0650210 100644
--- a/test/lisp/emacs-lisp/vtable-tests.el
+++ b/test/lisp/emacs-lisp/vtable-tests.el
@@ -39,4 +39,34 @@ test-vstable-compute-columns
                          :insert nil)))
           '(left right left))))
 
+(ert-deftest test-vtable-insert-object ()
+  (should
+   (equal (let ((buffer (get-buffer-create " *vtable-test*")))
+            (pop-to-buffer buffer)
+            (erase-buffer)
+            (let* ((object1 '("Foo" 3))
+                   (object2 '("Gazonk" 8))
+                   (table (make-vtable
+                           :columns '("Name" (:name "Rank" :width 5))
+                           :objects (list object1 object2))))
+              (mapc (lambda (args)
+                      (pcase-let ((`(,object ,location ,before) args))
+                        (vtable-insert-object table object location before)))
+                    `( ; Some correct inputs.
+                      ;; object    location        before
+                      (("Fizz" 4)  ,object1        nil)
+                      (("Bop"  7)  ,object2        t)
+                      (("Zat"  5)  2               nil)
+                      (("Dib"  6)  3               t)
+                      (("Wup"  9)  nil             nil)
+                      (("Quam" 2)  nil             t)
+                      ;; And some faulty inputs.
+                      (("Yat"  1)  -1              nil) ; non-existing index, `before' is ignored.
+                      (("Vop"  10) 100             t)   ; non-existing index, `before' is ignored.
+                      (("Jib"  11) ("Bleh"  0)     nil) ; non-existing object.
+                      (("Nix"  0)  ("Ugh"   0)     t)   ; non-existing object.
+                      ))
+              (mapcar #'cadr (vtable-objects table))))
+          (number-sequence 0 11))))
+
 ;;; vtable-tests.el ends here
-- 
2.45.0

bug#70664: 29.3; vtable-insert-object cannot insert at top of table

[Eli Zaretskii]

> From: Joost Kremers <joostkremers@fastmail.fm>
> Cc: adam@alphapapa.net,  70664@debbugs.gnu.org
> Date: Thu, 09 May 2024 18:45:52 +0200
> 
> On Thu, May 09 2024, Eli Zaretskii wrote:
> > This changes a public API, so it does need to be called out in NEWS,
> > just in the section which lists Lisp-level changes.
> 
> OK, I added an entry, now contained in the new patch.
> 
> > A test can be interactive (since the test suite can be run
> > interactively as well), but then please skip the test if it's run in
> > batch mode.
> 
> Actually, once I took out the 'y-or-n-p' calls, it turned out the test runs fine
> non-interactively. I included it in the patch.
> 
> > @code{nil}, in both cases.
> [...]
> > Two spaces between sentences, please.
> 
> Done.
> 
> Here's the new patch.

Thanks, installed on the master branch, and closing the bug.