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


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