From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [bug, patch, ox] INCLUDE and footnotes Date: Sun, 21 Dec 2014 21:52:06 +0100 Message-ID: <87mw6gyctg.fsf@nicolasgoaziou.fr> References: <87h9x5hwso.fsf@gmx.us> <87oarcbppe.fsf@nicolasgoaziou.fr> <87fvcozfhf.fsf@gmx.us> <87h9x4bj33.fsf@nicolasgoaziou.fr> <87iohks4ne.fsf@gmx.us> <87d27rbvio.fsf@nicolasgoaziou.fr> <87bnnbhg2x.fsf@gmx.us> <878uifbjc7.fsf@nicolasgoaziou.fr> <87388j9qbv.fsf@gmx.us> <87y4q57t2i.fsf@nicolasgoaziou.fr> <87lhm4n9ky.fsf@pank.eu> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:46367) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y2nTF-0007Zj-UT for emacs-orgmode@gnu.org; Sun, 21 Dec 2014 15:51:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y2nT7-0007ri-Rx for emacs-orgmode@gnu.org; Sun, 21 Dec 2014 15:51:21 -0500 Received: from relay4-d.mail.gandi.net ([2001:4b98:c:538::196]:34370) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y2nT7-0007pA-Ic for emacs-orgmode@gnu.org; Sun, 21 Dec 2014 15:51:13 -0500 In-Reply-To: <87lhm4n9ky.fsf@pank.eu> (rasmus@gmx.us's message of "Thu, 18 Dec 2014 18:37:01 +0100") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Rasmus Cc: emacs-orgmode@gnu.org Rasmus 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,