Adam Porter <a...@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>