emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Derek Chen-Becker <derek@chen-becker.org>
To: Ihor Radchenko <yantar92@posteo.net>
Cc: emacs-orgmode@gnu.org
Subject: Re: [BUG] Cannot tangle src block in capture buffer [9.7.6]
Date: Mon, 13 Jan 2025 20:01:14 -0700	[thread overview]
Message-ID: <CAMbmz5mgqVbmg9W-PK03aWuKaO4-O4Y7AMSwoz4CrtxF0ZhdWg@mail.gmail.com> (raw)
In-Reply-To: <87wmey7hmb.fsf@localhost>


[-- Attachment #1.1: Type: text/plain, Size: 1854 bytes --]

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,

Derek


On Mon, Jan 13, 2025 at 10:21 AM Ihor Radchenko <yantar92@posteo.net> wrote:

> Derek Chen-Becker <derek@chen-becker.org> writes:
>
> >> Also, what about other places in the code that use `buffer-file-name'?
> >> We may want to use the new function there as well.
> >
> > Definitely. There are hundreds of places in the code where the same
> pattern
> > applies. Per our previous discussion in the thread, though, I thought it
> > would be better to start small and incrementally work through them. If
> you
> > want me to include more changes,I'm going to need to help sorting all of
> > these out:
> >
> > ❯ rg -q --stats buffer-file-name
> > 185 matches
> > 182 matched lines
>
> It is ok to only limit the current patch to fixing the bug in question.
> We can continue with more changes later, after merging the more limited
> patch.
>
> --
> 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  |
+---------------------------------------------------------------+

[-- Attachment #1.2: Type: text/html, Size: 3920 bytes --]

[-- Attachment #2: 0002-ob-tangle-Ensure-proper-tangling-within-a-capture-bu.patch --]
[-- Type: application/octet-stream, Size: 5453 bytes --]

From 15c324abce9ce404f18af9a02ce57fab621e8424 Mon Sep 17 00:00:00 2001
From: Derek Chen-Becker <github@chen-becker.org>
Date: Mon, 13 Jan 2025 19:54:57 -0700
Subject: [PATCH 2/2] ob-tangle: Ensure proper tangling within a capture buffer

* lisp/org-macs.el (org-base-buffer-file-name): Add a function to properly
resolve the filename for both direct and indirect buffers.
* lisp/ob-tangle.el (org-babel-tangle): Update to utilize the new
org-base-buffer-file-name function.
* testing/lisp/test-ob-tangle.el (ob-tangle/tangle-from-capture-buffer): Add
a unit test to cover the use case of tangling within a capture buffer.
* testing/lisp/test-org-macs.el (test-org-base-buffer-file-name): Add a
unit test to exercise the behavior of the new org-base-buffer-file-name
function, particulary around handling base vis indirect buffers.

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
appears to be a common pitfall across Org, so I've added a helper function
to resolve the filename for both indirect and direct buffers so that it can
be used in other places within the codebase.

TINYCHANGE

Reported-by: Dilip <iDLip@protonmail.com>
Link: https://list.orgmode.org/87msfxd81c.fsf@localhost/T/#t
---
 lisp/ob-tangle.el              |  2 +-
 lisp/org-macs.el               |  9 +++++++++
 testing/lisp/test-ob-tangle.el | 21 +++++++++++++++++++++
 testing/lisp/test-org-macs.el  | 28 ++++++++++++++++++++++++++++
 4 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/lisp/ob-tangle.el b/lisp/ob-tangle.el
index 38cad78ab..64bdced84 100644
--- a/lisp/ob-tangle.el
+++ b/lisp/ob-tangle.el
@@ -269,7 +269,7 @@ matching a regular expression."
 	       (or (cdr (assq :tangle (nth 2 (org-babel-get-src-block-info 'no-eval))))
 		   (user-error "Point is not in a source code block"))))
 	    path-collector
-            (source-file buffer-file-name))
+            (source-file (org-base-buffer-file-name)))
 	(mapc ;; map over file-names
 	 (lambda (by-fn)
 	   (let ((file-name (car by-fn)))
diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index 044c49efb..f2c555e1d 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -1821,6 +1821,15 @@ indirectly called by the latter."
                    (eq (window-frame) (window-frame window))))
       (window--display-buffer buffer window 'reuse alist))))
 
+(defun org-base-buffer-file-name (&optional BUFFER)
+  "Resolve the base file name for the provided BUFFER.
+If BUFFER is not provided, default to the current buffer.  If
+BUFFER does not have a file name associated with it (e.g. a
+transient buffer) then return nil."
+  (if (buffer-base-buffer BUFFER)
+      (buffer-file-name (buffer-base-buffer BUFFER))
+    (buffer-file-name BUFFER)))
+
 (provide 'org-macs)
 
 ;; Local variables:
diff --git a/testing/lisp/test-ob-tangle.el b/testing/lisp/test-ob-tangle.el
index 4c953b15d..0b7428576 100644
--- a/testing/lisp/test-ob-tangle.el
+++ b/testing/lisp/test-ob-tangle.el
@@ -741,6 +741,27 @@ another block
                              bib))))
       (delete-file file))))
 
+;; 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))))))
+
 (provide 'test-ob-tangle)
 
 ;;; test-ob-tangle.el ends here
diff --git a/testing/lisp/test-org-macs.el b/testing/lisp/test-org-macs.el
index 93f00a4c5..3ef62ba84 100644
--- a/testing/lisp/test-org-macs.el
+++ b/testing/lisp/test-org-macs.el
@@ -145,5 +145,33 @@
                      (butlast (decode-time (org-matcher-time "<+14h>"))
                               3))))))
 
+\f
+;;; Buffers
+
+(ert-deftest test-org-base-buffer-file-name ()
+  "Test `org-base-buffer-file-name'."
+  (should
+   (org-test-with-temp-text-in-file
+    "File"
+    (and
+     ;; Confirm that we get the same answer whether we provide the buffer or use the default
+     (equal file (org-base-buffer-file-name))
+     (equal file (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"))))
+      (org-capture nil "t")
+      (and
+       ;; Confirm that we get the same answer whether we provide the buffer or use the default
+       (equal file (org-base-buffer-file-name))
+       (equal file (org-base-buffer-file-name (current-buffer)))
+       (equal file (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))))
+
 (provide 'test-org-macs)
 ;;; test-org-macs.el ends here
-- 
2.43.0


[-- Attachment #3: 0001-testing-README-Add-details-on-use-of-TEST_NO_AUTOCLE.patch --]
[-- Type: application/octet-stream, Size: 1178 bytes --]

From 8550757aa57bd7ebd2103d39e24f6e10bf787b5e Mon Sep 17 00:00:00 2001
From: Derek Chen-Becker <github@chen-becker.org>
Date: Sun, 12 Jan 2025 15:27:59 -0700
Subject: [PATCH 1/2] testing/README: Add details on use of TEST_NO_AUTOCLEAN

* testing/README: Add a section on the use of the TEST_NO_AUTOCLEAN make variable

While doing some testing I needed this functionality to debug a test and
found this variable was helpful. Add a section to the README so that others
may benefit without having to dig through makefiles.

TINYCHANGE
---
 testing/README | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/testing/README b/testing/README
index 1c4dd9e76..72a28d75c 100644
--- a/testing/README
+++ b/testing/README
@@ -86,6 +86,15 @@ Ran 2 tests, 2 results as expected (2017-12-28 15:04:45+0100)
 ...
 #+end_example
 
+** Keep test artifacts
+
+Set the ~TEST_NO_AUTOCLEAN~ variable to leave the test directory
+around for inspection.
+
+#+begin_src sh :dir (expand-file-name "..") :results silent
+make TEST_NO_AUTOCLEAN=1 test-dirty
+#+end_src
+
 * Interactive testing from within Emacs
 
 To run the Org mode test suite from a current Emacs instance simply
-- 
2.43.0


  reply	other threads:[~2025-01-14  3:02 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-26 16:29 [BUG] Cannot tangle src block in capture buffer [9.7.6] Dilip
2024-08-05 14:03 ` Ihor Radchenko
2024-12-16  3:26   ` Derek Chen-Becker
2024-12-16 17:39     ` Ihor Radchenko
2024-12-19 17:56       ` Derek Chen-Becker
2024-12-19 19:17         ` Ihor Radchenko
2024-12-23 23:36           ` Derek Chen-Becker
2024-12-24  9:14             ` Ihor Radchenko
2025-01-10 15:25               ` Derek Chen-Becker
2025-01-11  9:17                 ` Ihor Radchenko
2025-01-12 15:52                   ` Derek Chen-Becker
2025-01-12 16:45                     ` Ihor Radchenko
2025-01-12 22:24                       ` Derek Chen-Becker
2025-01-13 17:23                         ` Ihor Radchenko
2025-01-14  3:01                           ` Derek Chen-Becker [this message]
2025-01-14 17:56                             ` Ihor Radchenko
2025-01-14 19:26                               ` Derek Chen-Becker
2025-01-14 23:10                               ` Michael Heerdegen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMbmz5mgqVbmg9W-PK03aWuKaO4-O4Y7AMSwoz4CrtxF0ZhdWg@mail.gmail.com \
    --to=derek@chen-becker.org \
    --cc=emacs-orgmode@gnu.org \
    --cc=yantar92@posteo.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).