[bug, patch, ox] INCLUDE and footnotes

[bug, patch, ox] INCLUDE and footnotes

[Rasmus]

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]

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

[-- 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: 1375 bytes --]

From 3f352159d9011e6a00af853fa6dadb04a5b46c87 Mon Sep 17 00:00:00 2001
From: rasmus <rasmus@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: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]

Rasmus <rasmus@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

[-- 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: 1578 bytes --]

From 2a943b40c024df092cc2cf020bdf2646e7ab4b2c Mon Sep 17 00:00:00 2001
From: rasmus <rasmus@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: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

Hello,

Rasmus <rasmus@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: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

Hi,

Nicolas Goaziou <mail@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: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

Rasmus <rasmus@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: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Rasmus <rasmus@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

Re: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

Rasmus <rasmus@gmx.us> writes:

> Nicolas Goaziou <mail@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: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Rasmus <rasmus@gmx.us> writes:
>
>> Nicolas Goaziou <mail@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: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

Rasmus <rasmus@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,

Re: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

[-- Attachment #1: Type: text/plain, Size: 2176 bytes --]

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

[-- 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: 8624 bytes --]

From 04726b6e5915fc47f3ecc261f9c7d9dfb2b44f56 Mon Sep 17 00:00:00 2001
From: rasmus <rasmus@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 (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))
+		    (file-name-directory file)
+		    footnotes)
+		   (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) "")))))))
 		   (buffer-string)))))))))))))
 
 (defun org-export--inclusion-absolute-lines (file location only-contents lines)
@@ -3227,8 +3242,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 +3261,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
@@ -3308,20 +3327,43 @@ with footnotes is included in a document."
     ;; Append ID to all footnote references and definitions, so they
     ;; 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)))))))))
