From: Rasmus <rasmus@gmx.us>
To: emacs-orgmode@gnu.org
Subject: Re: [bug, patch, ox] INCLUDE and footnotes
Date: Mon, 22 Dec 2014 02:49:32 +0100 [thread overview]
Message-ID: <87lhm01mj7.fsf@gmx.us> (raw)
In-Reply-To: <87mw6gyctg.fsf@nicolasgoaziou.fr> (Nicolas Goaziou's message of "Sun, 21 Dec 2014 21:52:06 +0100")
[-- Attachment #1: Type: text/plain, Size: 3430 bytes --]
Hi,
Thanks for the comments. I managed to make the patch less complicated.
Nicolas Goaziou <mail@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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ox.el-Fix-footnote-bug-in-INCLUDE-keyword.patch --]
[-- Type: text/x-diff, Size: 8131 bytes --]
From 36bde4b87a1b3405ab80867d66b074722a251837 Mon Sep 17 00:00:00 2001
From: rasmus <rasmus@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 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,24 @@ 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.
+ (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)))))))))))
(defun org-export--inclusion-absolute-lines (file location only-contents lines)
"Resolve absolute lines for an included file with file-link.
@@ -3227,8 +3239,8 @@ Return a string of lines to be included in the format expected by
(while (< (point) end) (incf counter) (forward-line))
counter))))))))
-(defun org-export--prepare-file-contents (file &optional lines ind minlevel id)
- "Prepare the contents of FILE for inclusion and return them as a string.
+(defun org-export--prepare-file-contents (file &optional lines ind minlevel id footnotes)
+ "Prepare contents of FILE for inclusion and return it as a string.
When optional argument LINES is a string specifying a range of
lines, include only those lines.
@@ -3246,7 +3258,11 @@ file should have.
Optional argument ID is an integer that will be inserted before
each footnote definition and reference if FILE is an Org file.
This is useful to avoid conflicts when more than one Org file
-with footnotes is included in a document."
+with footnotes is included in a document.
+
+Optional argument FOOTNOTES is a hash-table to store footnotes in
+the included document.
+"
(with-temp-buffer
(insert-file-contents file)
(when lines
@@ -3309,18 +3325,34 @@ with footnotes is included in a document."
;; become file specific and cannot collide with footnotes in other
;; included files.
(when id
- (goto-char (point-min))
- (while (re-search-forward org-footnote-re nil t)
- (let ((reference (org-element-context)))
- (when (memq (org-element-type reference)
- '(footnote-reference footnote-definition))
- (goto-char (org-element-property :begin reference))
- (forward-char)
- (let ((label (org-element-property :label reference)))
- (cond ((not label))
- ((org-string-match-p "\\`[0-9]+\\'" label)
- (insert (format "fn:%d-" id)))
- (t (forward-char 3) (insert (format "%d-" id)))))))))
+ (let ((marker-min (point-min-marker))
+ (marker-max (point-max-marker)))
+ (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)
+ (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)
+ (org-with-wide-buffer
+ (let* ((definition (org-footnote-get-definition label))
+ (beginning (set-marker (make-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))))))))))))
(org-element-normalize-string (buffer-string))))
(defun org-export-execute-babel-code ()
diff --git a/testing/lisp/test-ox.el b/testing/lisp/test-ox.el
index 9a0e787..37e2e23 100644
--- a/testing/lisp/test-ox.el
+++ b/testing/lisp/test-ox.el
@@ -904,12 +904,13 @@ Footnotes[fn:1], [fn:test] and [fn:inline:anonymous footnote].
(org-element-property :label ref)))))))))))))
;; Footnotes labels are not local to each include keyword.
(should
- (= 3
+ (= 4
(length
(delete-dups
(let ((contents "
-Footnotes[fn:1], [fn:test] and [fn:inline:anonymous footnote].
+Footnotes[fn:1], [fn:test], [2] and [fn:inline:anonymous footnote].
\[fn:1] Footnote 1
+\[2] Footnote 2
\[fn:test] Footnote \"test\""))
(org-test-with-temp-text-in-file contents
(let ((file (buffer-file-name)))
@@ -919,6 +920,33 @@ Footnotes[fn:1], [fn:test] and [fn:inline:anonymous footnote].
(org-element-map (org-element-parse-buffer)
'footnote-reference
(lambda (ref) (org-element-property :label ref)))))))))))
+ ;; Footnotes are supported by :lines-like elements and unnecessary
+ ;; footnotes are dropped.
+ (should
+ (= 4
+ (length
+ (delete-dups
+ (let ((contents "
+* foo
+Footnotes[fn:1]
+* bar
+Footnotes[fn:2], foot[fn:test], digit only[3], and [fn:inline:anonymous footnote]
+
+\[fn:1] Footnote 1
+\[fn:2] Footnote 1
+* Footnotes
+\[fn:test] Footnote \"test\"
+\[3] Footnote 3
+"))
+ (org-test-with-temp-text-in-file contents
+ (let ((file (buffer-file-name)))
+ (org-test-with-temp-text
+ (format "#+INCLUDE: \"%s::*bar\"
+" file)
+ (org-export-expand-include-keyword)
+ (org-element-map (org-element-parse-buffer)
+ 'footnote-definition
+ (lambda (ref) (org-element-property :label ref)))))))))))
;; If only-contents is non-nil only include contents of element.
(should
(equal
--
2.2.1
next prev parent reply other threads:[~2014-12-22 1:49 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-09 11:44 [bug, patch, ox] INCLUDE and footnotes Rasmus
2014-12-09 19:10 ` Rasmus
2014-12-09 19:14 ` Nicolas Goaziou
2014-12-09 21:21 ` Rasmus
2014-12-09 21:37 ` Nicolas Goaziou
2014-12-10 0:57 ` Rasmus
2014-12-10 11:21 ` Nicolas Goaziou
2014-12-10 11:58 ` Rasmus
2014-12-10 15:44 ` Nicolas Goaziou
2014-12-13 21:45 ` Rasmus
2014-12-17 23:30 ` Nicolas Goaziou
2014-12-18 17:37 ` Rasmus
2014-12-19 16:44 ` Rasmus
2014-12-21 21:04 ` Nicolas Goaziou
2014-12-21 22:39 ` Rasmus
2014-12-21 23:38 ` Nicolas Goaziou
2014-12-22 1:42 ` Rasmus
2014-12-22 9:05 ` Nicolas Goaziou
2014-12-24 18:03 ` Rasmus
2014-12-24 21:14 ` Nicolas Goaziou
2014-12-25 1:38 ` Rasmus
2014-12-25 2:04 ` Rasmus
2014-12-21 20:52 ` Nicolas Goaziou
2014-12-22 1:49 ` Rasmus [this message]
2014-12-22 11:10 ` Nicolas Goaziou
2014-12-22 12:36 ` Rasmus
2014-12-22 20:54 ` Nicolas Goaziou
2014-12-22 22:11 ` Rasmus
2014-12-22 22:51 ` Nicolas Goaziou
2014-12-23 2:09 ` Rasmus
2014-12-24 17:54 ` Rasmus
2014-12-24 18:10 ` [git-101] How to push a branch and avoid merge-message? (was: [bug, patch, ox] INCLUDE and footnotes) Rasmus
2014-12-24 21:09 ` [git-101] How to push a branch and avoid merge-message? Nicolas Goaziou
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=87lhm01mj7.fsf@gmx.us \
--to=rasmus@gmx.us \
--cc=emacs-orgmode@gnu.org \
/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).