I'm still sorting out the copyright assignment, but I have another style question: It feels like I should move the `should' clauses closer to the checks so that I get a more concise report when something is wrong:

(ert-deftest test-org-base-buffer-file-name ()
"Test `org-base-buffer-file-name'."
;; Test direct buffer resolution
(org-test-with-temp-text-in-file
"File"
(let ((base-filename (buffer-file-name)))
;; Confirm that we get the same answer whether we provide the buffer or use the default
(should (equal base-filename (org-base-buffer-file-name)))
(should (equal base-filename (org-base-buffer-file-name (current-buffer))))))
;; Test indirect buffer resolution
(org-test-with-temp-text-in-file
"File with indirect buffer"
(let ((base-filename (buffer-file-name))
(base-buffer (current-buffer))
(indirect-test-buffer (make-indirect-buffer (current-buffer) "test")))
(set-buffer indirect-test-buffer)
;; Confirm that we get the same answer for the default, the indirect buffer, and the base buffer
(should (equal base-filename (org-base-buffer-file-name)))
(should (equal base-filename (org-base-buffer-file-name base-buffer)))
(should (equal base-filename (org-base-buffer-file-name indirect-test-buffer)))
(kill-buffer indirect-test-buffer)))
;; Test for a buffer with no associated file
(org-test-with-temp-text
"Buffer without file"
(should-not (org-base-buffer-file-name))))

Is that OK?

Thanks,

Derek

On Thu, Jan 16, 2025 at 12:13 PM Ihor Radchenko <yantar92@posteo.net> wrote:

Derek Chen-Becker <derek@chen-becker.org> writes:

>> The latest version of your patch exceeds 20LOC and cannot be accepted
>> unless you have FSF copyright assignment.
>
> Sorry, that was clear in your previous comment. I am working on the
> assignment form right now but I expect that to be a solved problem and I
> want to continue to contribute to Org mode anyway, so I wanted to
> parallelize that with continuing on the patch. If you would like me to stop
> working on the patch until I have the copyright assignment taken care of, I
> can do that, too.

Great. If you are going to have the copyright assignment, there is no
limit on the patch size. It is just that we will need to wait until you
finalize the assignment before we actually merge it.

But nothing stops you and me from working on the patch before that.

>> I recommend using `org-babel-effective-tangled-filename' instead.
>> This is more future-proof against possible changes in the way file name
>> is computed.
>>
>
> I'm sorry, I don't quite understand. Do you mean I should change the
> function name `org-base-buffer-file-name' to
> `org-babel-effective-tangled-filename' or something else?

Ouch. Never mind. I misread your patch. I thought that you are using
:tangle yes in which case `org-babel-effective-tangle-filename' would be
used to auto-generate the tangle file name. But that's not the case -
you explicitly specify the tangle target.

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