-    (org-element-normalize-string (buffer-string))))
+    (let* ((include-footnotes (and id lines)))
+      (when id
+	(unless (eq major-mode 'org-mode)
+	  (let ((org-inhibit-startup t)) (org-mode)))
+	(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))
+	      (goto-char (org-element-property :begin reference))
+	      (when label
+		(goto-char (org-element-property :begin reference))
+		(forward-char 4)
+		(insert (format "%d-" id))
+		(and (not (eq footnote-type 'inline))
+		     (let ((new-label (org-element-property
+				       :label (org-element-context))))
+		       (save-restriction
+			 (save-excursion
+			   (widen)
+			   (org-footnote-goto-definition label)
+			   (let ((definition (org-element-context)))
+			     (and include-footnotes
+				  (puthash new-label
+					   (org-element-normalize-string
+					    (buffer-substring
+					     (org-element-property
+					      :contents-begin definition)
+					     (org-element-property
+					      :contents-end definition)))
+					   footnotes))
+			     (goto-char (org-element-property :begin definition))
+			     (forward-char 4)
+			     (insert (format "%d-" id))))))))))))
+      (org-element-normalize-string (buffer-string)))))
 
 (defun org-export-execute-babel-code ()
   "Execute every Babel code in the visible part of current buffer."
diff --git a/testing/lisp/test-ox.el b/testing/lisp/test-ox.el
index 9a0e787..da4850d 100644
--- a/testing/lisp/test-ox.el
+++ b/testing/lisp/test-ox.el
@@ -919,6 +919,31 @@ 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
+   (= 3
+      (length
+       (delete-dups
+	(let ((contents "
+* foo
+Footnotes[fn:1]
+* bar
+Footnotes[fn:2], [fn:test], and [fn:inline:anonymous footnote]
+
+[fn:1] Footnote 1
+[fn:2] Footnote 1
+* Footnotes
+[fn:test] Footnote \"test\""))
+	  (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.1.3

Re: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

Hello,

Rasmus <rasmus@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
> +					   (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.

Regards,

-- 
Nicolas Goaziou

Re: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

[-- Attachment #1: Type: text/plain, Size: 4004 bytes --]

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 <mail@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

[-- 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: 10083 bytes --]

From 5d79c76c6a93666a1521a5d5eefe3d79bda3d00d 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: Add test for INCLUDE with :lines and footnotes.
---
 lisp/ox.el              | 116 +++++++++++++++++++++++++++++++++++++-----------
 testing/lisp/test-ox.el |  70 +++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+), 27 deletions(-)

diff --git a/lisp/ox.el b/lisp/ox.el
index 9d9e794..99c4e9b 100644
--- a/lisp/ox.el
+++ b/lisp/ox.el
@@ -3052,18 +3052,22 @@ 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)))
+	(include-re "^[ \t]*#\\+INCLUDE:"))
     (goto-char (point-min))
-    (while (re-search-forward "^[ \t]*#\\+INCLUDE:" nil t)
+    ;; Expand INCLUDE keywords.
+    (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)
@@ -3155,15 +3159,44 @@ 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 in an
+	       ;; `org-footnote-section' if that variable is
+	       ;; non-nil.  Otherwise they are stored close to the definition.
+	      (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))))))))))))
 
 (defun org-export--inclusion-absolute-lines (file location only-contents lines)
   "Resolve absolute lines for an included file with file-link.
@@ -3227,8 +3260,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 +3279,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,19 +3346,44 @@ 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)))))))))
-    (org-element-normalize-string (buffer-string))))
+      (let* ((lines (and lines (split-string lines "-")))
+	     (lbeg (and lines (string-to-number (car lines))))
+	     (lend (and lines (string-to-number (cadr lines)))))
+	(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)
+	      (goto-char (org-element-property :begin reference))
+	      (when label
+		(forward-char 4)
+		(insert (format "%d-" id))
+		(and (not (eq footnote-type 'inline))
+		     (org-with-wide-buffer
+		      (org-footnote-goto-definition label)
+		      (beginning-of-line)
+		      (org-skip-whitespace)
+		      (forward-char 4)
+		      (insert (format "%d-" id))
+		      (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)))))))))))
+	(org-element-normalize-string (buffer-string))))
 
 (defun org-export-execute-babel-code ()
   "Execute every Babel code in the visible part of current buffer."
diff --git a/testing/lisp/test-ox.el b/testing/lisp/test-ox.el
index 9a0e787..140c0a8 100644
--- a/testing/lisp/test-ox.el
+++ b/testing/lisp/test-ox.el
@@ -919,6 +919,76 @@ 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
+   (= 3
+      (length
+       (delete-dups
+	(let ((contents "
+* foo
+Footnotes[fn:1]
+* bar
+Footnotes[fn:2], [fn:test], and [fn:inline:anonymous footnote]
+
+[fn:1] Footnote 1
+[fn:2] Footnote 1
+* Footnotes
+[fn:test] Footnote \"test\""))
+	  (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)))))))))))
+  ;; Footnotes within :lines are not collected in the Footnotes section
+  (should
+   (not (member org-footnote-section
+		(let ((contents "
+* foo
+Footnote[fn:test]
+* bar
+Footnote[fn:1], and [fn:inline:anonymous footnote]
+
+[fn:1] Footnote 1
+* Footnotes
+[fn:test] Footnote \"test\""))
+	    (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)
+		    'headline (lambda (elt) (org-element-property :title elt))))))))))
+  ;; Footnotes outside of scope are placed in Footnotes section.
+  (should
+   (let ((org-footnote-section "Footnotes")
+(contents "
+* foo
+Footnote[fn:1]
+* bar
+baz
+[fn:1] Footnote 1"))
+     (org-test-with-temp-text-in-file contents
+       (let ((file (buffer-file-name)))
+	 (org-test-with-temp-text
+	     (format "#+INCLUDE: \"%s::*foo\" " file)
+	   (org-export-expand-include-keyword)
+	   (<
+	    (org-element-map (org-element-parse-buffer)
+		'headline (lambda (head) (and (equal org-footnote-section
+						(org-element-property :raw-value head))
+					 (org-element-property :begin head)))
+		nil t)
+	    (org-element-map (org-element-parse-buffer)
+		'footnote-definition (lambda (def) (and (equal "fn:1-1"
+							  (org-element-property :label def))
+						   (org-element-property :begin def)))
+		nil t)))))))
   ;; If only-contents is non-nil only include contents of element.
   (should
    (equal
-- 
2.2.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-ox.el-Guess-the-minlevel-for-INCLUDE-keywords.patch --]
[-- Type: text/x-diff, Size: 1385 bytes --]

From f6426704852aa84f7b8fa4efda8560eb66a73a9a Mon Sep 17 00:00:00 2001
From: Rasmus <rasmus@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.
---
 lisp/ox.el | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lisp/ox.el b/lisp/ox.el
index 99c4e9b..cba624c 100644
--- a/lisp/ox.el
+++ b/lisp/ox.el
@@ -3066,6 +3066,18 @@ 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 "^[ \t]*#\\+INCLUDE: \\(.*\\)$" nil t)
+	(let ((matched (match-string 1)))
+	  (unless (or (string-match-p ":minlevel" matched)
+		      (string-match-p "\\(\\<src\\(?: +\\(.*\\)\\)?\\|\\<example\\>\\)" matched))
+	    (goto-char (line-end-position))
+	    (insert (format " :minlevel %d"
+			    (1+ (save-excursion
+				  (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))))
-- 
2.2.0

Re: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]

Rasmus <rasmus@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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-ox.el-Guess-the-minlevel-for-INCLUDE-keywords.patch --]
[-- Type: text/x-diff, Size: 3868 bytes --]

From 5ae354993662b3a3d2fc1c995861401f28b2af1c Mon Sep 17 00:00:00 2001
From: Rasmus <rasmus@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\n<point>#+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\n<point>#+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: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

Rasmus <rasmus@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: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

[-- Attachment #1: Type: text/plain, Size: 2510 bytes --]

Hi,

Thanks for the comments.  I've attached a new version.

Nicolas Goaziou <mail@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\n<point>#+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!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-ox.el-Guess-the-minlevel-for-INCLUDE-keywords.patch --]
[-- Type: text/x-diff, Size: 3879 bytes --]

From b7112471b3e4b5334d98caf528e1e687232dee2f Mon Sep 17 00:00:00 2001
From: Rasmus <rasmus@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)
+		(org-element-map (org-element-parse-buffer) 'headline
+		  (lambda (head) (org-element-property :level head))))))
+  ;; INCLUDE is automatically inserted for src elements.
+  (should
+   (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\n<point>#+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\n<point>#+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.1

Re: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

Rasmus <rasmus@gmx.us> writes:

> Thanks for the comments.  I've attached a new version.

Thanks.

> Nicolas Goaziou <mail@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: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

[-- Attachment #1: Type: text/plain, Size: 500 bytes --]

Nicolas Goaziou <mail@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!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-ox.el-Guess-the-minlevel-for-INCLUDE-keywords.patch --]
[-- Type: text/x-diff, Size: 4438 bytes --]

From 1fa88054255e66922ea9e2cd61310461901ac6ee Mon Sep 17 00:00:00 2001
From: Rasmus <rasmus@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\n<point>#+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\n<point>#+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: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

Rasmus <rasmus@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: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

Hi,

Nicolas Goaziou <mail@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\n<point>#+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\n<point>#+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: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

Hello,

Rasmus <rasmus@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: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

Hi,

Nicolas Goaziou <mail@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: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

[-- Attachment #1: Type: text/plain, Size: 589 bytes --]

Nicolas Goaziou <mail@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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-make-test-break.patch --]
[-- Type: text/x-diff, Size: 877 bytes --]

From e1bf99586e8e7484d34a35d24430cf5ab7b66911 Mon Sep 17 00:00:00 2001
From: Rasmus <rasmus@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\n<point>#+INCLUDE: \"%s/examples/include.org::#ah\"" org-test-dir)
      (narrow-to-region (point) (point-max))
      (org-export-expand-include-keyword)
-- 
2.2.1

Re: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

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,

Re: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

[-- 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

Re: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

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,

Re: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

[-- Attachment #1: Type: text/plain, Size: 3722 bytes --]

Hi,

Thanks again!

Nicolas Goaziou <mail@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

[-- 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: 8816 bytes --]

From d1e02f19e20341a650164f7bfb9aea97465225e1 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              | 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.  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.
+	      (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)))))))))))
 
 (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.
@@ -3242,11 +3254,14 @@ headline encountered.
 Optional argument MINLEVEL, when non-nil, is an integer
 specifying the level that any top-level headline in the included
 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
@@ -3308,19 +3323,46 @@ with footnotes is included in a document."
     ;; 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.
     (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)))
+	    (when (eq (org-element-type reference) 'footnote-reference)
+	      (let* ((label (org-element-property :label reference))
+		     (digit-label (and label (org-string-match-p "\\`[0-9]+\\'" label))))
+		;; Update the footnote-reference at point and collect
+		;; the new label, which is only used for footnotes
+		;; outsides LINES.
+		(goto-char (1+ (org-element-property :begin reference)))
+		(when label
+		  ;; If label is akin to [1] convert it to [fn:ID-1].
+		  ;; Otherwise add "ID-" after "fn:".
+		  (if digit-label (insert (format "fn:%d-" id))
+		    (forward-char 3)
+		    (insert (format "%d-" id)))
+		  (unless (eq (org-element-property :type reference) 'inline)
+		    (org-with-wide-buffer
+		     (let* ((definition (org-footnote-get-definition label))
+			    (beginning (copy-marker (nth 1 definition))))
+		       (goto-char (1+ beginning))
+		       ;; Update label cf. above.  Necessary when
+		       ;; including whole files.
+		       (if digit-label (insert (format "fn:%d-" id))
+			 (forward-char 3)
+			 (insert (format "%d-" id)))
+		       ;; Store footnotes outside of LINES.
+		       (when (or (< beginning marker-min) (> beginning marker-max))
+			 (puthash
+			  (buffer-substring (goto-char (1+ beginning))
+					    (1- (search-forward "]")))
+			  (org-element-normalize-string (nth 3 definition))
+			  footnotes))
+		       (set-marker beginning nil)))))))))
+	(set-marker marker-min nil)
+	(set-marker marker-max nil)))
     (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

Re: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

Thanks for the update.

Rasmus <rasmus@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: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]

