From: Adam Porter <adam@alphapapa.net>
To: Ihor Radchenko <yantar92@posteo.net>
Cc: emacs-orgmode@gnu.org
Subject: Re: [PATCH] * lisp/org.el: (org-get-indirect-buffer) Allow indirect base buffers
Date: Sun, 6 Nov 2022 15:40:15 -0600 [thread overview]
Message-ID: <8b2c1814-2c6f-fdca-8ba7-63c415bfca5e@alphapapa.net> (raw)
In-Reply-To: <87a655lt4x.fsf@localhost>
[-- Attachment #1: Type: text/plain, Size: 2117 bytes --]
Hi Ihor,
On 11/5/22 03:09, Ihor Radchenko wrote:
> Adam Porter <adam@alphapapa.net> writes:
>
>> The attached patch improves the function org-get-indirect-buffer, fixing
>> a bug, clarifying the code, and adding a docstring.
>
> Thanks! I have some comments.
Thanks for your review. I've attached a new patch.
>> +(cl-defun org-get-indirect-buffer (&optional (buffer (current-buffer)) heading)
>> + "Return an indirect buffer based on BUFFER.
>> +If HEADING, prepend it to the name of the new buffer."
>
> Maybe append to the name?
Yes, thanks. In the new patch I took the liberty of improving the
format to make it more distinctive (i.e. it won't resemble a normal
filename).
>
>> + (let* ((base-buffer (or (buffer-base-buffer buffer) buffer))
>> + (suffix-prefix (if heading
>> + (concat heading "-")
>> + ""))
>
> Why not pre-define the whole prefix instead?
> (prefix (format "%s-%s" (buffer-name base-buffer)
> (if heading (concat heading "-") "")))
>
> then, can just say (format "%s%s" prefix n) in the loop.
>
>> + (buffer-name (cl-loop for n from 1 to 100
>
> why to 100? It may fail (even though unlikely) and also unnecessary.
> Can just say for n from 1.
I chose to limit the index because IME it's better to signal an error
than to potentially go into an infinite loop. Although it shouldn't
happen, it could in the case of unforeseen circumstances, one of which I
experienced while writing the patch.
But, even better, the new patch uses the built-in function
generate-new-buffer-name, which handles it for us, and more efficiently.
>> + ;; FIXME: Explain why this `condition-case' is necessary. Why
>> + ;; could an error be signaled with the CLONE argument non-nil,
>> + ;; and why would trying again without CLONE solve the problem?
>> + (error (make-indirect-buffer base-buffer buffer-name)))))
>
> I did not find why in the git logs. It looks like some ancient code. You
> can remove it in a followup patch.
Very well, the new patch omits it.
Thanks,
Adam
[-- Attachment #2: 0001-lisp-org.el-org-get-indirect-buffer-Allow-indirect-b.patch --]
[-- Type: text/x-patch, Size: 2778 bytes --]
From 22e7091b5d6dd8a84446b303a2706ab3d422b52f Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Fri, 4 Nov 2022 14:52:58 -0500
Subject: [PATCH] * lisp/org.el: (org-get-indirect-buffer) Allow indirect base
buffers
Previously, calling this function on an indirect buffer would fail,
preventing the user from making a new indirect buffer based on an
indirect buffer (e.g. imagine the user makes an indirect buffer for a
large subtree, then wants to make another one for a subtree of that).
Now, the base buffer of the buffer is used, when applicable.
Also, the function is partially rewritten to be clearer, and a
docstring is added.
---
lisp/org.el | 35 ++++++++++++++++-------------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/lisp/org.el b/lisp/org.el
index d8708f8f2..6c39f351f 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -6160,25 +6160,22 @@ frame is not changed."
(run-hook-with-args 'org-cycle-hook 'all)
(and (window-live-p cwin) (select-window cwin))))
-(defun org-get-indirect-buffer (&optional buffer heading)
- (setq buffer (or buffer (current-buffer)))
- (let ((n 1) (base (buffer-name buffer)) bname)
- (while (buffer-live-p
- (get-buffer
- (setq bname
- (concat base "-"
- (if heading (concat heading "-" (number-to-string n))
- (number-to-string n))))))
- (setq n (1+ n)))
- (condition-case nil
- (let ((indirect-buffer (make-indirect-buffer buffer bname 'clone)))
- ;; Decouple folding state. We need to do it manually since
- ;; `make-indirect-buffer' does not run
- ;; `clone-indirect-buffer-hook'.
- (org-fold-core-decouple-indirect-buffer-folds)
- ;; Return the buffer.
- indirect-buffer)
- (error (make-indirect-buffer buffer bname)))))
+(cl-defun org-get-indirect-buffer (&optional (buffer (current-buffer)) heading)
+ "Return an indirect buffer based on BUFFER.
+If HEADING, append it to the name of the new buffer."
+ (let* ((base-buffer (or (buffer-base-buffer buffer) buffer))
+ (buffer-name (generate-new-buffer-name
+ (format "%s%s"
+ (buffer-name base-buffer)
+ (if heading
+ (concat "::" heading)
+ ""))))
+ (indirect-buffer (make-indirect-buffer base-buffer buffer-name 'clone)))
+ ;; Decouple folding state. We need to do it manually since
+ ;; `make-indirect-buffer' does not run
+ ;; `clone-indirect-buffer-hook'.
+ (org-fold-core-decouple-indirect-buffer-folds)
+ indirect-buffer))
(defun org-set-frame-title (title)
"Set the title of the current frame to the string TITLE."
--
2.34.0
next prev parent reply other threads:[~2022-11-06 21:41 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-04 19:58 [PATCH] * lisp/org.el: (org-get-indirect-buffer) Allow indirect base buffers Adam Porter
2022-11-05 8:09 ` Ihor Radchenko
2022-11-06 21:40 ` Adam Porter [this message]
2022-11-07 3:17 ` Ihor Radchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8b2c1814-2c6f-fdca-8ba7-63c415bfca5e@alphapapa.net \
--to=adam@alphapapa.net \
--cc=emacs-orgmode@gnu.org \
--cc=yantar92@posteo.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.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).