Derek Chen-Becker <de...@chen-becker.org> writes: > Thanks for the help on this! I've reworked the changes into two patches > (one for the README and one for tangle) and I think I've covered your > concerns. I also added a unit test for the org-base-buffer-file-name > function to cover the miss on provided buffers. Please let me know if you > have any questions.
Thanks! Although, I am afraid that the second set of unit tests is a bit much - they no longer fit within TINYCHANGE (15-20 significant LOC). We have a legal limit on how much code we can accept without requiring copyright assignment. See https://orgmode.org/worg/org-contribute.html#copyright More comments below. > * lisp/ob-tangle.el (org-babel-tangle): Update to utilize the new > org-base-buffer-file-name function. Our convention is to quote Elisp symbols in commit messages like `this'. > The previous use of buffer-file-name would fail inside of a capture buffer > because it is indirect and does not have an associated filename. This Another convention is double space between sentences. > +(defun org-base-buffer-file-name (&optional BUFFER) Elisp convention is to use lower case for function arguments. Upper case in the docstring is used to highlight the function arguments; it does not mean that the arguments themselves need to be in upper case. > + (if (buffer-base-buffer BUFFER) > + (buffer-file-name (buffer-base-buffer BUFFER)) > + (buffer-file-name BUFFER))) Nit: Can also use `if-let*' to avoid calling `buffer-base-buffer' twice. > +;; See https://list.orgmode.org/87msfxd81c.fsf@localhost/T/#t > +(ert-deftest ob-tangle/tangle-from-capture-buffer () > + "Test tangling of source blocks from within a capture buffer. > +This is to ensure that we properly resolve the buffer name." > + (org-test-with-temp-text-in-file > + "* Header\n\nCapture after this point:\n<point>" > + (should > + (unwind-protect > + (progn > + (let ((org-capture-templates '(("t" "Test" entry (here) "* Test > Header\n\n")))) > + (org-capture nil "t") > + (goto-char (point-max)) > + (insert " > +#+begin_src elisp :tangle yes :comments org > + (message \"FOO\") > +#+end_src") > + (search-backward "message") > + (org-babel-tangle 4))) > + ;; Clean up the tangled file with the filename from > org-test-with-temp-text-in-file > + (delete-file (format "%s.el" file)))))) This is not the best style. While technically `file' variable will be let-bound within the `org-test-with-temp-text-in-file' body, it is an internal implementation detail that we should not rely upon. Instead, it is a good idea to examine `buffer-file-name' inside the macro body to get the file name. Also, what if "%s.el" file does not exist? Say, tangling does happen, but to a wrong file. Note that `delete-file' does not throw any error when there is nothing to delete. -- Ihor Radchenko // yantar92, Org mode maintainer, 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>