Hi,

I fixed the nitpicks, but no major changes.

Nicolas Goaziou <mail@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

-- 
ツ

[-- 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: 8818 bytes --]

From 2382ee420c97a801560dff3e9bea343bca83dc13 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              | 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
 		       (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.
@@ -3242,11 +3253,14 @@ headline encountered.
 Optional argument MINLEVEL, when non-nil, is an integer
 specifying the level that any top-level headline in the included
 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
@@ -3307,20 +3321,46 @@ with footnotes is included in a document."
 			     (insert (make-string offset ?*)))))))))))
     ;; Append ID to all footnote references and definitions, so they
     ;; become file specific and cannot collide with footnotes in other
-    ;; included files.
+    ;; included files.  Further, collect relevant footnotes outside of
+    ;; LINES.
     (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)))
+	    (when (eq (org-element-type reference) 'footnote-reference)
+	      (let* ((label (org-element-property :label reference))
+		     (digit-label (and label (org-string-match-p "\\`[0-9]+\\'" label))))
+		;; Update the footnote-reference at point and collect
+		;; the new label, which is only used for footnotes
+		;; outsides LINES.
+		(when label
+		  ;; If label is akin to [1] convert it to [fn:ID-1].
+		  ;; Otherwise add "ID-" after "fn:".
+		  (goto-char (1+ (org-element-property :begin reference)))
+		  (if digit-label (insert (format "fn:%d-" id))
+		    (forward-char 3)
+		    (insert (format "%d-" id)))
+		  (unless (eq (org-element-property :type reference) 'inline)
+		    (org-with-wide-buffer
+		     (let* ((definition (org-footnote-get-definition label))
+			    (beginning (line-beginning-position)))
+		       (goto-char (1+ beginning))
+		       ;; Update label cf. above.  Necessary when
+		       ;; including whole files.
+		       (if digit-label (insert (format "fn:%d-" id))
+			 (forward-char 3)
+			 (insert (format "%d-" id)))
+		       ;; Store footnotes outside of LINES.
+		       (when (or (< beginning marker-min) (> beginning marker-max))
+			 (puthash
+			  (buffer-substring (goto-char (1+ beginning))
+					    (1- (search-forward "]")))
+			  (org-element-normalize-string (nth 3 definition))
+			  footnotes))))))))))
+	(set-marker marker-min nil)
+	(set-marker marker-max nil)))
     (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

Re: [bug, patch, ox] INCLUDE and footnotes

[Nicolas Goaziou]

Rasmus <rasmus@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: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

Hi,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Rasmus <rasmus@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: [bug, patch, ox] INCLUDE and footnotes

[Rasmus]

Hi,

Nicolas Goaziou <mail@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

[git-101] How to push a branch and avoid merge-message? (was: [bug, patch, ox] INCLUDE and footnotes)

[Rasmus]

Hi,

Rasmus <rasmus@gmx.us> writes:

> 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. . .

Hmm, inspecting cgit I see that one of those persky merge messages got up
there even though it wasn't in my local git log before pushing.  Sorry!

What is my mistake:

- I wrote the patch locally in a branch called include.
- I did git pull --rebase origin master before pushing.  I then did push
  origin master from my include branch.  I did not merge it into master
  exactly to avoid this merge-message...
- I know I can get a "clean" log by doing git format-patch and then
  git am'ing them into my master.  But this is tedious.

How can I develop on branch, retain history and not get the merge message?

Thanks,
Rasmus

PS: I did some searching on the webs beforehand, but apparently not well
    enough.

-- 
A clever person solves a problem. A wise person avoids it

Re: [git-101] How to push a branch and avoid merge-message?

[Nicolas Goaziou]

Hello,

Rasmus <rasmus@gmx.us> writes:

> Hmm, inspecting cgit I see that one of those persky merge messages got up
> there even though it wasn't in my local git log before pushing.
> Sorry!

No worries, this is not a problem.

> What is my mistake:
>
> - I wrote the patch locally in a branch called include.
> - I did git pull --rebase origin master before pushing.  I then did push
>   origin master from my include branch.  I did not merge it into master
>   exactly to avoid this merge-message...

I usually rebase to master just before pushing (i.e., R "master" from
"include" branch).


Regards,

-- 
Nicolas Goaziou