Re: [BUG] Cannot tangle src block in capture buffer [9.7.6]

[Derek Chen-Becker <derek@chen-becker.org>]

[-- Attachment #1: Type: text/plain, Size: 7365 bytes --]

Before I submit the updated patch, does this look like a better approach?

In the test for `org-base-buffer-file-name' I changed it to explicitly find
the current buffer file name and use that:

(ert-deftest test-org-base-buffer-file-name ()

  "Test `org-base-buffer-file-name'."

  (should

   (org-test-with-temp-text-in-file

    "File"

    (let ((base-filename (buffer-file-name)))

      (and

       ;; Confirm that we get the same answer whether we provide the buffer
or use the default
       (equal base-filename (org-base-buffer-file-name))

       (equal base-filename (org-base-buffer-file-name
(current-buffer)))))))
  (should

   (org-test-with-temp-text-in-file

    "File with capture buffer"

    (let ((org-capture-templates '(("t" "Test" entry (here) "* Test
Header\n\n")))
          (base-filename (buffer-file-name)))

      (org-capture nil "t")

      (and

       ;; Confirm that we get the same answer whether we provide the buffer
or use the default
       (equal base-filename (org-base-buffer-file-name))

       (equal base-filename (org-base-buffer-file-name (current-buffer)))

       (equal base-filename (org-base-buffer-file-name (buffer-base-buffer
(current-buffer))))
       ))))

  (should-not

   (org-test-with-temp-text

    "Buffer without file"

    (org-base-buffer-file-name))))


For the tangle test, I also capture the base buffer filename, use an
explicit tangle filename, and ensure that the call to `org-babel-tangle'
returns the expected list of tangled files:

;; 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>"

   (let ((tangle-filename (format "%s.el" (buffer-file-name))))

     (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

               (format "

#+begin_src elisp :tangle \"%s\" :comments org

  (message \"FOO\")

#+end_src" tangle-filename))

              (search-backward "message")

              ;; Confirm that we tangled to the right file

              (equal (org-babel-tangle 4) (list tangle-filename))))

        ;; Clean up the tangled file with the filename from
org-test-with-temp-text-in-file
        (delete-file tangle-filename))))))



On Tue, Jan 14, 2025 at 12:26 PM Derek Chen-Becker <derek@chen-becker.org>
wrote:

> Thanks for the comments! This is my first real foray into non-trivial
> elisp so I really appreciate your patience and help. Let me try and get
> these addressed.
>
> Cheers,
>
> derek
>
> On Tue, Jan 14, 2025 at 10:54 AM Ihor Radchenko <yantar92@posteo.net>
> wrote:
>
>> Derek Chen-Becker <derek@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>
>>
>
>
> --
> +---------------------------------------------------------------+
> | Derek Chen-Becker                                             |
> | GPG Key available at https://keybase.io/dchenbecker and       |
> | https://pgp.mit.edu/pks/lookup?search=derek%40chen-becker.org |
> | Fngrprnt: EB8A 6480 F0A3 C8EB C1E7  7F42 AFC5 AFEE 96E4 6ACC  |
> +---------------------------------------------------------------+
>
>

-- 
+---------------------------------------------------------------+
| Derek Chen-Becker                                             |
| GPG Key available at https://keybase.io/dchenbecker and       |
| https://pgp.mit.edu/pks/lookup?search=derek%40chen-becker.org |
| Fngrprnt: EB8A 6480 F0A3 C8EB C1E7  7F42 AFC5 AFEE 96E4 6ACC  |
+---------------------------------------------------------------+

[-- Attachment #2: Type: text/html, Size: 15922 bytes --]