Hello! I seem to have come across a bug today in org-footnote. I had just learned about the variable org-footnote-auto-adjust and set it to t. Then I tried to test it by invoking org-footnote-new in my Org file in between existing footnotes 2 and 3. N.B., my Footnotes heading, prior to doing above also had a CUSTOM_ID property set: #+begin_src org ,** Footnotes :PROPERTIES: :CUSTOM_ID: footnotes :END: [fn:1] original footnote 1 [fn:2] original footnote 2 [fn:3] original footnote 3 [fn:4] original footnote 4 #+end_src The new footnote seems to get inserted into correct place, however there appears to be a problem if there is a property drawer: #+begin_src org ,** Footnotes [fn:1] original footnote 1 [fn:2] original footnote 2 [fn:3] new footnote :PROPERTIES: :CUSTOM_ID: footnotes :END: [fn:4] original footnote 3 [fn:5] original footnote 4 #+end_src Since I was just studying the org-footnote code anyway, I will attempt to further diagnose the issue, and perhaps even send a patch. As I was filling out bug report I realized I am on slightly dated version of Orgmode. So I went ahead and cloned latest version and did a diff on org-footnote.el between my affected version here locally and latest, and the only change I saw was the copyright date. So with that out of the way, I will start digging and see what I can come up with. Cheers, TRS-80 Emacs : GNU Emacs 26.3 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.20) of 2020-05-16, modified by Debian Package: Org mode version 9.3.6 (9.3.6-23-g01ee25-elpaplus @ /home/user/.emacs.d/elpa/org-plus-contrib-20200309/)
[-- Attachment #1: Type: text/plain, Size: 2181 bytes --] On 2021-01-03 16:27, TRS-80 wrote: > Hello! > > I seem to have come across a bug today in org-footnote. > > I had just learned about the variable org-footnote-auto-adjust and set > it to t. Then I tried to test it by invoking org-footnote-new in my > Org file in between existing footnotes 2 and 3. > > N.B., my Footnotes heading, prior to doing above also had a CUSTOM_ID > property set: > > #+begin_src org > ,** Footnotes > :PROPERTIES: > :CUSTOM_ID: footnotes > :END: > > [fn:1] original footnote 1 > > [fn:2] original footnote 2 > > [fn:3] original footnote 3 > > [fn:4] original footnote 4 > > #+end_src > > The new footnote seems to get inserted into correct place, however > there > appears to be a problem if there is a property drawer: > > #+begin_src org > ,** Footnotes > > [fn:1] original footnote 1 > > [fn:2] original footnote 2 > > [fn:3] new footnote > :PROPERTIES: > :CUSTOM_ID: footnotes > :END: > > [fn:4] original footnote 3 > > [fn:5] original footnote 4 > > #+end_src > > Since I was just studying the org-footnote code anyway, I will attempt > to further diagnose the issue, and perhaps even send a patch. > > As I was filling out bug report I realized I am on slightly dated > version of Orgmode. So I went ahead and cloned latest version and did > a > diff on org-footnote.el between my affected version here locally and > latest, and the only change I saw was the copyright date. > > So with that out of the way, I will start digging and see what I can > come up with. > > Cheers, > TRS-80 > > > Emacs : GNU Emacs 26.3 (build 2, x86_64-pc-linux-gnu, GTK+ Version > 3.24.20) > of 2020-05-16, modified by Debian > Package: Org mode version 9.3.6 (9.3.6-23-g01ee25-elpaplus @ > /home/user/.emacs.d/elpa/org-plus-contrib-20200309/) Attached please find a very simple (one line) patch that I believe should fix this issue. This patch is of course based on latest git (not my personal outdated version) and also maint branch. I think I've got that right? However as it will be my first patch to Orgmode, any feedback would be welcomed. Cheers, TRS-80 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-org-footnote-fix-inserting-new-footnote-mangling-dra.patch --] [-- Type: text/x-diff; name=0001-org-footnote-fix-inserting-new-footnote-mangling-dra.patch, Size: 1077 bytes --] From cf7111a87645262c68214a03ca88f72bb0710049 Mon Sep 17 00:00:00 2001 From: TRS-80 <lists.trs-80@isnotmyreal.name> Date: Sat, 9 Jan 2021 11:50:50 -0500 Subject: [PATCH] org-footnote: fix inserting new footnote mangling drawers * org-footnote.el (org-footnote-create-definition): Replace `forward-line' with `org-end-of-meta-data' to skip over any properties and/or drawers that may be present on the `org-footnote-section' heading (default "Footnotes"). TINYCHANGE --- lisp/org-footnote.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisp/org-footnote.el b/lisp/org-footnote.el index 3d42421e0..47ad4aa04 100644 --- a/lisp/org-footnote.el +++ b/lisp/org-footnote.el @@ -704,7 +704,7 @@ function doesn't move point." (concat "^\\*+[ \t]+" (regexp-quote org-footnote-section) "[ \t]*$") nil t)) (goto-char (match-end 0)) - (forward-line) + (org-end-of-meta-data t) (unless (bolp) (insert "\n"))) (t (org-footnote--clear-footnote-section))) (when (zerop (org-back-over-empty-lines)) (insert "\n")) -- 2.29.2
Thanks for the initial report and the patch. TRS-80 writes: > Attached please find a very simple (one line) patch that I believe > should fix this issue. > > This patch is of course based on latest git (not my personal outdated > version) and also maint branch. I think I've got that right? Looks good to me. > Subject: [PATCH] org-footnote: fix inserting new footnote mangling drawers convention nit: s/fix/Fix/ (no need to resend) > * org-footnote.el (org-footnote-create-definition): Replace > `forward-line' with `org-end-of-meta-data' to skip over any > properties and/or drawers that may be present on the > `org-footnote-section' heading (default "Footnotes"). > > TINYCHANGE > --- > lisp/org-footnote.el | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I'm planning to squash the following test in when applying. Look okay to you? diff --git a/testing/lisp/test-org-footnote.el b/testing/lisp/test-org-footnote.el index eca24d315..50a430785 100644 --- a/testing/lisp/test-org-footnote.el +++ b/testing/lisp/test-org-footnote.el @@ -138,7 +138,20 @@ (ert-deftest test-org-footnote/new () (org-test-with-temp-text "Paragraph<point>\n# Local Variables:\n# foo: t\n# End:" (let ((org-footnote-section "Footnotes")) (org-footnote-new)) - (buffer-string))))) + (buffer-string)))) + (should + (equal "Para[fn:1] +* Footnotes +:properties: +:custom_id: id +:end: + +\[fn:1]" + (org-test-with-temp-text + "Para<point>\n* Footnotes\n:properties:\n:custom_id: id\n:end:" + (let ((org-footnote-section "Footnotes")) + (org-footnote-new)) + (org-trim (buffer-string)))))) (ert-deftest test-org-footnote/delete () "Test `org-footnote-delete' specifications."
On 2021-01-10 19:57, Kyle Meyer wrote: > Thanks for the initial report and the patch. I am very happy to contribute! Thanks for taking it easy on me the first time around. :) > TRS-80 writes: >> Subject: [PATCH] org-footnote: fix inserting new footnote mangling >> drawers > > convention nit: s/fix/Fix/ (no need to resend) Duly noted! > I'm planning to squash the following test in when applying. Look okay > to you? > > > diff --git a/testing/lisp/test-org-footnote.el > b/testing/lisp/test-org-footnote.el > index eca24d315..50a430785 100644 > --- a/testing/lisp/test-org-footnote.el > +++ b/testing/lisp/test-org-footnote.el > @@ -138,7 +138,20 @@ (ert-deftest test-org-footnote/new () > (org-test-with-temp-text > "Paragraph<point>\n# Local Variables:\n# foo: t\n# End:" > (let ((org-footnote-section "Footnotes")) (org-footnote-new)) > - (buffer-string))))) > + (buffer-string)))) > + (should > + (equal "Para[fn:1] > +* Footnotes > +:properties: > +:custom_id: id > +:end: > + > +\[fn:1]" > + (org-test-with-temp-text > + "Para<point>\n* Footnotes\n:properties:\n:custom_id: > id\n:end:" > + (let ((org-footnote-section "Footnotes")) > + (org-footnote-new)) > + (org-trim (buffer-string)))))) > > (ert-deftest test-org-footnote/delete () > "Test `org-footnote-delete' specifications." I must admit that currently I am still unfamiliar with the testing framework(s). It is something I am interested in learning, but haven't gotten around to /yet/. Therefore, hopefully some other set of eyeballs could give that another look? Cheers, TRS-80
TRS-80 writes: > On 2021-01-10 19:57, Kyle Meyer wrote: >> I'm planning to squash the following test in when applying. Look okay >> to you? [...] > I must admit that currently I am still unfamiliar with the testing > framework(s). It is something I am interested in learning, but haven't > gotten around to /yet/. > > Therefore, hopefully some other set of eyeballs could give that another > look? Sure, I'll wait another day or two for any comments on this patch as a whole before applying.
Kyle Meyer writes:
> Sure, I'll wait another day or two for any comments on this patch as a
> whole before applying.
Applied (1806abdc3).