* bug#69666: [PATCH] (vtable-update-object): Make old-object argument optional @ 2024-03-09 5:51 Adam Porter 2024-03-14 9:00 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: Adam Porter @ 2024-03-09 5:51 UTC (permalink / raw) To: 69666 [-- Attachment #1: Type: text/plain, Size: 225 bytes --] Hi, Please see the attached patch which makes `vtable-update-object' easier to use in the common case of updating an existing object's representation in a table (rather than replacing it with another object). Thanks, Adam [-- Attachment #2: 0001-vtable-update-object-Make-old-object-argument-option.patch --] [-- Type: text/x-patch, Size: 2337 bytes --] From ebb5a2a6ea5bc0437fd39d0e87406fe723183e5a Mon Sep 17 00:00:00 2001 From: Adam Porter <adam@alphapapa.net> Date: Fri, 8 Mar 2024 23:43:14 -0600 Subject: [PATCH] (vtable-update-object): Make old-object argument optional * lisp/emacs-lisp/vtable.el (vtable-update-object): Make 'old-object' argument optional. * doc/misc/vtable.texi (Interface Functions): Update documentation. It's often necessary to update the representation of a single object in a table (e.g a struct, whose identity does not change when its slots' values are changed). To do so, now the function may be called like this: (vtable-update-object table foo) Instead of like this: (vtable-update-object table foo foo) --- doc/misc/vtable.texi | 7 ++++--- lisp/emacs-lisp/vtable.el | 7 +++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/doc/misc/vtable.texi b/doc/misc/vtable.texi index a4f2ed29d93..f6a2ae05818 100644 --- a/doc/misc/vtable.texi +++ b/doc/misc/vtable.texi @@ -554,9 +554,10 @@ Interface Functions also updates the displayed table. @end defun -@defun vtable-update-object table object old-object -Change @var{old-object} into @var{object} in @var{table}. This also -updates the displayed table. +@defun vtable-update-object table object &optional old-object +Change @var{old-object} into @var{object} in @var{table}; or, without +@var{old-object}, update existing @var{object} in @var{table}. This +also updates the displayed table. This has the same effect as calling @code{vtable-remove-object} and then @code{vtable-insert-object}, but is more efficient. diff --git a/lisp/emacs-lisp/vtable.el b/lisp/emacs-lisp/vtable.el index 02020552e7f..ae6a5296cef 100644 --- a/lisp/emacs-lisp/vtable.el +++ b/lisp/emacs-lisp/vtable.el @@ -283,8 +283,11 @@ vtable-goto-column (goto-char (prop-match-beginning match)) (end-of-line))) -(defun vtable-update-object (table object old-object) - "Replace OLD-OBJECT in TABLE with OBJECT." +(defun vtable-update-object (table object &optional old-object) + "Replace OLD-OBJECT in TABLE with OBJECT. +Without OLD-OBJECT, just update existing OBJECT in TABLE." + (unless old-object + (setq old-object object)) (let* ((objects (vtable-objects table)) (inhibit-read-only t)) ;; First replace the object in the object storage. -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#69666: [PATCH] (vtable-update-object): Make old-object argument optional 2024-03-09 5:51 bug#69666: [PATCH] (vtable-update-object): Make old-object argument optional Adam Porter @ 2024-03-14 9:00 ` Eli Zaretskii 2024-03-16 0:41 ` Adam Porter 0 siblings, 1 reply; 6+ messages in thread From: Eli Zaretskii @ 2024-03-14 9:00 UTC (permalink / raw) To: Adam Porter; +Cc: 69666 > Date: Fri, 8 Mar 2024 23:51:33 -0600 > From: Adam Porter <adam@alphapapa.net> > > Please see the attached patch which makes `vtable-update-object' easier > to use in the common case of updating an existing object's > representation in a table (rather than replacing it with another object). Thanks, I have some minor comments below. > Subject: [PATCH] (vtable-update-object): Make old-object argument optional Since this changes the API of a public function, we need to call this out in NEWS. > +@defun vtable-update-object table object &optional old-object > +Change @var{old-object} into @var{object} in @var{table}; or, without > +@var{old-object}, update existing @var{object} in @var{table}. This > +also updates the displayed table. This is backwards: the documentation should first say what happens if the function is called with just 2 arguments, and then what happens if the 3rd one is supplied. Like this: Update @var{object} in @var{table} and redisplay @var{table}. Optional argument @var{old-object}, if non-@code{nil}, means to change @var{old-object} into @var{object}. > +(defun vtable-update-object (table object &optional old-object) > + "Replace OLD-OBJECT in TABLE with OBJECT. > +Without OLD-OBJECT, just update existing OBJECT in TABLE." Same here. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#69666: [PATCH] (vtable-update-object): Make old-object argument optional 2024-03-14 9:00 ` Eli Zaretskii @ 2024-03-16 0:41 ` Adam Porter 2024-03-16 10:29 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: Adam Porter @ 2024-03-16 0:41 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 69666 [-- Attachment #1: Type: text/plain, Size: 592 bytes --] Hi Eli, On 3/14/24 04:00, Eli Zaretskii wrote: >> Date: Fri, 8 Mar 2024 23:51:33 -0600 >> From: Adam Porter <adam@alphapapa.net> >> >> Please see the attached patch which makes `vtable-update-object' easier >> to use in the common case of updating an existing object's >> representation in a table (rather than replacing it with another object). > > Thanks, I have some minor comments below. Thanks for your review. Please see the attached patch which addresses those three items and is rebased on current master. Please let me know if I need to make any further changes. Thanks, Adam [-- Attachment #2: 0001-vtable-update-object-Make-old-object-argument-option.patch --] [-- Type: text/x-patch, Size: 3527 bytes --] From 4b762015ebb82d1861c2efe51c1e902ac329bc8e Mon Sep 17 00:00:00 2001 From: Adam Porter <adam@alphapapa.net> Date: Fri, 8 Mar 2024 23:43:14 -0600 Subject: [PATCH] (vtable-update-object): Make old-object argument optional (Bug#69666) * lisp/emacs-lisp/vtable.el (vtable-update-object): Make 'old-object' argument optional. * doc/misc/vtable.texi (Interface Functions): Update documentation. * etc/NEWS: Add news entry. It's often necessary to update the representation of a single object in a table (e.g a struct, whose identity does not change when its slots' values are changed). To do so, now the function may be called like this: (vtable-update-object table object) Instead of like this: (vtable-update-object table object object) --- doc/misc/vtable.texi | 9 ++++++--- etc/NEWS | 9 +++++++++ lisp/emacs-lisp/vtable.el | 9 +++++++-- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/doc/misc/vtable.texi b/doc/misc/vtable.texi index a4f2ed29d93..6881e9663c4 100644 --- a/doc/misc/vtable.texi +++ b/doc/misc/vtable.texi @@ -554,9 +554,12 @@ Interface Functions also updates the displayed table. @end defun -@defun vtable-update-object table object old-object -Change @var{old-object} into @var{object} in @var{table}. This also -updates the displayed table. +@defun vtable-update-object table object &optional old-object +Update @var{object}'s representation in @var{table}. Optional argument +@var{old-object}, if non-@code{nil}, means to replace @var{old-object} +with @var{object} and redisplay the associated row in the table. In +either case, if the existing object is not found in the table (being +compared with @code{equal}), signal an error. This has the same effect as calling @code{vtable-remove-object} and then @code{vtable-insert-object}, but is more efficient. diff --git a/etc/NEWS b/etc/NEWS index a654d2d8d79..85751074aaa 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -2175,6 +2175,15 @@ aforementioned functions: (and (arrayp executing-kbd-macro) (>= executing-kbd-macro-index (length executing-kbd-macro)))) ++++ +** 'vtable-update-object' updates an existing object with just two arguments. +It is now possible to update the representation of an object in a vtable +by calling 'vtable-update-object' with just the vtable and the object as +arguments. (Previously the 'old-object' argument was required which, in +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. + \f * Changes in Emacs 30.1 on Non-Free Operating Systems diff --git a/lisp/emacs-lisp/vtable.el b/lisp/emacs-lisp/vtable.el index 15a430f5c26..dfcfa434135 100644 --- a/lisp/emacs-lisp/vtable.el +++ b/lisp/emacs-lisp/vtable.el @@ -283,8 +283,13 @@ vtable-goto-column (goto-char (prop-match-beginning match)) (end-of-line))) -(defun vtable-update-object (table object old-object) - "Replace OLD-OBJECT in TABLE with OBJECT." +(defun vtable-update-object (table object &optional old-object) + "Update OBJECT's representation in TABLE. +When OLD-OBJECT is non-nil, replace OLD-OBJECT with OBJECT and display +it. In either case, if the existing object is not found in the +table (being compared with `equal'), signal an error." + (unless old-object + (setq old-object object)) (let* ((objects (vtable-objects table)) (inhibit-read-only t)) ;; First replace the object in the object storage. -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#69666: [PATCH] (vtable-update-object): Make old-object argument optional 2024-03-16 0:41 ` Adam Porter @ 2024-03-16 10:29 ` Eli Zaretskii 2024-03-17 4:29 ` Adam Porter 0 siblings, 1 reply; 6+ messages in thread From: Eli Zaretskii @ 2024-03-16 10:29 UTC (permalink / raw) To: Adam Porter; +Cc: 69666 > Date: Fri, 15 Mar 2024 19:41:33 -0500 > Cc: 69666@debbugs.gnu.org > From: Adam Porter <adam@alphapapa.net> > > > Thanks, I have some minor comments below. > > Thanks for your review. Please see the attached patch which addresses > those three items and is rebased on current master. Please let me know > if I need to make any further changes. Thanks, this LGTM. Just a couple of minor nits: > Subject: [PATCH] (vtable-update-object): Make old-object argument optional > (Bug#69666) This is too long for the generated ChangeLog. Since this just reiterates what the log message below it says, I suggest: . make the heading more terse, like 'vtable-update-object' can now be called with one argument . move the bug number to the actual log entry: * lisp/emacs-lisp/vtable.el (vtable-update-object): Make 'old-object' argument optional. (Bug#69666) > +(defun vtable-update-object (table object &optional old-object) > + "Update OBJECT's representation in TABLE. > +When OLD-OBJECT is non-nil, replace OLD-OBJECT with OBJECT and display "When" has a meaning of time, which is not what you mean here. I suggest to use "If" in these cases. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#69666: [PATCH] (vtable-update-object): Make old-object argument optional 2024-03-16 10:29 ` Eli Zaretskii @ 2024-03-17 4:29 ` Adam Porter 2024-03-21 10:44 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: Adam Porter @ 2024-03-17 4:29 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 69666 [-- Attachment #1: Type: text/plain, Size: 1130 bytes --] Hi Eli, Thanks, please see the attached patch. On 3/16/24 05:29, Eli Zaretskii wrote: > Thanks, this LGTM. Just a couple of minor nits: > >> Subject: [PATCH] (vtable-update-object): Make old-object argument >> optional (Bug#69666) > > This is too long for the generated ChangeLog. Since this just > reiterates what the log message below it says, I suggest: > > . make the heading more terse, like > > 'vtable-update-object' can now be called with one argument > > . move the bug number to the actual log entry: > > * lisp/emacs-lisp/vtable.el (vtable-update-object): Make > 'old-object' argument optional. (Bug#69666) Done. For future reference, is there a way to know whether the heading line is too long for the ChangeLog entry, other than manually counting? > "When" has a meaning of time, which is not what you mean here. I > suggest to use "If" in these cases. Done. Also, please note that I just discovered another limitation of this function, which I've filed as bug#69837. Since it seems quite important, I've also updated the patch to mention the limitation in the docstring and manual. Thanks, Adam [-- Attachment #2: 0001-vtable-update-object-can-now-be-called-with-one-argu.patch --] [-- Type: text/x-patch, Size: 4623 bytes --] From 92e2d16b0da3dd1d19b7ac4e3a80fec7967f1259 Mon Sep 17 00:00:00 2001 From: Adam Porter <adam@alphapapa.net> Date: Fri, 8 Mar 2024 23:43:14 -0600 Subject: [PATCH] 'vtable-update-object' can now be called with one argument * lisp/emacs-lisp/vtable.el (vtable-update-object): Make 'old-object' argument optional. (Bug#69666) * doc/misc/vtable.texi (Interface Functions): Update documentation. * etc/NEWS: Add news entry. It's often necessary to update the representation of a single object in a table (e.g a struct, whose identity does not change when its slots' values are changed). To do so, now the function may be called like this: (vtable-update-object table object) Instead of like this: (vtable-update-object table object object) This also documents the behavior of the just-discovered limitation filed as bug#69837. --- doc/misc/vtable.texi | 13 ++++++++++--- etc/NEWS | 9 +++++++++ lisp/emacs-lisp/vtable.el | 15 +++++++++++++-- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/doc/misc/vtable.texi b/doc/misc/vtable.texi index a4f2ed29d93..dd5b70cf32f 100644 --- a/doc/misc/vtable.texi +++ b/doc/misc/vtable.texi @@ -554,12 +554,19 @@ Interface Functions also updates the displayed table. @end defun -@defun vtable-update-object table object old-object -Change @var{old-object} into @var{object} in @var{table}. This also -updates the displayed table. +@defun vtable-update-object table object &optional old-object +Update @var{object}'s representation in @var{table}. Optional argument +@var{old-object}, if non-@code{nil}, means to replace @var{old-object} +with @var{object} and redisplay the associated row in the table. In +either case, if the existing object is not found in the table (being +compared with @code{equal}), signal an error. This has the same effect as calling @code{vtable-remove-object} and then @code{vtable-insert-object}, but is more efficient. + +Note a limitation: if the table's buffer is not in a visible window, or +if its window has changed width since it was updated, updating the table +is not possible, and an error is signaled. @end defun @defun vtable-column table index diff --git a/etc/NEWS b/etc/NEWS index a654d2d8d79..85751074aaa 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -2175,6 +2175,15 @@ aforementioned functions: (and (arrayp executing-kbd-macro) (>= executing-kbd-macro-index (length executing-kbd-macro)))) ++++ +** 'vtable-update-object' updates an existing object with just two arguments. +It is now possible to update the representation of an object in a vtable +by calling 'vtable-update-object' with just the vtable and the object as +arguments. (Previously the 'old-object' argument was required which, in +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. + \f * Changes in Emacs 30.1 on Non-Free Operating Systems diff --git a/lisp/emacs-lisp/vtable.el b/lisp/emacs-lisp/vtable.el index 15a430f5c26..de52ab8b36f 100644 --- a/lisp/emacs-lisp/vtable.el +++ b/lisp/emacs-lisp/vtable.el @@ -283,8 +283,16 @@ vtable-goto-column (goto-char (prop-match-beginning match)) (end-of-line))) -(defun vtable-update-object (table object old-object) - "Replace OLD-OBJECT in TABLE with OBJECT." +(defun vtable-update-object (table object &optional old-object) + "Update OBJECT's representation in TABLE. +If OLD-OBJECT is non-nil, replace OLD-OBJECT with OBJECT and display it. +In either case, if the existing object is not found in the table (being +compared with `equal'), signal an error. Note a limitation: if TABLE's +buffer is not in a visible window, or if its window has changed width +since it was updated, updating the TABLE is not possible, and an error +is signaled." + (unless old-object + (setq old-object object)) (let* ((objects (vtable-objects table)) (inhibit-read-only t)) ;; First replace the object in the object storage. @@ -300,6 +308,9 @@ vtable-update-object (error "Can't find the old object")) (setcar (cdr objects) object)) ;; Then update the cache... + ;; FIXME: If the table's buffer has no visible window, or if its + ;; width has changed since the table was updated, the cache key will + ;; not match and the object can't be updated. (Bug #69837). (if-let ((line-number (seq-position (car (vtable--cache table)) old-object (lambda (a b) (equal (car a) b)))) -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#69666: [PATCH] (vtable-update-object): Make old-object argument optional 2024-03-17 4:29 ` Adam Porter @ 2024-03-21 10:44 ` Eli Zaretskii 0 siblings, 0 replies; 6+ messages in thread From: Eli Zaretskii @ 2024-03-21 10:44 UTC (permalink / raw) To: Adam Porter; +Cc: 69666-done > Date: Sat, 16 Mar 2024 23:29:52 -0500 > Cc: 69666@debbugs.gnu.org > From: Adam Porter <adam@alphapapa.net> > > Thanks, please see the attached patch. Thanks, installed, and closing the bug. > For future reference, is there a way to know whether the heading line is > too long for the ChangeLog entry, other than manually counting? I suggest to arrange for fill-column to be set in the buffers where you edit the log messages, then M-q or auto-fill-mode will do this for you. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-21 10:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-09 5:51 bug#69666: [PATCH] (vtable-update-object): Make old-object argument optional Adam Porter 2024-03-14 9:00 ` Eli Zaretskii 2024-03-16 0:41 ` Adam Porter 2024-03-16 10:29 ` Eli Zaretskii 2024-03-17 4:29 ` Adam Porter 2024-03-21 10:44 ` 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).