* bug#70664: 29.3; vtable-insert-object cannot insert at top of table
@ 2024-04-30 9:30 Joost Kremers
2024-04-30 12:18 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Joost Kremers @ 2024-04-30 9:30 UTC (permalink / raw)
To: 70664
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
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#70664: 29.3; vtable-insert-object cannot insert at top of table 2024-04-30 9:30 bug#70664: 29.3; vtable-insert-object cannot insert at top of table Joost Kremers @ 2024-04-30 12:18 ` Eli Zaretskii [not found] ` <d55f9a57-c9aa-439e-b8e1-004f445f1a24@alphapapa.net> 0 siblings, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2024-04-30 12:18 UTC (permalink / raw) To: Joost Kremers, Adam Porter; +Cc: 70664 > 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <d55f9a57-c9aa-439e-b8e1-004f445f1a24@alphapapa.net>]
* bug#70664: 29.3; vtable-insert-object cannot insert at top of table [not found] ` <d55f9a57-c9aa-439e-b8e1-004f445f1a24@alphapapa.net> @ 2024-05-02 6:52 ` Joost Kremers 2024-05-02 9:54 ` Adam Porter 0 siblings, 1 reply; 10+ messages in thread From: Joost Kremers @ 2024-05-02 6:52 UTC (permalink / raw) To: Adam Porter; +Cc: Eli Zaretskii, 70664 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#70664: 29.3; vtable-insert-object cannot insert at top of table 2024-05-02 6:52 ` Joost Kremers @ 2024-05-02 9:54 ` Adam Porter 2024-05-02 10:12 ` Joost Kremers 0 siblings, 1 reply; 10+ messages in thread From: Adam Porter @ 2024-05-02 9:54 UTC (permalink / raw) To: Joost Kremers; +Cc: Eli Zaretskii, 70664 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#70664: 29.3; vtable-insert-object cannot insert at top of table 2024-05-02 9:54 ` Adam Porter @ 2024-05-02 10:12 ` Joost Kremers 2024-05-03 4:16 ` Adam Porter 0 siblings, 1 reply; 10+ messages in thread From: Joost Kremers @ 2024-05-02 10:12 UTC (permalink / raw) To: Adam Porter; +Cc: Eli Zaretskii, 70664 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#70664: 29.3; vtable-insert-object cannot insert at top of table 2024-05-02 10:12 ` Joost Kremers @ 2024-05-03 4:16 ` Adam Porter 2024-05-07 10:52 ` Joost Kremers 0 siblings, 1 reply; 10+ messages in thread From: Adam Porter @ 2024-05-03 4:16 UTC (permalink / raw) To: Joost Kremers; +Cc: Eli Zaretskii, 70664 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#70664: 29.3; vtable-insert-object cannot insert at top of table 2024-05-03 4:16 ` Adam Porter @ 2024-05-07 10:52 ` Joost Kremers 2024-05-09 8:45 ` Eli Zaretskii 0 siblings, 1 reply; 10+ messages in thread From: Joost Kremers @ 2024-05-07 10:52 UTC (permalink / raw) To: Adam Porter; +Cc: Eli Zaretskii, 70664 [-- 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#70664: 29.3; vtable-insert-object cannot insert at top of table 2024-05-07 10:52 ` Joost Kremers @ 2024-05-09 8:45 ` Eli Zaretskii 2024-05-09 16:45 ` Joost Kremers 0 siblings, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2024-05-09 8:45 UTC (permalink / raw) To: Joost Kremers; +Cc: adam, 70664 > 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#70664: 29.3; vtable-insert-object cannot insert at top of table 2024-05-09 8:45 ` Eli Zaretskii @ 2024-05-09 16:45 ` Joost Kremers 2024-05-18 8:54 ` Eli Zaretskii 0 siblings, 1 reply; 10+ messages in thread From: Joost Kremers @ 2024-05-09 16:45 UTC (permalink / raw) To: Eli Zaretskii; +Cc: adam, 70664 [-- 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#70664: 29.3; vtable-insert-object cannot insert at top of table 2024-05-09 16:45 ` Joost Kremers @ 2024-05-18 8:54 ` Eli Zaretskii 0 siblings, 0 replies; 10+ messages in thread From: Eli Zaretskii @ 2024-05-18 8:54 UTC (permalink / raw) To: Joost Kremers; +Cc: adam, 70664-done > 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-05-18 8:54 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-30 9:30 bug#70664: 29.3; vtable-insert-object cannot insert at top of table Joost Kremers
2024-04-30 12:18 ` Eli Zaretskii
[not found] ` <d55f9a57-c9aa-439e-b8e1-004f445f1a24@alphapapa.net>
2024-05-02 6:52 ` Joost Kremers
2024-05-02 9:54 ` Adam Porter
2024-05-02 10:12 ` Joost Kremers
2024-05-03 4:16 ` Adam Porter
2024-05-07 10:52 ` Joost Kremers
2024-05-09 8:45 ` Eli Zaretskii
2024-05-09 16:45 ` Joost Kremers
2024-05-18 8:54 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).