From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Rasmus <rasmus@gmx.us>
Cc: emacs-orgmode@gnu.org
Subject: Re: [bug, patch, ox] INCLUDE and footnotes
Date: Sun, 21 Dec 2014 21:52:06 +0100 [thread overview]
Message-ID: <87mw6gyctg.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <87lhm4n9ky.fsf@pank.eu> (rasmus@gmx.us's message of "Thu, 18 Dec 2014 18:37:01 +0100")
Rasmus <rasmus@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
> + :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'.
Regards,
next prev parent reply other threads:[~2014-12-21 20:51 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 [this message]
2014-12-22 1:49 ` Rasmus
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=87mw6gyctg.fsf@nicolasgoaziou.fr \
--to=mail@nicolasgoaziou.fr \
--cc=emacs-orgmode@gnu.org \
--cc=rasmus@gmx.us \
/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).