Re: [PATCH] * lisp/org.el: (org-get-indirect-buffer) Allow indirect base buffers

[Ihor Radchenko <yantar92@posteo.net>]

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>