* Re: master 048e72a403e 1/2: Add file/buffer-to-register (Bug#73308) [not found] ` <20241005100743.4B6DB1AA78D@vcs3.savannah.gnu.org> @ 2024-10-06 7:52 ` Eshel Yaron 2024-10-06 23:21 ` Barra Ó Catháin via Emacs development discussions. 0 siblings, 1 reply; 4+ messages in thread From: Eshel Yaron @ 2024-10-06 7:52 UTC (permalink / raw) To: emacs-devel; +Cc: Barra Ó Catháin Eli Zaretskii <eliz@gnu.org> writes: > branch: master > commit 048e72a403e22dc394b265c6ff290b8d40e806a5 > Author: Barra Ó Catháin <barra@ocathain.ie> > Commit: Eli Zaretskii <eliz@gnu.org> > > Add file/buffer-to-register (Bug#73308) [...] > +(defun buffer-to-register (buffer register) > + "Insert BUFFER into REGISTER. > +To visit the buffer, use \\[jump-to-register]. > + > +Interactively, prompt for REGISTER using `register-read-with-preview'. > +With a prefix-argument, prompt for BUFFER-NAME using `read-buffer', > +With no prefix-argument, use the current buffer for BUFFER." > + (interactive (list (if (eq current-prefix-arg nil) > + (current-buffer) > + (read-buffer "Buffer: ")) > + (register-read-with-preview "Buffer to register: "))) > + (add-hook 'kill-buffer-hook 'register-buffer-to-file-query nil t) > + (set-register register (cons 'buffer buffer))) Hmm, this looks like a subtle bug: the BUFFER argument can be any buffer, but the add-hook call is always local to the current buffer. Shouldn't that add-hook happen inside a with-current-buffer or something like that? Best, Eshel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: master 048e72a403e 1/2: Add file/buffer-to-register (Bug#73308) 2024-10-06 7:52 ` master 048e72a403e 1/2: Add file/buffer-to-register (Bug#73308) Eshel Yaron @ 2024-10-06 23:21 ` Barra Ó Catháin via Emacs development discussions. 2024-10-08 9:36 ` Eshel Yaron 0 siblings, 1 reply; 4+ messages in thread From: Barra Ó Catháin via Emacs development discussions. @ 2024-10-06 23:21 UTC (permalink / raw) To: Eshel Yaron; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 325 bytes --] D'oh! You're absolutely correct. I've attached a patch containing a fix for exactly this; It adds a check to see if buffer is currently alive, and uses with-current-buffer to ensure that it's adding to the correct one. Let me know if I should send this somewhere else; I'm still learning mailing list development etiquette. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Fix-buffer-to-register-add-hook.patch --] [-- Type: text/patch, Size: 1215 bytes --] From 91cee4c5a72fb0fba5513d2fcab0d2596bfba684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barra=20=C3=93=20Cath=C3=A1in?= <barra@ocathain.ie> Date: Mon, 7 Oct 2024 00:16:23 +0100 Subject: [PATCH] Fix buffer-to-register add-hook. * lisp/register.el (buffer-to-register): Add with-current-buffer so add-hook correctly refers to buffer when given argument. --- lisp/register.el | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lisp/register.el b/lisp/register.el index 1b4281ae4ec..d9dbc3ab186 100644 --- a/lisp/register.el +++ b/lisp/register.el @@ -728,8 +728,10 @@ buffer-to-register (current-buffer) (read-buffer "Buffer: ")) (register-read-with-preview "Buffer to register: "))) - (add-hook 'kill-buffer-hook 'register-buffer-to-file-query nil t) - (set-register register (cons 'buffer buffer))) + (let ((buffer (or (get-buffer buffer) buffer))) + (with-current-buffer buffer + (add-hook 'kill-buffer-hook 'register-buffer-to-file-query nil t)) + (set-register register (cons 'buffer buffer)))) (cl-defgeneric register-val-jump-to (_val _arg) "Execute the \"jump\" operation of VAL. -- 2.46.0 [-- Attachment #3: Type: text/plain, Size: 1342 bytes --] Thanks for the catch! ----- Barra Eshel Yaron <me@eshelyaron.com> writes: > Eli Zaretskii <eliz@gnu.org> writes: > >> branch: master >> commit 048e72a403e22dc394b265c6ff290b8d40e806a5 >> Author: Barra Ó Catháin <barra@ocathain.ie> >> Commit: Eli Zaretskii <eliz@gnu.org> >> >> Add file/buffer-to-register (Bug#73308) > > [...] > >> +(defun buffer-to-register (buffer register) >> + "Insert BUFFER into REGISTER. >> +To visit the buffer, use \\[jump-to-register]. >> + >> +Interactively, prompt for REGISTER using `register-read-with-preview'. >> +With a prefix-argument, prompt for BUFFER-NAME using `read-buffer', >> +With no prefix-argument, use the current buffer for BUFFER." >> + (interactive (list (if (eq current-prefix-arg nil) >> + (current-buffer) >> + (read-buffer "Buffer: ")) >> + (register-read-with-preview "Buffer to register: "))) >> + (add-hook 'kill-buffer-hook 'register-buffer-to-file-query nil t) >> + (set-register register (cons 'buffer buffer))) > > Hmm, this looks like a subtle bug: the BUFFER argument can be any > buffer, but the add-hook call is always local to the current buffer. > Shouldn't that add-hook happen inside a with-current-buffer or something > like that? > > > Best, > > Eshel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: master 048e72a403e 1/2: Add file/buffer-to-register (Bug#73308) 2024-10-06 23:21 ` Barra Ó Catháin via Emacs development discussions. @ 2024-10-08 9:36 ` Eshel Yaron 2024-10-08 10:07 ` Barra Ó Catháin via Emacs development discussions. 0 siblings, 1 reply; 4+ messages in thread From: Eshel Yaron @ 2024-10-08 9:36 UTC (permalink / raw) To: Barra Ó Catháin; +Cc: emacs-devel Hi, Barra Ó Catháin <barra@ocathain.ie> writes: > D'oh! You're absolutely correct. I've attached a patch containing a fix > for exactly this; adds a check to see if buffer is currently alive, > and uses with-current-buffer to ensure that it's adding to the correct > one. Thanks! I've applied your patch (with a minor tweak to the commit message) in commit c0f10fc3d48. I then followed up with a couple of small refinements in commit 6a272dedad3It. I mostly improved the docstring and the minibuffer prompts, but note that I removed the (let ((buffer (or (get-buffer buffer) buffer))) ...) binding: it only has any effect if BUFFER is a string, which it isn't, it's a buffer. Let me know if I misread something. > Let me know if I should send this somewhere else; I'm still > learning mailing list development etiquette. This is fine for such a follow up, I think. Best, Eshel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: master 048e72a403e 1/2: Add file/buffer-to-register (Bug#73308) 2024-10-08 9:36 ` Eshel Yaron @ 2024-10-08 10:07 ` Barra Ó Catháin via Emacs development discussions. 0 siblings, 0 replies; 4+ messages in thread From: Barra Ó Catháin via Emacs development discussions. @ 2024-10-08 10:07 UTC (permalink / raw) To: Eshel Yaron; +Cc: emacs-devel Looks nice to me; the binding I was trying to do was mostly because I didn't actually require a match in read-buffer, and read-buffer returns a string. It looks silly because it was silly; as you have done here, should have just gotten a buffer in the first place! (the only difference in functionality would be the ability to specify a buffer that isn't currently open. Being that this is mostly going to be used interactively... just open a buffer :P) I'm personally happy enough with how it behaves now. Thanks for your help! Thanks, ----- Barra Eshel Yaron <me@eshelyaron.com> writes: > Hi, > > Barra Ó Catháin <barra@ocathain.ie> writes: > >> D'oh! You're absolutely correct. I've attached a patch containing a fix >> for exactly this; adds a check to see if buffer is currently alive, >> and uses with-current-buffer to ensure that it's adding to the correct >> one. > > Thanks! I've applied your patch (with a minor tweak to the commit > message) in commit c0f10fc3d48. I then followed up with a couple of > small refinements in commit 6a272dedad3It. I mostly improved the > docstring and the minibuffer prompts, but note that I removed the > (let ((buffer (or (get-buffer buffer) buffer))) ...) binding: it only > has any effect if BUFFER is a string, which it isn't, it's a buffer. > Let me know if I misread something. > >> Let me know if I should send this somewhere else; I'm still >> learning mailing list development etiquette. > > This is fine for such a follow up, I think. > > > Best, > > Eshel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-10-08 10:07 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <172812286261.457694.4614807424585269986@vcs3.savannah.gnu.org> [not found] ` <20241005100743.4B6DB1AA78D@vcs3.savannah.gnu.org> 2024-10-06 7:52 ` master 048e72a403e 1/2: Add file/buffer-to-register (Bug#73308) Eshel Yaron 2024-10-06 23:21 ` Barra Ó Catháin via Emacs development discussions. 2024-10-08 9:36 ` Eshel Yaron 2024-10-08 10:07 ` Barra Ó Catháin via Emacs development discussions.
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).