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: Mon, 22 Dec 2014 12:10:15 +0100 [thread overview]
Message-ID: <87wq5kvt2g.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <87lhm01mj7.fsf@gmx.us> (rasmus@gmx.us's message of "Mon, 22 Dec 2014 02:49:32 +0100")
Rasmus <rasmus@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,
next prev parent reply other threads:[~2014-12-22 11:09 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
2014-12-22 11:10 ` Nicolas Goaziou [this message]
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=87wq5kvt2g.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).