[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] * lisp/org.el: (org-get-indirect-buffer) Allow indirect base
From: |
Ihor Radchenko |
Subject: |
Re: [PATCH] * lisp/org.el: (org-get-indirect-buffer) Allow indirect base buffers |
Date: |
Sat, 05 Nov 2022 08:09:34 +0000 |
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>