* [PATCH] * lisp/org.el: (org-get-indirect-buffer) Allow indirect base buffers
@ 2022-11-04 19:58 Adam Porter
2022-11-05 8:09 ` Ihor Radchenko
0 siblings, 1 reply; 4+ messages in thread
From: Adam Porter @ 2022-11-04 19:58 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 144 bytes --]
Hi,
The attached patch improves the function org-get-indirect-buffer, fixing
a bug, clarifying the code, and adding a docstring.
Thanks,
Adam
[-- Attachment #2: 0001-lisp-org.el-org-get-indirect-buffer-Allow-indirect-b.patch --]
[-- Type: text/x-patch, Size: 3127 bytes --]
From 8e70024cae3f4569d6a0c86a0e4d8079126fe9e5 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 and a FIXME are added.
---
lisp/org.el | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/lisp/org.el b/lisp/org.el
index d8708f8f2..3b67040f7 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -6160,25 +6160,32 @@ 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)))
+(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."
+ (let* ((base-buffer (or (buffer-base-buffer buffer) buffer))
+ (suffix-prefix (if heading
+ (concat heading "-")
+ ""))
+ (buffer-name (cl-loop for n from 1 to 100
+ for suffix = (format "%s%s" suffix-prefix n)
+ for name = (format "%s-%s"
+ (buffer-name base-buffer)
+ suffix)
+ while (buffer-live-p (get-buffer name))
+ finally return name)))
(condition-case nil
- (let ((indirect-buffer (make-indirect-buffer buffer bname 'clone)))
+ (let ((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)
;; Return the buffer.
indirect-buffer)
- (error (make-indirect-buffer buffer bname)))))
+ ;; 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)))))
(defun org-set-frame-title (title)
"Set the title of the current frame to the string TITLE."
--
2.34.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] * lisp/org.el: (org-get-indirect-buffer) Allow indirect base buffers
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
0 siblings, 1 reply; 4+ messages in thread
From: Ihor Radchenko @ 2022-11-05 8:09 UTC (permalink / raw)
To: Adam Porter; +Cc: emacs-orgmode
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.
> +(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?
> + (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.
> + ;; 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.
--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] * lisp/org.el: (org-get-indirect-buffer) Allow indirect base buffers
2022-11-05 8:09 ` Ihor Radchenko
@ 2022-11-06 21:40 ` Adam Porter
2022-11-07 3:17 ` Ihor Radchenko
0 siblings, 1 reply; 4+ messages in thread
From: Adam Porter @ 2022-11-06 21:40 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
[-- 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-11-07 3:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2022-11-07 3:17 ` Ihor Radchenko
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).