Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hi, Nicolas Goaziou m...@nicolasgoaziou.fr writes: I think you can push once the issues above are fixed. Thank you for the work. Pushed! However, this raises a question: why are we modifying definition at all? We are only interested in its new label, which we can get without modifying buffer (i.e. if definition is within range, modify it, otherwise, compute new label and store its definition). I fixed this in the train today (going home). I just committed it, though, so I hope I didn't break org (the tests were running, though before all of your re-factoring today which I just saw). Please let me know if I did any git mistakes. Somehow the time got skewed up and it's behind some of your commits in cgit. . . Cheers, Rasmus -- May the Force be with you
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hi, Nicolas Goaziou m...@nicolasgoaziou.fr writes: You can push the patch once this is fixed. Pushed. See general comments in the other mail. One funny thing I observed was that the following test fails, but only when run via make test, not from e.g. my own Emacs or emacs -q and emacs -q -nw. (should (org-test-with-temp-text (format * h1\npoint#+INCLUDE: \%s/examples/include.org::#ah\ org-test-dir) (narrow-to-region (point) (point-max)) (org-export-expand-include-keyword) (eq 1 (org-current-level When running through make test (org-current-level) evaluate to one (before expansion) even when narrowed (should be nil). However, this test passes make test: (should (org-test-with-temp-text-in-file (format * h1\npoint#+INCLUDE: \%s/examples/include.org::#ah\ org-test-dir) (narrow-to-region (point) (point-max)) (org-export-expand-include-keyword) (eq 1 (org-current-level If somebody knows *why* this I would appreciate an explanation. (Can anybody confirm the above on their system?) Thanks, Rasmus -- . . . It begins of course with The Internet. A Net of Peers
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hello, Rasmus ras...@gmx.us writes: Pushed. See general comments in the other mail. Thank you. One funny thing I observed was that the following test fails, but only when run via make test, not from e.g. my own Emacs or emacs -q and emacs -q -nw. I don't see any failing test. When running through make test (org-current-level) evaluate to one (before expansion) even when narrowed (should be nil). Why? `org-current-level' ignores narrowing. Regards, -- Nicolas Goaziou
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hi, Nicolas Goaziou m...@nicolasgoaziou.fr writes: When running through make test (org-current-level) evaluate to one (before expansion) even when narrowed (should be nil). Why? `org-current-level' ignores narrowing. Perhaps something changed recently then. In the version I was testing with, (with-temp-buffer (org-mode) (insert * h1\np1) (goto-char (point-max)) (cons (org-current-level) (progn (narrow-to-region (line-beginning-position) (line-end-position)) (or (org-current-level) it's nil = (1 . it's nil) But now I updated and indeed it returns (1 . 1). —Rasmus -- Tack, ni svenska vakttorn. Med plutonium tvingar vi dansken på knä!
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Nicolas Goaziou m...@nicolasgoaziou.fr writes: One funny thing I observed was that the following test fails, but only when run via make test, not from e.g. my own Emacs or emacs -q and emacs -q -nw. I don't see any failing test. When I use something like the attached patch, make test goes from passing to failing with a fresh Org. Why is it 1 when evaluated from a text-file?. Or perhaps the Magic Pixies are just angry with me. . . —Rasmus PS: I'll fix the test and push it. -- Not everything that goes around comes back around, you know From e1bf99586e8e7484d34a35d24430cf5ab7b66911 Mon Sep 17 00:00:00 2001 From: Rasmus ras...@gmx.us Date: Thu, 25 Dec 2014 02:51:15 +0100 Subject: [PATCH 3/3] make test break --- testing/lisp/test-ox.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/lisp/test-ox.el b/testing/lisp/test-ox.el index 8447875..bb57118 100644 --- a/testing/lisp/test-ox.el +++ b/testing/lisp/test-ox.el @@ -1027,7 +1027,7 @@ Footnotes[fn:2], foot[fn:test], digit only[3], and [fn:inline:anonymous footnote (buffer-string ;; INCLUDE assigns the relative :minlevel conditional on narrowing. (should - (org-test-with-temp-text-in-file + (org-test-with-temp-text (format * h1\npoint#+INCLUDE: \%s/examples/include.org::#ah\ org-test-dir) (narrow-to-region (point) (point-max)) (org-export-expand-include-keyword) -- 2.2.1
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Rasmus ras...@gmx.us writes: That's a nice solution. Implemented in attached patch. Thanks. Two minor comments follow. Should this be added to ORG-NEWS? Is a feature or a bug-fix? Bug-fix I'd say. There was an attempt to do it (see MINLEVEL binding), but it was incorrect. +;; Add :minlevel to all include words that no explicitly have one. Please update this comment, as it is no longer valid (property instead of :minlevel). In particular, it would be nice to describe how we use :org-include-induced-level. (goto-char (point-min)) +(while (re-search-forward include-re nil t) + (add-text-properties (line-beginning-position) (line-end-position) +`(org-include-induced-level + ,(1+ (org-reduced-level (or (org-current-level) 0)) Properties are usually keywords, not plain symbols. Also, for single properties, `put-text-property' is slightly more lightweight. (put-text-property (line-beginning-position) (line-end-position) :org-include-induced-level (1+ (org-reduced-level (or (org-current-level) 0 + (get-text-property (point) 'org-include-induced-level As a consequence: (get-text-property (point) :org-include-induced-level) You can push the patch once this is fixed. Regards,
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Rasmus ras...@gmx.us writes: Thanks for the comments. I managed to make the patch less complicated. Thanks. Another round of comments follows. I did not know markers but they seem perfect in this case. The manual mentions setting markers to nil after use. I guess it's not necessary here since they are in a (let ⋯)? It is. Binding between the symbol and the marker disappears with the `let', but the marker still exists. It cannot be GC'ed unless it is set to nil. It is not terribly important here as we're working in a temporary buffer anyway, so the marker will ultimately disappear at the end of the export process. However, it's a good habit to have. + (goto-char (point-min)) + (while (re-search-forward org-footnote-re nil t) + (let* ((reference (org-element-context)) +(type (org-element-type reference)) +(footnote-type (org-element-property :type reference)) +(label (org-element-property :label reference))) + (when (eq type 'footnote-reference) The order is confusing here. First you check if you're really at a footnote reference, then you bind LABEL and FOOTNOTE-TYPE. Actually, the latter doesn't even need to bound since you use it only once. I check if I'm at a footnote reference 'cause I never want to deal with a footnote-*definition* directly. AFAIK org-footnote-re matches both. It seems a footnote-reference can never be at the beginning of line, but I would still prefer a more explicit test through org-element. I wasn't clear. The check is important. The minor issue is that you bind LABEL and FOOTNOTE-TYPE too early, before making sure you are at a footnote reference. It would be more logical to do (let ((object (org-element-context))) (when (eq (org-element-type object) 'footnote-reference) (let ((footnote-type (org-element-property :type object)) (label (org-element-property :label object))) ...))) The only bug *I'm aware of* is that it will pick up the wrong new-label for footnote for something like [fn:ref with space]. But this is anyway not a proper label, I think. Is that OK? [fn: ref with space] * test-ox.el (test-org-export/expand-include): Add test for INCLUDE with :lines and footnotes. Line is too long. + (when (and (not included) ( (hash-table-count footnotes) 0)) + (org-with-wide-buffer + (goto-char (point-max)) + (unless (bolp) (insert \n)) + (maphash (lambda (ref def) (insert (format [%s] %s\n ref def))) + footnotes))) (unless included ...) is sufficient. No need to check for table emptiness (although it doesn't cost much): maphash will simply do nothing. If (unless (bolp) (insert \n)) bothers you, you can drop it and use (lambda (ref def) (insert (format \n[%s] %s\n ref def))) in the maphash instead. + (let ((marker-min (point-min-marker)) This marker is not necessary since you're not going to add contents before (point-min) anyway. A plain number is enough. + (marker-max (point-max-marker))) As explained above, don't forget (set-marker marker-max nil) at the end of the `let'. + (goto-char (point-min)) + (while (re-search-forward org-footnote-re nil t) + (let* ((reference (org-element-context)) + (label (org-element-property :label reference)) + (digit-label (and label (org-string-match-p \\`[0-9]+\\' label + (when (eq (org-element-type reference) 'footnote-reference) See above about order of bindings. + (goto-char (1+ (org-element-property :begin reference))) + (when label + (let ((new-label +(buffer-substring-no-properties + (point) + (progn (if digit-label (insert (format fn:%d- id)) + (forward-char 3) + (insert (format %d- id))) +(1- (search-forward ])) + (unless (eq (org-element-property :type reference) 'inline) A comment about what we're going to do would be nice. + (org-with-wide-buffer + (let* ((definition (org-footnote-get-definition label)) + (beginning (set-marker (make-marker) (nth 1 definition Nitpick: (beginning (copy-marker (nth 1 definition))) +(goto-char (1+ beginning)) +(if digit-label (insert (format fn:%d- id)) + (forward-char 3) + (insert (format %d- id))) +(when (or ( beginning marker-min) ( beginning marker-max)) + (puthash new-label (nth 3 definition) + footnotes (set-marker beginning nil) Regards,
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hi,
Thanks again!
Nicolas Goaziou m...@nicolasgoaziou.fr writes:
I did not know markers but they seem perfect in this case. The manual
mentions setting markers to nil after use. I guess it's not necessary
here since they are in a (let ⋯)?
It is. Binding between the symbol and the marker disappears with the
`let', but the marker still exists. It cannot be GC'ed unless it is set
to nil.
It is not terribly important here as we're working in a temporary buffer
anyway, so the marker will ultimately disappear at the end of the export
process. However, it's a good habit to have.
Thanks for the explanation.
I check if I'm at a footnote reference 'cause I never want to deal with a
footnote-*definition* directly. AFAIK org-footnote-re matches both. It
seems a footnote-reference can never be at the beginning of line, but I
would still prefer a more explicit test through org-element.
I wasn't clear. The check is important. The minor issue is that you bind
LABEL and FOOTNOTE-TYPE too early, before making sure you are at
a footnote reference. It would be more logical to do
(let ((object (org-element-context)))
(when (eq (org-element-type object) 'footnote-reference)
(let ((footnote-type (org-element-property :type object))
(label (org-element-property :label object)))
...)))
I see. I disagree that it's more since it's directly inside a loop over
org-footnote-re. So if we are not at a footnote-{reference,definition}
it's probably a bug in the regexp.
The only bug *I'm aware of* is that it will pick up the wrong new-label
for footnote for something like [fn:ref with space]. But this is anyway
not a proper label, I think. Is that OK?
[fn: ref with space]
While your comment excels in preciseness the terseness makes it hard to
appreciate its depth. In my org-installation [fn: ref with space] is
not a valid footnote.
+ (when (and (not included) ( (hash-table-count footnotes) 0))
+(org-with-wide-buffer
+ (goto-char (point-max))
+ (unless (bolp) (insert \n))
+ (maphash (lambda (ref def) (insert (format [%s] %s\n ref
def)))
+ footnotes)))
(unless included ...)
is sufficient. No need to check for table emptiness (although it doesn't
cost much): maphash will simply do nothing.
Thanks. It's not necessary anymore.
+ (let ((marker-min (point-min-marker))
This marker is not necessary since you're not going to add contents
before (point-min) anyway. A plain number is enough.
I might if I include *Bar here:
* Foo
[1] foo
* Bar
Baz[1]
+(goto-char (point-min))
+(while (re-search-forward org-footnote-re nil t)
+ (let* ((reference (org-element-context))
+ (label (org-element-property :label reference))
+ (digit-label (and label (org-string-match-p \\`[0-9]+\\'
label
+(when (eq (org-element-type reference) 'footnote-reference)
See above about order of bindings.
Adopted and commented on above.
+ (goto-char (1+ (org-element-property :begin reference)))
+ (when label
+(let ((new-label
+ (buffer-substring-no-properties
+(point)
+(progn (if digit-label (insert (format fn:%d- id))
+ (forward-char 3)
+ (insert (format %d- id)))
+ (1- (search-forward ]))
+ (unless (eq (org-element-property :type reference) 'inline)
A comment about what we're going to do would be nice.
Comments are added.
—Rasmus
--
I feel emotional landscapes they puzzle me
From d1e02f19e20341a650164f7bfb9aea97465225e1 Mon Sep 17 00:00:00 2001
From: rasmus ras...@gmx.us
Date: Tue, 9 Dec 2014 12:40:52 +0100
Subject: [PATCH 1/2] ox.el: Fix footnote-bug in #+INCLUDE-keyword
* ox.el (org-export--prepare-file-contents): Preserve footnotes
when using the LINES argument. New optional argument FOOTNOTES.
(org-export-expand-include-keyword): New optional argument
FOOTNOTES.
* test-ox.el (test-org-export/expand-include): Add test for INCLUDE
with :lines and footnotes.
---
lisp/ox.el | 94 +++--
testing/lisp/test-ox.el | 32 +++--
2 files changed, 98 insertions(+), 28 deletions(-)
diff --git a/lisp/ox.el b/lisp/ox.el
index 9d9e794..6f3888b 100644
--- a/lisp/ox.el
+++ b/lisp/ox.el
@@ -3052,17 +3052,20 @@ locally for the subtree through node properties.
(car key)
(if (org-string-nw-p val) (format %s val)
-(defun org-export-expand-include-keyword (optional included dir)
+(defun org-export-expand-include-keyword (optional included dir footnotes)
Expand every include keyword in buffer.
Optional argument INCLUDED is a list of included file names along
with their line restriction, when appropriate. It is used to
avoid infinite recursion.
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Thanks for the update.
Rasmus ras...@gmx.us writes:
I see. I disagree that it's more since it's directly inside a loop over
org-footnote-re. So if we are not at a footnote-{reference,definition}
it's probably a bug in the regexp.
Pleonasm.
Note that the regexp can match even if not at a footnote reference:
#+begin_example
int x = k[1]
#+end_example
[fn: ref with space]
While your comment excels in preciseness the terseness makes it hard to
appreciate its depth. In my org-installation [fn: ref with space] is
not a valid footnote.
Actually, I wanted to say it wasn't valid, then changed my mind, and
eventually forgot to remove it from my mail.
In a thousand years, scholars might debate over the secret meaning
behind these symbols.
This marker is not necessary since you're not going to add contents
before (point-min) anyway. A plain number is enough.
I might if I include *Bar here:
* Foo
[1] foo
* Bar
Baz[1]
I'm not sure to understand. Would you mind elaborating?
+ (unless included
+ (org-with-wide-buffer
+ (goto-char (point-max))
+ (unless (bolp) (insert \n))
+ (maphash (lambda (ref def) (insert (format [%s] %s\n ref
def)))
+ footnotes)))
The more I look at it, the more I'm seduced by
(unless included
(org-with-wide-buffer
(goto-char (point-max))
(maphash (lambda (ref def) (insert (format \n[%s] %s\n ref def)))
footnotes)))
I'm really nitpicking, tho.
;; Append ID to all footnote references and definitions, so they
;; become file specific and cannot collide with footnotes in other
;; included files.
+;; Further, collect relevant footnotes outside of LINES.
You can include it in the previous paragraph, or insert a blank comment
line, as it wouldn't survive a M-q.
+ (goto-char (1+ (org-element-property :begin reference)))
+ (when label
Shouldn't these two lines be inverted?
+ (let* ((definition (org-footnote-get-definition label))
+ (beginning (copy-marker (nth 1 definition
Actually, BEGINNING doesn't need to be a marker either: you always
modify buffer after it.
Regards,
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hi, I fixed the nitpicks, but no major changes. Nicolas Goaziou m...@nicolasgoaziou.fr writes: Pleonasm. Note that the regexp can match even if not at a footnote reference [...]: Fair enough. In a thousand years, scholars might debate over the secret meaning behind these symbols. Let's hope so! In the meantime i will let the user shoot herself in the foot at specify wrong labels. This marker is not necessary since you're not going to add contents before (point-min) anyway. A plain number is enough. I might if I include *Bar here: * Foo [1] foo * Bar Baz[1] I'm not sure to understand. Would you mind elaborating? If I have #+INCLUDE: example-above.org::*Bar then point-min of the include area will be pushed forward by four since the definition of [1] is changed to fn:1-1 or something like that. So min-marker should be a marker. Or I'm misunderstanding something. The more I look at it, the more I'm seduced by (unless included (org-with-wide-buffer (goto-char (point-max)) (maphash (lambda (ref def) (insert (format \n[%s] %s\n ref def))) footnotes))) I'm really nitpicking, tho. Since it's soon Christmas, so I could perhaps accommodate. ;; Append ID to all footnote references and definitions, so they ;; become file specific and cannot collide with footnotes in other ;; included files. +;; Further, collect relevant footnotes outside of LINES. You can include it in the previous paragraph, or insert a blank comment line, as it wouldn't survive a M-q. Let's M-q it then. +(goto-char (1+ (org-element-property :begin reference))) +(when label Shouldn't these two lines be inverted? Sure that's prettier. Cheers, Rasmus -- ツ From 2382ee420c97a801560dff3e9bea343bca83dc13 Mon Sep 17 00:00:00 2001 From: rasmus ras...@gmx.us Date: Tue, 9 Dec 2014 12:40:52 +0100 Subject: [PATCH 1/2] ox.el: Fix footnote-bug in #+INCLUDE-keyword * ox.el (org-export--prepare-file-contents): Preserve footnotes when using the LINES argument. New optional argument FOOTNOTES. (org-export-expand-include-keyword): New optional argument FOOTNOTES. * test-ox.el (test-org-export/expand-include): Add test for INCLUDE with :lines and footnotes. --- lisp/ox.el | 94 +++-- testing/lisp/test-ox.el | 32 +++-- 2 files changed, 97 insertions(+), 29 deletions(-) diff --git a/lisp/ox.el b/lisp/ox.el index 9d9e794..60f06cc 100644 --- a/lisp/ox.el +++ b/lisp/ox.el @@ -3052,17 +3052,20 @@ locally for the subtree through node properties. (car key) (if (org-string-nw-p val) (format %s val) -(defun org-export-expand-include-keyword (optional included dir) +(defun org-export-expand-include-keyword (optional included dir footnotes) Expand every include keyword in buffer. Optional argument INCLUDED is a list of included file names along with their line restriction, when appropriate. It is used to avoid infinite recursion. Optional argument DIR is the current working directory. It is used to properly resolve relative -paths. +paths. Optional argument FOOTNOTES is a hash-table used for +storing and resolving footnotes. It is created automatically. (let ((case-fold-search t) (file-prefix (make-hash-table :test #'equal)) - (current-prefix 0)) + (current-prefix 0) + (footnotes (or footnotes (make-hash-table :test #'equal (goto-char (point-min)) +;; Expand INCLUDE keywords. (while (re-search-forward ^[ \t]*#\\+INCLUDE: nil t) (let ((element (save-match-data (org-element-at-point (when (eq (org-element-type element) 'keyword) @@ -3155,15 +3158,23 @@ paths. file location only-contents lines) lines))) (org-mode) - (insert - (org-export--prepare-file-contents - file lines ind minlevel - (or (gethash file file-prefix) - (puthash file (incf current-prefix) file-prefix) + (insert (org-export--prepare-file-contents + file lines ind minlevel + (or (gethash file file-prefix) + (puthash file (incf current-prefix) file-prefix)) + footnotes))) (org-export-expand-include-keyword (cons (list file lines) included) - (file-name-directory file)) - (buffer-string) + (file-name-directory file) + footnotes) + (buffer-string) + ;; Expand footnotes after all files have been + ;; included. Footnotes are stored at end of buffer. + (unless included + (org-with-wide-buffer + (goto-char (point-max)) + (maphash (lambda (ref def) (insert (format \n[%s] %s\n ref def))) + footnotes))) (defun org-export--inclusion-absolute-lines (file location only-contents lines) Resolve absolute lines for an included file with file-link. @@ -3227,8 +3238,8 @@ Return a string of lines to be included in the format expected by
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Rasmus ras...@gmx.us writes: * Foo [1] foo * Bar Baz[1] I'm not sure to understand. Would you mind elaborating? If I have #+INCLUDE: example-above.org::*Bar then point-min of the include area will be pushed forward by four since the definition of [1] is changed to fn:1-1 or something like that. So min-marker should be a marker. Or I'm misunderstanding something. No, you're right. However, this raises a question: why are we modifying definition at all? We are only interested in its new label, which we can get without modifying buffer (i.e. if definition is within range, modify it, otherwise, compute new label and store its definition). Anyway, it doesn't matter much. The marker is fine, indeed. Since it's soon Christmas, so I could perhaps accommodate. I ensure you I have mostly been kind all year long. + (org-with-wide-buffer + (let* ((definition (org-footnote-get-definition label)) + (beginning (line-beginning-position))) There's one potential problem here: `org-footnote-get-definition' may return a nil value if there is no matching definition for label. Maybe throw an error? Also, BEGINNING should refer to (nth 1 definition) since you're not using `org-footnote-goto-definition' and therefore, not moving point. I think you can push once the issues above are fixed. Thank you for the work. Regards,
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hi, Nicolas Goaziou m...@nicolasgoaziou.fr writes: Rasmus ras...@gmx.us writes: * Foo [1] foo * Bar Baz[1] I'm not sure to understand. Would you mind elaborating? If I have #+INCLUDE: example-above.org::*Bar then point-min of the include area will be pushed forward by four since the definition of [1] is changed to fn:1-1 or something like that. So min-marker should be a marker. Or I'm misunderstanding something. No, you're right. However, this raises a question: why are we modifying definition at all? We are only interested in its new label, which we can get without modifying buffer (i.e. if definition is within range, modify it, otherwise, compute new label and store its definition). We modify buffer because that's what we want to do when including whole files. The routines /could/ be split up, I just deemed it not worth the trouble. Operations on the table are of course limited to when it's needed. Buffer-editing is not. It's simple to wrap it in an if-statement if you think it's worth it, e.g. performance-wise. I'd only need to move the catching of new-label back to the footnote-reference. +(org-with-wide-buffer + (let* ((definition (org-footnote-get-definition label)) +(beginning (line-beginning-position))) There's one potential problem here: `org-footnote-get-definition' may return a nil value if there is no matching definition for label. Maybe throw an error? Ox already throws an error when a footnote is not found in org-export-get-footnote-definition which is why I didn't add this. But I guess it would be friendly to add another error here stating which *file* is missing a footnote and I added this now. Also, BEGINNING should refer to (nth 1 definition) since you're not using `org-footnote-goto-definition' and therefore, not moving point. Indeed. Thanks. I think you can push once the issues above are fixed. Thank you for the work. Cool. I think the #+INCLUDE-keyword is quite a bit better in 8.3 now and. Thanks for all the help on this series of patches (I think four in total)! —Rasmus -- However beautiful the theory, you should occasionally look at the evidence
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Rasmus ras...@gmx.us writes: Thanks for the notes. Hopefully patch one if good now. Thanks. Some comments follow. * ox.el (org-export--prepare-file-contents): Preserve footnotes when using the LINES argument. New optional argument FOOTNOTES. (org-export-expand-include-keyword): New optional argument FOOTNOTES. * test-ox.el: Add test for INCLUDE with :lines and footnotes. Please name the test modified. + (include-re ^[ \t]*#\\+INCLUDE:)) Why do you need it since you only use this regexp once in the function? +;; Expand footnotes after all files have been +;; included. Footnotes are stored in an +;; `org-footnote-section' if that variable is +;; non-nil. Otherwise they are stored close to the definition. Don't bother about `org-footnote-section'. Even if the variable is non-nil, footnote definitions are allowed everywhere. So in any case, the very end of the master document is an appropriate location. Also, inserting definitions close to the reference is wrong, as explained in this thread (it could break structure around reference, e.g., a plain list). + (when (and (not included) ( (hash-table-count footnotes) 0)) + (org-with-wide-buffer + (goto-char (point-min)) + (if org-footnote-section + (progn +(or (search-forward-regexp + (concat ^\\*[ \t]+ + (regexp-quote org-footnote-section) + [ \t]*$) + nil t) +(and + (goto-char (point-max)) + (insert (format * %s org-footnote-section +(insert \n) +(maphash (lambda (ref def) + (insert (format [%s] %s ref def) \n)) + footnotes)) +;; `org-footnote-section' is nil. Insert definitions close to references. +(maphash (lambda (ref def) + (save-excursion + (search-forward (format [%s] ref)) + (org-footnote-create-definition ref) + (insert def \n))) + footnotes IOW, I think (org-with-wide-buffer (goto-char (point-max)) (unless (bolp) (insert \n)) (maphash (lambda (label definition) (insert [ label ] definition \n)) footnotes)) is enough. + (let* ((lines (and lines (split-string lines -))) + (lbeg (and lines (string-to-number (car lines + (lend (and lines (string-to-number (cadr lines) This is not needed. `point-min' and `point-max' refer to the boundaries of the area to be included. It avoids relying on the expensive `line-number-at-pos' function later. Moreover, I suggest store (point-max) as a marker since you are going to modify contents of the buffer (e.g., adding ID to labels). + (goto-char (point-min)) + (while (re-search-forward org-footnote-re nil t) + (let* ((reference (org-element-context)) + (type (org-element-type reference)) + (footnote-type (org-element-property :type reference)) + (label (org-element-property :label reference))) + (when (eq type 'footnote-reference) The order is confusing here. First you check if you're really at a footnote reference, then you bind LABEL and FOOTNOTE-TYPE. Actually, the latter doesn't even need to bound since you use it only once. + (goto-char (org-element-property :begin reference)) + (when label + (forward-char 4) + (insert (format %d- id)) This looks wrong. Labels can be 1, fn:label or even fn: in anonymous footnotes. You need to check if label matches \\`[0-9]+\\', in which case prepending fn: is also necessary. + (and (not (eq footnote-type 'inline)) (unless (eq footnote-type 'inline) ...) or (when (eq (org-element-property :type reference) 'standard) ...) + (org-with-wide-buffer + (org-footnote-goto-definition label) + (beginning-of-line) + (org-skip-whitespace) Footnote definitions start at column 0. Skipping white spaces is not needed. + (forward-char 4) + (insert (format %d- id)) Dangerous. See above. + (let ((definition (org-element-context))) + (when (and lines +(or ( lend (line-number-at-pos + (org-element-property + :contents-begin definition))) +( lbeg (line-number-at-pos + (org-element-property +
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Rasmus ras...@gmx.us writes: Here's a new version of the second patch with tests. Thanks. Some comments follow. The recognition regexp is still not great, but the idea of the regexp is to only act on includes where there's no :minlevel already and no plain words (most obviously src and example, but any block really) when disregarding :key value pairs. This is not necessary. Even if :minlevel is used on these include keywords, its value is ignored when inserting contents of the file. * ox.el (org-export-expand-include-keyword): Guess :minlevel if missing and relevant. * test-ox.el: Tests for automatic :minlevel. Pleas name test modified. +;; Add :minlevel to all include words that no explicitly have one. +(save-excursion No need for that. + (while (re-search-forward + (concat include-re [ \t]* \\(?:\.+?\\\|[^ \t]+\\)[ \t]* \\(.*\\)$) + nil t) (re-search-forward include-re nil t) is enough. + (let ((matched (match-string 1))) + (unless (or (string-match-p :minlevel matched) + ;; matched a normal word + (string-match \\(?:^\\|[ \t]+\\)\\(\\w\\) + (replace-regexp-in-string \\(^\\|[ t]+\\):\\w+[ \t]+\\w+ + matched))) (unless (search-forward :minlevel (line-end-position) t) ...) + (goto-char (line-end-position)) aka (end-of-line) + (insert (format :minlevel %d + (1+ (org-with-wide-buffer + (if (search-backward-regexp org-heading-regexp nil t) + (length (match-string 1)) +0) (insert (format :minlevel %d (1+ (org-outline-level Regards,
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hi, Thanks for the comments. I've attached a new version. Nicolas Goaziou m...@nicolasgoaziou.fr writes: The recognition regexp is still not great, but the idea of the regexp is to only act on includes where there's no :minlevel already and no plain words (most obviously src and example, but any block really) when disregarding :key value pairs. This is not necessary. Even if :minlevel is used on these include keywords, its value is ignored when inserting contents of the file. It's not neural to the org export buffer, though it's probably neutral to the *exported* document. E.g. (org-test-with-temp-text (format #+INCLUDE: \%s/examples/include2.org\ src emacs-lisp org-test-dir) (org-export-expand-include-keyword) (buffer-string)) = #+BEGIN_src emacs-lisp :minlevel 1 Success! #+END_src Which is turn is passed to babel (which also seems to ignoring it). I cannot easily condition on src (without using the previous complex regexp) since the file ~/my src folder/FILE is not totally unlikely... If you are happy with the alteration, which might only be manifested in slightly uglier tests then we can go with the simpler solution. Note, for blocks the minlevel is not inserted: (org-test-with-temp-text (format #+INCLUDE: \%s/examples/include2.org\ foo org-test-dir) (org-export-expand-include-keyword) (buffer-string)) = #+BEGIN_foo Success! #+END_foo + (while (re-search-forward + (concat include-re [ \t]* \\(?:\.+?\\\|[^ \t]+\\)[ \t]* \\(.*\\)$) + nil t) (re-search-forward include-re nil t) is enough. The regexp was motivated by the above concerns. +(insert (format :minlevel %d +(1+ (org-with-wide-buffer + (if (search-backward-regexp org-heading-regexp nil t) + (length (match-string 1)) + 0) (insert (format :minlevel %d (1+ (org-outline-level But this if the buffer is narrowed this would not work correctly in getting the right level. But it probably does not matter for export. See this test for my initial reasoning: (should (org-test-with-temp-text (format * h1\npoint#+INCLUDE: \%s/examples/include.org::#ah\ org-test-dir) (org-narrow-to-element) (org-export-expand-include-keyword) (goto-char (point-min)) (eq 2 (org-element-property :level (org-element-at-point) This is changed now. —Rasmus -- Enough with the bla bla! From b7112471b3e4b5334d98caf528e1e687232dee2f Mon Sep 17 00:00:00 2001 From: Rasmus ras...@gmx.us Date: Thu, 18 Dec 2014 16:48:49 +0100 Subject: [PATCH 2/2] ox.el: Guess the :minlevel for INCLUDE-keywords * ox.el (org-export-expand-include-keyword): Guess :minlevel if missing and relevant. * test-ox.el (org-export-expand-include-keyword): Tests for automatic :minlevel. --- lisp/ox.el | 6 ++ testing/lisp/test-ox.el | 41 +++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/lisp/ox.el b/lisp/ox.el index 99c4e9b..b65cea0 100644 --- a/lisp/ox.el +++ b/lisp/ox.el @@ -3065,8 +3065,14 @@ storing and resolving footnotes. It is created automatically. (current-prefix 0) (footnotes (or footnotes (make-hash-table :test #'equal))) (include-re ^[ \t]*#\\+INCLUDE:)) +;; Add :minlevel to all include words that no explicitly have one. (goto-char (point-min)) +(while (re-search-forward include-re nil t) + (unless (search-forward-regexp [ \t]+:minlevel\\ (line-end-position) t) + (end-of-line) + (insert (format :minlevel %d (1+ (org-outline-level)) ;; Expand INCLUDE keywords. +(goto-char (point-min)) (while (re-search-forward include-re nil t) (let ((element (save-match-data (org-element-at-point (when (eq (org-element-type element) 'keyword) diff --git a/testing/lisp/test-ox.el b/testing/lisp/test-ox.el index 140c0a8..794de1f 100644 --- a/testing/lisp/test-ox.el +++ b/testing/lisp/test-ox.el @@ -854,7 +854,7 @@ text ;; Inclusion within a src-block. (should (equal -#+BEGIN_SRC emacs-lisp\n(+ 2 1)\n#+END_SRC\n +#+BEGIN_SRC emacs-lisp :minlevel 1\n(+ 2 1)\n#+END_SRC\n (org-test-with-temp-text (format #+INCLUDE: \%s/examples/include.org\ :lines \4-5\ SRC emacs-lisp @@ -1045,7 +1045,44 @@ baz (org-test-with-temp-text (format #+INCLUDE: \%s/examples/include.org::#dh\ :only-contents t org-test-dir) (org-export-expand-include-keyword) - (buffer-string) + (buffer-string + ;; Adjacent INCLUDE-keywords should have the same :minlevel if unspecified. + (should + (org-every (lambda (level) (zerop (1- level))) + (org-test-with-temp-text + (concat + (format #+INCLUDE: \%s/examples/include.org::#ah\\n org-test-dir) + (format #+INCLUDE: \%s/examples/include.org::*Heading\ org-test-dir)) + (org-export-expand-include-keyword)
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Rasmus ras...@gmx.us writes: Thanks for the comments. I've attached a new version. Thanks. Nicolas Goaziou m...@nicolasgoaziou.fr writes: This is not necessary. Even if :minlevel is used on these include keywords, its value is ignored when inserting contents of the file. It's not neural to the org export buffer, though it's probably neutral to the *exported* document. E.g. (org-test-with-temp-text (format #+INCLUDE: \%s/examples/include2.org\ src emacs-lisp org-test-dir) (org-export-expand-include-keyword) (buffer-string)) = #+BEGIN_src emacs-lisp :minlevel 1 Success! #+END_src Which is turn is passed to babel (which also seems to ignoring it). I cannot easily condition on src (without using the previous complex regexp) since the file ~/my src folder/FILE is not totally unlikely... Indeed. This is not pretty. I forgot about that feature. I suggest to do it differently, then. We first process every include keyword in the document, but simply add a text property (e.g. `org-include-level') over them specifying (1+ (org-reduced-level (or (org-current-level) 0))) as their value. Later, instead of (minlevel (and (not env) (if (string-match :minlevel +\\([0-9]+\\) value) (prog1 (string-to-number (match-string 1 value)) (setq value (replace-match nil nil value))) (let ((cur (org-current-level))) (if cur (1+ (org-reduced-level cur)) 1) we can use (minlevel (and (not env) (if (string-match :minlevel +\\([0-9]+\\) value) (prog1 (string-to-number (match-string 1 value)) (setq value (replace-match nil nil value))) (get-text-property (point) 'org-include-level Include lines are not modified and this variable only applies to Org documents. WDYT? Regards,
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Nicolas Goaziou m...@nicolasgoaziou.fr writes: I suggest to do it differently, then. We first process every include keyword in the document, but simply add a text property (e.g. `org-include-level') over them specifying [...] Include lines are not modified and this variable only applies to Org documents. WDYT? That's a nice solution. Implemented in attached patch. Should this be added to ORG-NEWS? Is a feature or a bug-fix? Thanks, Rasmus -- Vote for proprietary math! From 1fa88054255e66922ea9e2cd61310461901ac6ee Mon Sep 17 00:00:00 2001 From: Rasmus ras...@gmx.us Date: Thu, 18 Dec 2014 16:48:49 +0100 Subject: [PATCH 2/2] ox.el: Guess the :minlevel for INCLUDE-keywords * ox.el (org-export-expand-include-keyword): Guess :minlevel for included Org documents if missing. * test-ox.el (org-export-expand-include-keyword): Tests for automatic :minlevel. --- lisp/ox.el | 14 ++ testing/lisp/test-ox.el | 39 ++- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/lisp/ox.el b/lisp/ox.el index 11b9a29..11426fb 100644 --- a/lisp/ox.el +++ b/lisp/ox.el @@ -3063,10 +3063,17 @@ storing and resolving footnotes. It is created automatically. (let ((case-fold-search t) (file-prefix (make-hash-table :test #'equal)) (current-prefix 0) - (footnotes (or footnotes (make-hash-table :test #'equal + (footnotes (or footnotes (make-hash-table :test #'equal))) + (include-re ^[ \t]*#\\+INCLUDE:)) +;; Add :minlevel to all include words that no explicitly have one. (goto-char (point-min)) +(while (re-search-forward include-re nil t) + (add-text-properties (line-beginning-position) (line-end-position) + `(org-include-induced-level + ,(1+ (org-reduced-level (or (org-current-level) 0)) ;; Expand INCLUDE keywords. -(while (re-search-forward ^[ \t]*#\\+INCLUDE: nil t) +(goto-char (point-min)) +(while (re-search-forward include-re nil t) (let ((element (save-match-data (org-element-at-point (when (eq (org-element-type element) 'keyword) (beginning-of-line) @@ -3111,8 +3118,7 @@ storing and resolving footnotes. It is created automatically. (if (string-match :minlevel +\\([0-9]+\\) value) (prog1 (string-to-number (match-string 1 value)) (setq value (replace-match nil nil value))) - (let ((cur (org-current-level))) - (if cur (1+ (org-reduced-level cur)) 1) + (get-text-property (point) 'org-include-induced-level (src-args (and (eq env 'literal) (match-string 1 value))) (block (and (string-match (\\S-+\\)\\ value) diff --git a/testing/lisp/test-ox.el b/testing/lisp/test-ox.el index 37e2e23..91f9eab 100644 --- a/testing/lisp/test-ox.el +++ b/testing/lisp/test-ox.el @@ -1003,7 +1003,44 @@ Footnotes[fn:2], foot[fn:test], digit only[3], and [fn:inline:anonymous footnote (org-test-with-temp-text (format #+INCLUDE: \%s/examples/include.org::#dh\ :only-contents t org-test-dir) (org-export-expand-include-keyword) - (buffer-string) + (buffer-string + ;; Adjacent INCLUDE-keywords should have the same :minlevel if unspecified. + (should + (org-every (lambda (level) (zerop (1- level))) + (org-test-with-temp-text + (concat + (format #+INCLUDE: \%s/examples/include.org::#ah\\n org-test-dir) + (format #+INCLUDE: \%s/examples/include.org::*Heading\ org-test-dir)) + (org-export-expand-include-keyword) + (org-element-map (org-element-parse-buffer) 'headline + (lambda (head) (org-element-property :level head)) + ;; INCLUDE does not insert induced :minlevel for src-blocks. + (should-not + (equal +(org-test-with-temp-text + (format #+INCLUDE: \%s/examples/include2.org\ src emacs-lisp org-test-dir) + (org-export-expand-include-keyword) + (buffer-string)) +(org-test-with-temp-text + (format #+INCLUDE: \%s/examples/include2.org\ src emacs-lisp :minlevel 1 org-test-dir) + (org-export-expand-include-keyword) + (buffer-string + ;; INCLUDE assigns the relative :minlevel conditional on narrowing. + (should + (org-test-with-temp-text + (format * h1\npoint#+INCLUDE: \%s/examples/include.org::#ah\ org-test-dir) + (org-narrow-to-element) + (org-export-expand-include-keyword) + (goto-char (point-min)) + (eq 1 (org-element-property :level (org-element-at-point) + ;; If :minlevel is present do not alter it. + (should + (org-test-with-temp-text + (format * h1\npoint#+INCLUDE: \%s/examples/include.org::#ah\ :minlevel 3 org-test-dir) + (org-narrow-to-element) + (org-export-expand-include-keyword) + (goto-char (point-min)) + (eq 3 (org-element-property :level (org-element-at-point)) (ert-deftest test-org-export/expand-macro () Test macro expansion in an Org buffer. -- 2.2.1
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hi, Thanks for the comments. I managed to make the patch less complicated. Nicolas Goaziou m...@nicolasgoaziou.fr writes: + (let* ((lines (and lines (split-string lines -))) + (lbeg (and lines (string-to-number (car lines + (lend (and lines (string-to-number (cadr lines) This is not needed. `point-min' and `point-max' refer to the boundaries of the area to be included. It avoids relying on the expensive `line-number-at-pos' function later. Moreover, I suggest store (point-max) as a marker since you are going to modify contents of the buffer (e.g., adding ID to labels). I did not know markers but they seem perfect in this case. The manual mentions setting markers to nil after use. I guess it's not necessary here since they are in a (let ⋯)? +(goto-char (point-min)) +(while (re-search-forward org-footnote-re nil t) + (let* ((reference (org-element-context)) + (type (org-element-type reference)) + (footnote-type (org-element-property :type reference)) + (label (org-element-property :label reference))) +(when (eq type 'footnote-reference) The order is confusing here. First you check if you're really at a footnote reference, then you bind LABEL and FOOTNOTE-TYPE. Actually, the latter doesn't even need to bound since you use it only once. I check if I'm at a footnote reference 'cause I never want to deal with a footnote-*definition* directly. AFAIK org-footnote-re matches both. It seems a footnote-reference can never be at the beginning of line, but I would still prefer a more explicit test through org-element. + (goto-char (org-element-property :begin reference)) + (when label +(forward-char 4) +(insert (format %d- id)) This looks wrong. Labels can be 1, fn:label or even fn: in anonymous footnotes. You need to check if label matches \\`[0-9]+\\', in which case prepending fn: is also necessary. Ah, that explains the \\`[0-9]+\\' that always confused me. + (let ((definition (org-element-context))) +(when (and lines + (or ( lend (line-number-at-pos +(org-element-property + :contents-begin definition))) + ( lbeg (line-number-at-pos +(org-element-property + :contents-begin definition) + (puthash (org-element-property :label definition) + (org-element-normalize-string +(buffer-substring + (org-element-property + :contents-begin definition) + (org-element-property + :contents-end definition))) + footnotes))) You don't need this part. Basically move to the definition of the current reference in a wide buffer. If you're not within narrowed boundaries stored before, put it in the hash table. Otherwise, skip it. It doesn't require `org-element-context' or `line-number-at-pos'. OK. I do it in a differently now, relying on org-footnote-get-definition. Or do you have something more low-level in mind? The only bug *I'm aware of* is that it will pick up the wrong new-label for footnote for something like [fn:ref with space]. But this is anyway not a proper label, I think. Is that OK? Thanks, Rasmus -- Sådan en god dansk lagereddike kan man slet ikke bruge mere From 36bde4b87a1b3405ab80867d66b074722a251837 Mon Sep 17 00:00:00 2001 From: rasmus ras...@gmx.us Date: Tue, 9 Dec 2014 12:40:52 +0100 Subject: [PATCH 1/2] ox.el: Fix footnote-bug in #+INCLUDE-keyword * ox.el (org-export--prepare-file-contents): Preserve footnotes when using the LINES argument. New optional argument FOOTNOTES. (org-export-expand-include-keyword): New optional argument FOOTNOTES. * test-ox.el (test-org-export/expand-include): Add test for INCLUDE with :lines and footnotes. --- lisp/ox.el | 82 ++--- testing/lisp/test-ox.el | 32 +-- 2 files changed, 87 insertions(+), 27 deletions(-) diff --git a/lisp/ox.el b/lisp/ox.el index 9d9e794..11b9a29 100644 --- a/lisp/ox.el +++ b/lisp/ox.el @@ -3052,17 +3052,20 @@ locally for the subtree through node properties. (car key) (if (org-string-nw-p val) (format %s val) -(defun org-export-expand-include-keyword (optional included dir) +(defun org-export-expand-include-keyword (optional included dir footnotes) Expand every include keyword in buffer. Optional argument INCLUDED is a list of included file names along with their line restriction, when appropriate. It is used to avoid infinite
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Rasmus ras...@gmx.us writes: AFAICT, there's no reason to include a rule about whitespace separating anything. Just make sure that any INCLUDE keyword that doesn't have a :minlevel property gets one set to 1+N, where N is the current level (or 0 if at top level). Another option is to delay insertion of included files: expand them completely in different strings, then replace keywords with appropriate strings. IOW, just make sure expansion doesn't happen sequentially. OK. Solution one sounds easier. A quick attempt, without tests, is given in the second patch. I'll add patches if you agree with the easy approach. It seems to work, though I'm not sure if the matching of headlines which should have :minlevel added is robust enough. Here's a new version of the second patch with tests. The recognition regexp is still not great, but the idea of the regexp is to only act on includes where there's no :minlevel already and no plain words (most obviously src and example, but any block really) when disregarding :key value pairs. At least all tests are passed... —Rasmus -- This is the kind of tedious nonsense up with which I will not put From 5ae354993662b3a3d2fc1c995861401f28b2af1c Mon Sep 17 00:00:00 2001 From: Rasmus ras...@gmx.us Date: Thu, 18 Dec 2014 16:48:49 +0100 Subject: [PATCH 2/2] ox.el: Guess the :minlevel for INCLUDE-keywords * ox.el (org-export-expand-include-keyword): Guess :minlevel if missing and relevant. * test-ox.el: Tests for automatic :minlevel. --- lisp/ox.el | 17 + testing/lisp/test-ox.el | 39 ++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/lisp/ox.el b/lisp/ox.el index 99c4e9b..a9b88fe 100644 --- a/lisp/ox.el +++ b/lisp/ox.el @@ -3066,6 +3066,23 @@ storing and resolving footnotes. It is created automatically. (footnotes (or footnotes (make-hash-table :test #'equal))) (include-re ^[ \t]*#\\+INCLUDE:)) (goto-char (point-min)) +;; Add :minlevel to all include words that no explicitly have one. +(save-excursion + (while (re-search-forward + (concat include-re [ \t]* \\(?:\.+?\\\|[^ \t]+\\)[ \t]* \\(.*\\)$) + nil t) + (let ((matched (match-string 1))) + (unless (or (string-match-p :minlevel matched) + ;; matched a normal word + (string-match \\(?:^\\|[ \t]+\\)\\(\\w\\) +(replace-regexp-in-string \\(^\\|[ t]+\\):\\w+[ \t]+\\w+ + matched))) + (goto-char (line-end-position)) + (insert (format :minlevel %d + (1+ (org-with-wide-buffer + (if (search-backward-regexp org-heading-regexp nil t) + (length (match-string 1)) + 0) ;; Expand INCLUDE keywords. (while (re-search-forward include-re nil t) (let ((element (save-match-data (org-element-at-point diff --git a/testing/lisp/test-ox.el b/testing/lisp/test-ox.el index 140c0a8..47fe5a0 100644 --- a/testing/lisp/test-ox.el +++ b/testing/lisp/test-ox.el @@ -1045,7 +1045,44 @@ baz (org-test-with-temp-text (format #+INCLUDE: \%s/examples/include.org::#dh\ :only-contents t org-test-dir) (org-export-expand-include-keyword) - (buffer-string) + (buffer-string + ;; Adjacent INCLUDE-keywords should have the same :minlevel if unspecified + (should + (org-every (lambda (level) (zerop (1- level))) + (org-test-with-temp-text + (concat + (format #+INCLUDE: \%s/examples/include.org::#ah\\n org-test-dir) + (format #+INCLUDE: \%s/examples/include.org::*Heading\ org-test-dir)) + (org-export-expand-include-keyword) + (org-element-map (org-element-parse-buffer) 'headline + (lambda (head) (org-element-property :level head)) + ;; INCLUDE source code must not have a :minlevel keyword + (should-not + (equal +(org-test-with-temp-text + (format #+INCLUDE: \%s/examples/include2.org\ src emacs-lisp org-test-dir) + (org-export-expand-include-keyword) + (buffer-string)) +(org-test-with-temp-text + (format #+INCLUDE: \%s/examples/include2.org\ src emacs-lisp :minlevel 1 org-test-dir) + (org-export-expand-include-keyword) + (buffer-string + ;; INCLUDE should get the correct :minlevel even if narrowed. + (should + (org-test-with-temp-text + (format * h1\npoint#+INCLUDE: \%s/examples/include.org::#ah\ org-test-dir) + (org-narrow-to-element) + (org-export-expand-include-keyword) + (goto-char (point-min)) + (eq 2 (org-element-property :level (org-element-at-point) + ;; If :minlevel is present do not alter it + (should + (org-test-with-temp-text + (format * h1\npoint#+INCLUDE: \%s/examples/include.org::#ah\ :minlevel 1 org-test-dir) + (org-narrow-to-element) + (org-export-expand-include-keyword) + (goto-char (point-min)) + (eq 1 (org-element-property :level (org-element-at-point)) (ert-deftest test-org-export/expand-macro () Test macro expansion in an Org buffer. -- 2.2.0
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hi, Thanks for the notes. Hopefully patch one if good now. Patch two needs tests, but I can write those if we agree to impose minlevel automatically. Nicolas Goaziou m...@nicolasgoaziou.fr writes: AFAICT, there's no reason to include a rule about whitespace separating anything. Just make sure that any INCLUDE keyword that doesn't have a :minlevel property gets one set to 1+N, where N is the current level (or 0 if at top level). Another option is to delay insertion of included files: expand them completely in different strings, then replace keywords with appropriate strings. IOW, just make sure expansion doesn't happen sequentially. OK. Solution one sounds easier. A quick attempt, without tests, is given in the second patch. I'll add patches if you agree with the easy approach. It seems to work, though I'm not sure if the matching of headlines which should have :minlevel added is robust enough. Objects can be extracted via =#+INCLUDE= using file links. It is -possible to include only the contents of the object. See manual for +possible to include only the contents of the object. Further, +footnotes are now supported when using =#+INCLUDE=. See manual for This is not quite true. Footnotes are already supported with INCLUDE keywords. This is the combination of :lines and footnotes that is new. It is more a bugfix than a new feature. Right. Removed. + (goto-char (point-min)) + (while (and (search-forward-regexp org-footnote-re nil t)) + (let* ((reference (org-element-context)) +(type (org-element-type reference)) +(label (org-element-property :label reference))) + (when (and label (eq type 'footnote-reference)) + (unless (org-footnote-get-definition label) + (save-excursion + (org-footnote-create-definition label) + ;; We do not need an error here since ox + ;; will complain if a footnote is missing. + (insert (or (gethash label footnotes) ))) Why is the above necessary? Shouldn't you only insert footnotes definitions at the end of the master document (i.e. when INCLUDED is nil)? Indeed. Thanks for the hint! I think a `maphash' is enough. Also, looking for every footnote reference sounds tedious. You should simply insert every footnote definition collected there, and filter out unnecessary definitions at another level (e.g., before storing it in the hash table). Thanks! + (when id +(unless (eq major-mode 'org-mode) + (let ((org-inhibit-startup t)) (org-mode))) Is it necessary? I think org-with-wide-buffer is sufficient. +(forward-char 4) +(insert (format %d- id)) +(and (not (eq footnote-type 'inline)) + (let ((new-label (org-element-property + :label (org-element-context Why do you need to parse the new label, since you know it already: (concat (format %d- id) label) Almost, but label contains fn: first, so the above would be e.g. 1-fn:1. I didn't see an elegant way of doing it at first, thus I used elements, but now I just use regexp-replace... I solved in another way. + (save-restriction + (save-excursion + (widen) `save-restriction' + `save-excursion' + `widen' = `org-with-wide-buffer' Cool. + (puthash new-label + (org-element-normalize-string +(buffer-substring + (org-element-property + :contents-begin definition) + (org-element-property + :contents-end definition))) + footnotes)) Here you could check if :contents-begin is within LINES, in which case the definition needs not be inserted at the end of the master document. Good idea. I did it a bit more elaborated since footnotes can in principle also be before the definition. I don't check for the end. Thanks, Rasmus -- Slowly unravels in a ball of yarn and the devil collects it From 5d79c76c6a93666a1521a5d5eefe3d79bda3d00d Mon Sep 17 00:00:00 2001 From: rasmus ras...@gmx.us Date: Tue, 9 Dec 2014 12:40:52 +0100 Subject: [PATCH 1/2] ox.el: Fix footnote-bug in #+INCLUDE-keyword * ox.el (org-export--prepare-file-contents): Preserve footnotes when using the LINES argument. New optional argument FOOTNOTES. (org-export-expand-include-keyword): New optional argument FOOTNOTES. * test-ox.el: Add test for INCLUDE with :lines and footnotes. --- lisp/ox.el | 116 +--- testing/lisp/test-ox.el
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hello, Rasmus ras...@gmx.us writes: Attached is a patch that enables footnotes in INCLUDEd documents when using :lines and friends. It stores the footnotes in a hash-table initialized in `org-export-expand-include-keyword' and updated via `org-export--prepare-file-contents'. The footnotes are then inserted when all include keywords are expanded. Thanks. Some more comments follow. At the moment only footnotes from INCLUDEs with :lines-like arguments will be picket up here. But I think it might be nice to also use this functionality with footnotes when whole documents are included, and not include the footnote section directly from these documents. Though I expect the to be accused of worm-nurturing, do consider this curious example: [...] 2. fix the bug (IMO) that is that #+INCLUDE: /tmp/t00.org #+INCLUDE: /tmp/t01.org Is read as #+INCLUDE: /tmp/t00.org :minlevel N #+INCLUDE: /tmp/t01.org :minlevel N+1 The easiest way I can think of would be to do a pre-scan of the buffer to see if there exists any instances where include is only separated by whitespace in which case they should have the same level. AFAICT, there's no reason to include a rule about whitespace separating anything. Just make sure that any INCLUDE keyword that doesn't have a :minlevel property gets one set to 1+N, where N is the current level (or 0 if at top level). Another option is to delay insertion of included files: expand them completely in different strings, then replace keywords with appropriate strings. IOW, just make sure expansion doesn't happen sequentially. Objects can be extracted via =#+INCLUDE= using file links. It is -possible to include only the contents of the object. See manual for +possible to include only the contents of the object. Further, +footnotes are now supported when using =#+INCLUDE=. See manual for This is not quite true. Footnotes are already supported with INCLUDE keywords. This is the combination of :lines and footnotes that is new. It is more a bugfix than a new feature. + (footnotes (or footnotes (make-hash-table :test 'equal Nitpick: (make-hash-table :test #'equal) +(goto-char (point-min)) +(while (and (search-forward-regexp org-footnote-re nil t)) + (let* ((reference (org-element-context)) + (type (org-element-type reference)) + (label (org-element-property :label reference))) +(when (and label (eq type 'footnote-reference)) + (unless (org-footnote-get-definition label) +(save-excursion + (org-footnote-create-definition label) + ;; We do not need an error here since ox + ;; will complain if a footnote is missing. + (insert (or (gethash label footnotes) ))) Why is the above necessary? Shouldn't you only insert footnotes definitions at the end of the master document (i.e. when INCLUDED is nil)? I think a `maphash' is enough. Also, looking for every footnote reference sounds tedious. You should simply insert every footnote definition collected there, and filter out unnecessary definitions at another level (e.g., before storing it in the hash table). + (when id + (unless (eq major-mode 'org-mode) + (let ((org-inhibit-startup t)) (org-mode))) Is it necessary? + (goto-char (point-min)) + (while (re-search-forward org-footnote-re nil t) + (let* ((reference (org-element-context)) + (type (org-element-type reference)) + (footnote-type (org-element-property :type reference)) + (label (org-element-property :label reference))) + (when (and (eq type 'footnote-reference)) ^^^ Typo. + (goto-char (org-element-property :begin reference)) + (when label + (goto-char (org-element-property :begin reference)) You are already at reference beginning. + (forward-char 4) + (insert (format %d- id)) + (and (not (eq footnote-type 'inline)) + (let ((new-label (org-element-property +:label (org-element-context Why do you need to parse the new label, since you know it already: (concat (format %d- id) label) +(save-restriction + (save-excursion +(widen) `save-restriction' + `save-excursion' + `widen' = `org-with-wide-buffer' +(org-footnote-goto-definition label) +(let ((definition (org-element-context))) + (and include-footnotes Nitpick: (when include-footnotes ... + (puthash new-label +
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hi,
Attached is a patch that enables footnotes in INCLUDEd documents when
using :lines and friends. It stores the footnotes in a hash-table
initialized in `org-export-expand-include-keyword' and updated via
`org-export--prepare-file-contents'. The footnotes are then inserted when
all include keywords are expanded.
At the moment only footnotes from INCLUDEs with :lines-like arguments will
be picket up here. But I think it might be nice to also use this
functionality with footnotes when whole documents are included, and not
include the footnote section directly from these documents. Though I
expect the to be accused of worm-nurturing, do consider this curious example:
$ cat t{1,0*}.org
#+TITLE: This is t1.org
#+INCLUDE: /tmp/t00.org
#+INCLUDE: /tmp/t01.org
# This is t00.org
Footnotes[fn:1], [fn:test] and [fn:inline:anonymous footnote].
* Footnotes
[fn:1] Footnote 1
[fn:test] Footnote test
# t01.org
Footnotes[fn:1], [fn:test] and [fn:inline:anonymous footnote].
* Footnotes
[fn:1] Footnote 1
[fn:test] Footnote test
ox will in fact interpret t1.org as:
#+TITLE: This is t1.org
Footnotes[fn:1-1], [fn:1-test] and [fn:1-inline:anonymous footnote].
[fn:1-1] Footnote 1
[fn:1-test] Footnote test
Footnotes[fn:2-1], [fn:2-test] and [fn:2-inline:anonymous footnote].
So I see three approaches:
1. let the user shoot himself in the foot
2. fix the bug (IMO) that is that
#+INCLUDE: /tmp/t00.org
#+INCLUDE: /tmp/t01.org
Is read as
#+INCLUDE: /tmp/t00.org :minlevel N
#+INCLUDE: /tmp/t01.org :minlevel N+1
The easiest way I can think of would be to do a pre-scan of the
buffer to see if there exists any instances where include is only
separated by whitespace in which case they should have the same
level.
3. Fix the particular nastiness above by removing footnote sections and
reinserting them using the mechanism of this patch.
—Rasmus
--
A page of history is worth a volume of logic
From 04726b6e5915fc47f3ecc261f9c7d9dfb2b44f56 Mon Sep 17 00:00:00 2001
From: rasmus ras...@gmx.us
Date: Tue, 9 Dec 2014 12:40:52 +0100
Subject: [PATCH] ox.el: Fix footnote-bug in #+INCLUDE-keyword
* ox.el (org-export--prepare-file-contents): Preserve footnotes
when using the LINES argument. New optional argument FOOTNOTES.
(org-export-expand-include-keyword): New optional argument
FOOTNOTES.
* test-ox.el: Add test for INCLUDE with :lines and footnotes.
* ORG-NEWS: Update.
---
etc/ORG-NEWS| 3 +-
lisp/ox.el | 94 +++--
testing/lisp/test-ox.el | 25 +
3 files changed, 95 insertions(+), 27 deletions(-)
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index f719886..f831c60 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -103,7 +103,8 @@ The content of the included file can now be optionally marked up, for
instance as HTML. See the documentation for details.
*** File links with =#+INCLUDE= keyword
Objects can be extracted via =#+INCLUDE= using file links. It is
-possible to include only the contents of the object. See manual for
+possible to include only the contents of the object. Further,
+footnotes are now supported when using =#+INCLUDE=. See manual for
more information.
*** Additional =:hline= processing to ob-shell
If the argument =:hlines yes= is present in a babel call, an optional
diff --git a/lisp/ox.el b/lisp/ox.el
index 9d9e794..975f178 100644
--- a/lisp/ox.el
+++ b/lisp/ox.el
@@ -3052,16 +3052,18 @@ locally for the subtree through node properties.
(car key)
(if (org-string-nw-p val) (format %s val)
-(defun org-export-expand-include-keyword (optional included dir)
+(defun org-export-expand-include-keyword (optional included dir footnotes)
Expand every include keyword in buffer.
Optional argument INCLUDED is a list of included file names along
with their line restriction, when appropriate. It is used to
avoid infinite recursion. Optional argument DIR is the current
working directory. It is used to properly resolve relative
-paths.
+paths. Optional argument FOOTNOTES is a hash-table used for
+storing and resolving footnotes. It is created automatically.
(let ((case-fold-search t)
(file-prefix (make-hash-table :test #'equal))
- (current-prefix 0))
+ (current-prefix 0)
+ (footnotes (or footnotes (make-hash-table :test 'equal
(goto-char (point-min))
(while (re-search-forward ^[ \t]*#\\+INCLUDE: nil t)
(let ((element (save-match-data (org-element-at-point
@@ -3155,14 +3157,27 @@ paths.
file location only-contents lines)
lines)))
(org-mode)
- (insert
- (org-export--prepare-file-contents
- file lines ind minlevel
- (or (gethash file file-prefix)
- (puthash file (incf current-prefix) file-prefix)
+ (insert
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Rasmus ras...@gmx.us writes: Nicolas Goaziou m...@nicolasgoaziou.fr writes: I think required definitions should be extracted from the included file and inserted at the end of the source file, without any footnote section. The hard solution. I will look into it. It may not be that hard, but it will require tests. You also need to check that the footnote definition doesn't appear within included area, in which case it needs not be moved. However, it would be nice to store associations between files and footnote labels in, e.g., a hash table, in order to avoid inserting multiple times the same footnote. What is your reasoning for this statement? Aesthetics, performance or something else? Mainly performance, and to prevent some bugs that might be introduced with duplicate footnote definitions. The can is open, the worms wiggling. Have fun. Regards,
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Nicolas Goaziou m...@nicolasgoaziou.fr writes: Rasmus ras...@gmx.us writes: Nicolas Goaziou m...@nicolasgoaziou.fr writes: I think required definitions should be extracted from the included file and inserted at the end of the source file, without any footnote section. The hard solution. I will look into it. It may not be that hard, but it will require tests. You also need to check that the footnote definition doesn't appear within included area, in which case it needs not be moved. I have the collection of footnotes in the (beg end) area sorted out in the patch I wrote last night. Storing the triplet (document-path label footnote-text) should be enough to pin down one of them footnotes. I'm curious about the hash table. (info (elisp) Hash Tables) says For smaller tables (a few tens of elements) alists may still be faster [than hash tables]. For an Org document, might it not make more sense to use an alist for this? Or will the speed be regained when doing many includes from the same document (since I'd check if a footnote is already in the table)? Also, since INCLUDE is expanded before info, should I just create a new defvar holding the table during export? I guess that's the only way to hold it in memory across several INCLUDE words. The can is open, the worms wiggling. Have fun. My favorite kind. —Rasmus -- Evidence suggests Snowden used a powerful tool called monospaced fonts
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Rasmus ras...@gmx.us writes: I'm curious about the hash table. (info (elisp) Hash Tables) says For smaller tables (a few tens of elements) alists may still be faster [than hash tables]. True, but then, both a small table and a small alist are very fast. OTOH, hash tables scale better. For an Org document, might it not make more sense to use an alist for this? It doesn't matter much. I'd still favor a hash-table since it's hard to predict an upper bound for include keywords in a document, but it's your call, really. I doubt the alist or hash table will be the bottleneck. Or will the speed be regained when doing many includes from the same document (since I'd check if a footnote is already in the table)? You would need to access the alist/hash table for each include keyword, not necessarily from the same document. Also, since INCLUDE is expanded before info, should I just create a new defvar holding the table during export? I guess that's the only way to hold it in memory across several INCLUDE words. It should be in the scope of `org-export-expand-include-keyword', much like `file-prefix'. No need for a global variable. Regards,
[O] [bug, patch, ox] INCLUDE and footnotes
Hi,
When using LINES in `org-export--prepare-file-contents' the footnotes
section is not preserved causing export to fail.
Minimal example
$ cat t{1,2}.org
# this is t1.org
* intro
foo[fn:1]
* sec2
bar
* Footnotes
[fn:1] baz
# this is t2.org
#+INCLUDE: ./t1.org::#intro
And export t2.org.
The attached patch fixes this by explicitly saving the footnote section
(Aside: org-footnote-section is used in hackish ways; should we make a
function that returns to correct regexp for the footnotes section?).
It works in a rather large document of mine and in the minimal test.
Should I apply it, or is there a better way to fix this bug?
Thanks,
Rasmus
--
Slowly unravels in a ball of yarn and the devil collects it
From 3f352159d9011e6a00af853fa6dadb04a5b46c87 Mon Sep 17 00:00:00 2001
From: rasmus ras...@gmx.us
Date: Tue, 9 Dec 2014 12:40:52 +0100
Subject: [PATCH] ox.el: Fix footnote-bug in #+INCLUDE-keyword
* ox.el (org-export--prepare-file-contents): Preserve footnote-section
when using the LINES argument.
---
lisp/ox.el | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/lisp/ox.el b/lisp/ox.el
index 9d9e794..8d4fd6c 100644
--- a/lisp/ox.el
+++ b/lisp/ox.el
@@ -3260,8 +3260,22 @@ with footnotes is included in a document.
(end (if (zerop lend) (point-max)
(goto-char (point-min))
(forward-line (1- lend))
- (point
- (narrow-to-region beg end)))
+ (point)))
+ (footnote-section
+ (save-excursion
+ (goto-char (point-min))
+ (when (search-forward-regexp
+ (concat ^\\*+[ \t]+ org-footnote-section [ \t]*$)
+ nil t)
+ (buffer-substring
+ (point-at-bol)
+ (or
+ (and (search-forward-regexp org-heading-regexp nil t)
+ (point-at-bol))
+ (point-max)))
+ (narrow-to-region beg end)
+ (and footnote-section
+ (insert \n footnote-section
;; Remove blank lines at beginning and end of contents. The logic
;; behind that removal is that blank lines around include keyword
;; override blank lines in included file.
--
2.1.3
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Rasmus ras...@gmx.us writes:
The attached patch fixes this by explicitly saving the footnote section
As per usual my first patch is dodgy. It occurred to me that Org can
handle several footnote sections (that's how #+INCLUDE supports footnotes,
I guess). The attached patch how supports export of t2.org in this
example
$ cat t{1,2}.org
# this is t1.org
* foo
bar[fn:1]
baz[fn:2]
* Footnotes
[fn:1] bar.corp
* Footnotes
[fn:2] baz.corp
# this is t2.org
#+INCLUDE: /tmp/t1.org::*foo
However, there is a pitch-fall when doing
#+INCLUDE: /tmp/t1.org::*foo0
#+INCLUDE: /tmp/t1.org::*foo1
Now *foo1 will be inserted under the footnote-heading of *foo0 which means
it won't be exported. If min-level is used this is not an issue of
course, but that's kind of unexpected (so is the fact that the second
include will become a child of the first include, but that's another
patch) .
Perhaps a better overall approach (if the above limitation is
unacceptable) would to translate all footnotes in an INCLUDEd file to
inline ones, e.g. when including * foo in t1.org above, what would be
inserted is
* foo
bar[fn::bar.corp]
baz[fn::baz.corp]
Yet another solution would be to return a cond of
(included-text . included-footnotes)
and make sure to insert footnotes at the very end.
WDYT?
—Rasmus
--
. . . The proofs are technical in nature and provides no real understanding
From 2a943b40c024df092cc2cf020bdf2646e7ab4b2c Mon Sep 17 00:00:00 2001
From: rasmus ras...@gmx.us
Date: Tue, 9 Dec 2014 12:40:52 +0100
Subject: [PATCH] ox.el: Fix footnote-bug in #+INCLUDE-keyword
* ox.el (org-export--prepare-file-contents): Preserve footnote-section
when using the LINES argument.
---
lisp/ox.el | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/lisp/ox.el b/lisp/ox.el
index 9d9e794..bf2ce4d 100644
--- a/lisp/ox.el
+++ b/lisp/ox.el
@@ -3260,8 +3260,27 @@ with footnotes is included in a document.
(end (if (zerop lend) (point-max)
(goto-char (point-min))
(forward-line (1- lend))
- (point
- (narrow-to-region beg end)))
+ (point)))
+ (footnote-section-re
+ (concat ^\\*+[ \t]+ org-footnote-section [ \t]*$))
+ (footnote-sections
+ (save-match-data
+ (save-excursion
+ (goto-char (point-min))
+ (loop do
+ (or (search-forward-regexp footnote-section-re nil t)
+ (end-of-buffer))
+ while (not (eobp))
+ collect
+ (buffer-substring
+ (line-beginning-position)
+ (or (and
+ (search-forward-regexp org-heading-regexp nil t)
+ (goto-char (match-beginning 0)))
+ (point-max
+ (narrow-to-region beg end)
+ (and footnote-sections
+ (insert \n (mapconcat 'identity footnote-sections \n)
;; Remove blank lines at beginning and end of contents. The logic
;; behind that removal is that blank lines around include keyword
;; override blank lines in included file.
--
2.1.3
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hello,
Rasmus ras...@gmx.us writes:
When using LINES in `org-export--prepare-file-contents' the footnotes
section is not preserved causing export to fail.
Minimal example
$ cat t{1,2}.org
# this is t1.org
* intro
foo[fn:1]
* sec2
bar
* Footnotes
[fn:1] baz
# this is t2.org
#+INCLUDE: ./t1.org::#intro
And export t2.org.
The attached patch fixes this by explicitly saving the footnote section
(Aside: org-footnote-section is used in hackish ways; should we make a
function that returns to correct regexp for the footnotes section?).
It works in a rather large document of mine and in the minimal test.
Should I apply it, or is there a better way to fix this bug?
Thanks for the patch. However, it is incorrect.
First `org-footnote-section' could be nil, in which case there is no
headline to look after.
Also, there may be multiple footnote sections in the included document,
or even some footnote definitions inside and some outside the single
section...
Eventually, you are inserting a headline in the source document, which
could break its structure (e.g., if you're only inserting a table).
You should only extract the definitions associated to the references
within the included part of the document. However, you cannot insert
them right after the included text, as it could break the surrounding
environment, e.g.,
- item
#+INCLUDE: some-table.org
A possible solution would be to somehow postpone insertion of footnotes
at the very end of the source document, not at the location of the
keyword. However it would need some testing.
Regards,
--
Nicolas Goaziou
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Hi, Nicolas Goaziou m...@nicolasgoaziou.fr writes: First `org-footnote-section' could be nil, in which case there is no headline to look after. Sure. Also, there may be multiple footnote sections in the included document, or even some footnote definitions inside and some outside the single section... Multiple footnote sections are supported in the second revision of the patch... You should only extract the definitions associated to the references within the included part of the document. However, you cannot insert them right after the included text, as it could break the surrounding environment, e.g., - item #+INCLUDE: some-table.org A possible solution would be to somehow postpone insertion of footnotes at the very end of the source document, not at the location of the keyword. However it would need some testing. Right, I note something similar in my second post. http://permalink.gmane.org/gmane.emacs.orgmode/93299 Clearly the current situation is not satisfactory (You can use :lines, but only if no footnotes are present. . . IOW, :lines supports a subset of Org syntax.). I prefer converting [fn:N] references to [fn::FOOTNOTE] (see my other email). Any obvious downsides? —Rasmus -- Vote for proprietary math!
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Rasmus ras...@gmx.us writes: Clearly the current situation is not satisfactory (You can use :lines, but only if no footnotes are present. . . IOW, :lines supports a subset of Org syntax.). I prefer converting [fn:N] references to [fn::FOOTNOTE] (see my other email). Any obvious downsides? Yes: inline and regular footnotes are not equivalent. For example, a regular footnote can contain a table, a plain list... So this is not an option here. I think required definitions should be extracted from the included file and inserted at the end of the source file, without any footnote section. However, it would be nice to store associations between files and footnote labels in, e.g., a hash table, in order to avoid inserting multiple times the same footnote. Regards,
Re: [O] [bug, patch, ox] INCLUDE and footnotes
Nicolas Goaziou m...@nicolasgoaziou.fr writes: Rasmus ras...@gmx.us writes: Clearly the current situation is not satisfactory (You can use :lines, but only if no footnotes are present. . . IOW, :lines supports a subset of Org syntax.). I prefer converting [fn:N] references to [fn::FOOTNOTE] (see my other email). Any obvious downsides? Yes: inline and regular footnotes are not equivalent. For example, a regular footnote can contain a table, a plain list... So this is not an option here. Damn. I only saw this after changing my path to this behavior. Good point anyway. Though the idea of a table in a footnote is truly horrifying. I think required definitions should be extracted from the included file and inserted at the end of the source file, without any footnote section. The hard solution. I will look into it. However, it would be nice to store associations between files and footnote labels in, e.g., a hash table, in order to avoid inserting multiple times the same footnote. What is your reasoning for this statement? Aesthetics, performance or something else? —Rasmus -- Dobbelt-A
