Re: Allow #+SETUPFILE to point to an URL for the org file

[Kaushal Modi <kaushal.modi@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1482 bytes --]

Hi Nicolas,

Thanks for one more round of detailed feedback.

I have attached the revised patch.

On Tue, Jun 13, 2017 at 8:43 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> It should be something like
>
>   * lisp/org.el (org--file-cache): New variable
>   (org-reset-file-cache):
>   (org-file-url-p): New function.
>   (org-mode-restart): Use new function.
>
> The purpose is to know in what commit the function was introduced, not
> what it does.
>

OK. Done.


> The last sentence is not correct. C-c C-c on any settings line resets
> the cache, not specifically on "#+SETUPFILE".
>

I have rephrased it.


> I'm worried about this test, and some other below. They require a proper
> internet access.


Makes sense.. While this feature does need internet access, running 'make
test' shouldn't.


> Couldn't we fake `url-retrieve-synchronously' using
> `cl-letf' so as to produce a buffer with appropriate contents and return
> it? Something (untested) like
>
>   (should
>    (string=
>     "foo"
>     (let ((buffer (generate-new-buffer "test-ox")))
>       (unwind-protect
>           (cl-letf (((symbol-function 'url-retrieve-synchronously)
>                      (lambda (&rest _)
>                        (with-current-buffer buffer (insert "HTTP 200
> OK\n\nfoo"))
>                        buffer)))
>             (org-file-contents "http://whatever"))
>         (kill-buffer buffer)))))
>

Thanks. I have used that snippet at applicable places.
-- 

Kaushal Modi

[-- Attachment #1.2: Type: text/html, Size: 2637 bytes --]

[-- Attachment #2: 0001-Allow-org-file-contents-to-fetch-file-contents-from-.patch --]
[-- Type: application/octet-stream, Size: 19591 bytes --]

From 1e92f5ed39541a473f295b414e8c89f6b0390f83 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Tue, 13 Jun 2017 11:41:14 -0400
Subject: [PATCH] Allow org-file-contents to fetch file contents from a URL

* lisp/org.el (org--file-cache): New variable.
(org-reset-file-cache):
(org-file-url-p): New function.
(org-mode-restart): Use new function.

* lisp/org.el (org-file-contents): Allow the FILE argument to be a
URL.  If the URL contents are already cached, return the cache
contents, else download the file and return contents of that.  The
file is automatically cached each time it is downloaded.  Add a new
optional argument NOCACHE.  If this is non-nil, the URL is always
downloaded afresh.  Use `org--file-cache' and `org-file-url-p'.

* lisp/org.el (org-edit-special): Do not allow editing the "file" if a
URL is specified for the "#+SETUPFILE".

* lisp/ox.el (org-export--list-bound-variables)
(org-export--prepare-file-contents):
* lisp/org-macro.el (org-macro--collect-macros) : Adapt to the
possibility that the input to `org-file-contents' can be a URL too.

* doc/org.texi (Export settings, In-buffer settings)
(The very busy C-c C-c key): Mention that #+SETUPFILE keyword can now
take a URL as a value, and that C-c C-c on the #+SETUPFILE line will
clear the org file cache.

* testing/lisp/test-org.el (test-org/org-file-contents-url)
(test-org/org-file-contents-file): Add tests for org-file-contents.

* testing/lisp/test-ox.el (test-org-export/get-inbuffer-options): Add
test for reading setupfile specified via a URL.
---
 doc/org.texi             | 38 +++++++++++++-----------
 etc/ORG-NEWS             | 12 +++++++-
 lisp/org-macro.el        | 22 ++++++++------
 lisp/org.el              | 74 +++++++++++++++++++++++++++++++++++++++--------
 lisp/ox.el               | 38 ++++++++++++++----------
 testing/lisp/test-org.el | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
 testing/lisp/test-ox.el  | 32 +++++++++++++++++++++
 7 files changed, 238 insertions(+), 53 deletions(-)

diff --git a/doc/org.texi b/doc/org.texi
index 83016282ca..36d6aa7aa3 100644
--- a/doc/org.texi
+++ b/doc/org.texi
@@ -10406,14 +10406,14 @@ override options set at a more general level.
 
 @cindex #+SETUPFILE
 In-buffer settings may appear anywhere in the file, either directly or
-indirectly through a file included using @samp{#+SETUPFILE: filename} syntax.
-Option keyword sets tailored to a particular back-end can be inserted from
-the export dispatcher (@pxref{The export dispatcher}) using the @code{Insert
-template} command by pressing @key{#}.  To insert keywords individually,
-a good way to make sure the keyword is correct is to type @code{#+} and then
-to use @kbd{M-@key{TAB}}@footnote{Many desktops intercept @kbd{M-TAB} to
-switch windows.  Use @kbd{C-M-i} or @kbd{@key{ESC} @key{TAB}} instead.} for
-completion.
+indirectly through a file included using @samp{#+SETUPFILE: filename or URL}
+syntax.  Option keyword sets tailored to a particular back-end can be
+inserted from the export dispatcher (@pxref{The export dispatcher}) using the
+@code{Insert template} command by pressing @key{#}.  To insert keywords
+individually, a good way to make sure the keyword is correct is to type
+@code{#+} and then to use @kbd{M-@key{TAB}}@footnote{Many desktops intercept
+@kbd{M-TAB} to switch windows.  Use @kbd{C-M-i} or @kbd{@key{ESC} @key{TAB}}
+instead.} for completion.
 
 The export keywords available for every back-end, and their equivalent global
 variables, include:
@@ -17174,14 +17174,16 @@ have a lower ASCII number than the lowest priority.
 This line sets a default inheritance value for entries in the current
 buffer, most useful for specifying the allowed values of a property.
 @cindex #+SETUPFILE
-@item #+SETUPFILE: file
-The setup file is for additional in-buffer settings.  Org loads this file and
-parses it for any settings in it only when Org opens the main file.  @kbd{C-c
-C-c} on the settings line will also parse and load.  Org also parses and
-loads the file during normal exporting process.  Org parses the contents of
-this file as if it was included in the buffer.  It can be another Org file.
-To visit the file, @kbd{C-c '} while the cursor is on the line with the file
-name.
+@item #+SETUPFILE: file or URL
+The setup file or a URL pointing to such file is for additional in-buffer
+settings.  Org loads this file and parses it for any settings in it only when
+Org opens the main file.  If URL is specified, the contents are downloaded
+and stored in a temporary file cache.  @kbd{C-c C-c} on the settings line
+will parse and load the file, and also reset the temporary file cache.  Org
+also parses and loads the document during normal exporting process.  Org
+parses the contents of this document as if it was included in the buffer.  It
+can be another Org file.  To visit the file (not a URL), @kbd{C-c '} while
+the cursor is on the line with the file name.
 @item #+STARTUP:
 @cindex #+STARTUP
 Startup options Org uses when first visiting a file.
@@ -17422,7 +17424,9 @@ If any highlights shown in the buffer from the creation of a sparse tree, or
 from clock display, remove such highlights.
 @item
 If the cursor is in one of the special @code{#+KEYWORD} lines, scan the
-buffer for these lines and update the information.
+buffer for these lines and update the information.  Also reset the Org file
+cache used to temporary store the contents of URLs used as values for
+keywords like @code{#+SETUPFILE}.
 @item
 If the cursor is inside a table, realign the table.  The table realigns even
 if automatic table editor is turned off.
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index eb0e11c4aa..83972d4e94 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -203,7 +203,7 @@ manual for details.
 **** Add global macros through ~org-export-global-macros~
 With this variable, one can define macros available for all documents.
 **** New keyword ~#+EXPORT_FILE_NAME~
-Simiralry to ~:EXPORT_FILE_NAME:~ property, this keyword allows the
+Similarly to ~:EXPORT_FILE_NAME:~ property, this keyword allows the
 user to specify the name of the output file upon exporting the
 document.  This also has an effect on publishing.
 **** Horizontal rules are no longer ignored in LaTeX table math mode
@@ -240,6 +240,16 @@ which causes refile targets to be prefixed with the buffer’s
 name. This is particularly useful when used in conjunction with
 ~uniquify.el~.
 
+*** ~org-file-contents~ now allows the FILE argument to be a URL.
+This allows ~#+SETUPFILE:~ to accept a URL instead of a local file
+path.  The URL contents are auto-downloaded and saved to a temporary
+cache ~org--file-cache~.  A new optional argument ~NOCACHE~ is added
+to ~org-file-contents~.
+
+*** ~org-mode-restart~ now resets the newly added ~org--file-cache~.
+Using ~C-c C-c~ on any keyword (like ~#+SETUPFILE~) will reset the
+that file cache.
+
 ** Removed functions
 
 *** Org Timeline
diff --git a/lisp/org-macro.el b/lisp/org-macro.el
index 6758d31f06..828c5e9e3d 100644
--- a/lisp/org-macro.el
+++ b/lisp/org-macro.el
@@ -55,7 +55,8 @@
 (declare-function org-element-macro-parser "org-element" ())
 (declare-function org-element-property "org-element" (property element))
 (declare-function org-element-type "org-element" (element))
-(declare-function org-file-contents "org" (file &optional noerror))
+(declare-function org-file-contents "org" (file &optional noerror nocache))
+(declare-function org-file-url-p "org" (file))
 (declare-function org-in-commented-heading-p "org" (&optional no-inheritance))
 (declare-function org-mode "org" ())
 (declare-function vc-backend "vc-hooks" (f))
@@ -102,16 +103,21 @@ Return an alist containing all macro templates found."
 				 (if old-cell (setcdr old-cell template)
 				   (push (cons name template) templates))))
 			   ;; Enter setup file.
-			   (let ((file (expand-file-name
-					(org-unbracket-string "\"" "\"" val))))
-			     (unless (member file files)
+			   (let* ((uri (org-unbracket-string "\"" "\"" (org-trim val)))
+				  (uri-is-url (org-file-url-p uri))
+				  (uri (if uri-is-url
+					   uri
+					 (expand-file-name uri))))
+			     ;; Avoid circular dependencies.
+			     (unless (member uri files)
 			       (with-temp-buffer
-				 (setq default-directory
-				       (file-name-directory file))
+				 (unless uri-is-url
+				   (setq default-directory
+					 (file-name-directory uri)))
 				 (org-mode)
-				 (insert (org-file-contents file 'noerror))
+				 (insert (org-file-contents uri 'noerror))
 				 (setq templates
-				       (funcall collect-macros (cons file files)
+				       (funcall collect-macros (cons uri files)
 						templates)))))))))))
 		templates))))
     (funcall collect-macros nil nil)))
diff --git a/lisp/org.el b/lisp/org.el
index 2101ec7d13..191990db75 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -181,6 +181,8 @@ Stars are put in group 1 and the trimmed body in group 2.")
 (declare-function org-export-get-environment "ox" (&optional backend subtreep ext-plist))
 (declare-function org-latex-make-preamble "ox-latex" (info &optional template snippet?))
 
+(defvar ffap-url-regexp)		;Silence byte-compiler
+
 (defsubst org-uniquify (list)
   "Non-destructively remove duplicate elements from LIST."
   (let ((res (copy-sequence list))) (delete-dups res)))
@@ -5280,17 +5282,62 @@ a string, summarizing TAGS, as a list of strings."
 	   (setq current-group (list tag))))
 	(_ nil)))))
 
-(defun org-file-contents (file &optional noerror)
-  "Return the contents of FILE, as a string."
-  (if (and file (file-readable-p file))
+(defvar org--file-cache (make-hash-table :test #'equal)
+  "Hash table to store contents of files referenced via a URL.
+This is the cache of file URLs read using `org-file-contents'.")
+
+(defun org-reset-file-cache ()
+  "Reset the cache of files downloaded by `org-file-contents'."
+  (clrhash org--file-cache))
+
+(defun org-file-url-p (file)
+  "Non-nil if FILE is a URL."
+  (require 'ffap)
+  (string-match-p ffap-url-regexp file))
+
+(defun org-file-contents (file &optional noerror nocache)
+  "Return the contents of FILE, as a string.
+
+FILE can be a file name or URL.
+
+If FILE is a URL, download the contents.  If the URL contents are
+already cached in the `org--file-cache' hash table, the download step
+is skipped.
+
+If NOERROR is non-nil, ignore the error when unable to read the FILE
+from file or URL.
+
+If NOCACHE is non-nil, do a fresh fetch of FILE even if cached version
+is available.  This option applies only if FILE is a URL."
+  (let* ((is-url (org-file-url-p file))
+         (cache (and is-url
+                     (not nocache)
+                     (gethash file org--file-cache))))
+    (cond
+     (cache)
+     (is-url
+      (with-current-buffer (url-retrieve-synchronously file)
+	(goto-char (point-min))
+	;; Move point to after the url-retrieve header.
+	(search-forward "\n\n" nil :move)
+	;; Search for the success code only in the url-retrieve header.
+	(if (save-excursion (re-search-backward "HTTP.*\\s-+200\\s-OK" nil :noerror))
+	    ;; Update the cache `org--file-cache' and return contents.
+	    (puthash file
+		     (buffer-substring-no-properties (point) (point-max))
+		     org--file-cache)
+	  (funcall (if noerror #'message #'user-error)
+		   "Unable to fetch file from %S"
+		   file))))
+     (t
       (with-temp-buffer
-	(insert-file-contents file)
-	(buffer-string))
-    (funcall (if noerror 'message 'error)
-	     "Cannot read file \"%s\"%s"
-	     file
-	     (let ((from (buffer-file-name (buffer-base-buffer))))
-	       (if from (concat " (referenced in file \"" from "\")") "")))))
+        (condition-case err
+	    (progn
+	      (insert-file-contents file)
+	      (buffer-string))
+	  (file-error
+           (funcall (if noerror #'message #'user-error)
+		    (error-message-string err)))))))))
 
 (defun org-extract-log-state-settings (x)
   "Extract the log state setting from a TODO keyword string.
@@ -20687,7 +20734,9 @@ Otherwise, return a user error."
 	    (format "[[%s]]"
 		    (expand-file-name
 		     (let ((value (org-element-property :value element)))
-		       (cond ((not (org-string-nw-p value))
+		       (cond ((org-file-url-p value)
+			      (user-error "The file is specified as a URL, cannot be edited"))
+			     ((not (org-string-nw-p value))
 			      (user-error "No file to edit"))
 			     ((string-match "\\`\"\\(.*?\\)\"" value)
 			      (match-string 1 value))
@@ -20951,7 +21000,8 @@ Use `\\[org-edit-special]' to edit table.el tables"))
     (funcall major-mode)
     (hack-local-variables)
     (when (and indent-status (not (bound-and-true-p org-indent-mode)))
-      (org-indent-mode -1)))
+      (org-indent-mode -1))
+    (org-reset-file-cache))
   (message "%s restarted" major-mode))
 
 (defun org-kill-note-or-show-branches ()
diff --git a/lisp/ox.el b/lisp/ox.el
index 53d35bba8f..3b793a00f2 100644
--- a/lisp/ox.el
+++ b/lisp/ox.el
@@ -1499,17 +1499,20 @@ Assume buffer is in Org mode.  Narrowing, if any, is ignored."
 			 (cond
 			  ;; Options in `org-export-special-keywords'.
 			  ((equal key "SETUPFILE")
-			   (let ((file
-				  (expand-file-name
-				   (org-unbracket-string "\"" "\"" (org-trim val)))))
+			   (let* ((uri (org-unbracket-string "\"" "\"" (org-trim val)))
+				  (uri-is-url (org-file-url-p uri))
+				  (uri (if uri-is-url
+					   uri
+					 (expand-file-name uri))))
 			     ;; Avoid circular dependencies.
-			     (unless (member file files)
+			     (unless (member uri files)
 			       (with-temp-buffer
-				 (setq default-directory
-				   (file-name-directory file))
-				 (insert (org-file-contents file 'noerror))
+				 (unless uri-is-url
+				   (setq default-directory
+					 (file-name-directory uri)))
+				 (insert (org-file-contents uri 'noerror))
 				 (let ((org-inhibit-startup t)) (org-mode))
-				 (funcall get-options (cons file files))))))
+				 (funcall get-options (cons uri files))))))
 			  ((equal key "OPTIONS")
 			   (setq plist
 				 (org-combine-plists
@@ -1647,17 +1650,22 @@ an alist where associations are (VARIABLE-NAME VALUE)."
 				      "BIND")
 			       (push (read (format "(%s)" val)) alist)
 			     ;; Enter setup file.
-			     (let ((file (expand-file-name
-					  (org-unbracket-string "\"" "\"" val))))
-			       (unless (member file files)
+			     (let* ((uri (org-unbracket-string "\"" "\"" val))
+				    (uri-is-url (org-file-url-p uri))
+				    (uri (if uri-is-url
+					     uri
+					   (expand-file-name uri))))
+			       ;; Avoid circular dependencies.
+			       (unless (member uri files)
 				 (with-temp-buffer
-				   (setq default-directory
-					 (file-name-directory file))
+				   (unless uri-is-url
+				     (setq default-directory
+					   (file-name-directory uri)))
 				   (let ((org-inhibit-startup t)) (org-mode))
-				   (insert (org-file-contents file 'noerror))
+				   (insert (org-file-contents uri 'noerror))
 				   (setq alist
 					 (funcall collect-bind
-						  (cons file files)
+						  (cons uri files)
 						  alist))))))))))
 		   alist)))))
       ;; Return value in appropriate order of appearance.
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index e55ee077b8..35674baf4a 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -6498,6 +6498,81 @@ Paragraph<point>"
      (org-show-set-visibility 'minimal)
      (org-invisible-p2))))
 
+(ert-deftest test-org/org-file-contents-file ()
+  "Test `org-file-contents' with a file as input."
+  (should
+   (string= "#+BIND: variable value
+#+DESCRIPTION: l2
+#+LANGUAGE: en
+#+SELECT_TAGS: b
+#+TITLE: b
+#+PROPERTY: a 1
+" (org-file-contents (expand-file-name "setupfile3.org"
+				       (concat org-test-dir "examples/")))))
+  
+  (let ((invalid-file "this-file-must-not-exist"))
+    ;; Throw error when trying to access an invalid file.
+    (should-error
+     (org-file-contents invalid-file))
+    ;; Try to access an invalid file, but do not throw an error.
+    (should
+     (string-match-p "\\`Opening input file: No such file or directory"
+		     (org-file-contents invalid-file :noerror)))))
+
+(ert-deftest test-org/org-file-contents-url ()
+  "Test `org-file-contents' with a URL as input."
+  (should
+   (string= "foo"
+	    (let ((buffer (generate-new-buffer "url-retrieve-output")))
+	      (unwind-protect
+		  ;; Simulate successful retrieval of a URL.
+		  (cl-letf (((symbol-function 'url-retrieve-synchronously)
+			     (lambda (&rest_)
+			       (with-current-buffer buffer
+				 (insert "HTTP/1.1 200 OK\n\nfoo"))
+			       buffer)))
+		    (org-file-contents "http://some-valid-url"))
+		(kill-buffer buffer)))))
+
+  (let ((invalid-url "http://this-url-must-not-exist"))
+    ;; Throw error when trying to access an invalid URL.
+    (should-error
+     (let ((buffer (generate-new-buffer "url-retrieve-output")))
+       (unwind-protect
+	   ;; Simulate unsuccessful retrieval of a URL.
+	   (cl-letf (((symbol-function 'url-retrieve-synchronously)
+		      (lambda (&rest_)
+			(with-current-buffer buffer
+			  (insert "HTTP/1.1 404 Not found\n\ndoes not matter"))
+			buffer)))
+	     (org-file-contents invalid-url))
+	 (kill-buffer buffer))))
+    ;; Try to access an invalid URL, but do not throw an error.
+    (should-error
+     (let ((buffer (generate-new-buffer "url-retrieve-output")))
+       (unwind-protect
+	   ;; Simulate unsuccessful retrieval of a URL.
+	   (cl-letf (((symbol-function 'url-retrieve-synchronously)
+		      (lambda (&rest_)
+			(with-current-buffer buffer
+			  (insert "HTTP/1.1 404 Not found\n\ndoes not matter"))
+			buffer)))
+	     (org-file-contents invalid-url))
+	 (kill-buffer buffer))))
+    (should
+     (string=
+      (format "Unable to fetch file from \"%s\"" invalid-url)
+      (let ((buffer (generate-new-buffer "url-retrieve-output")))
+	(unwind-protect
+	    ;; Simulate unsuccessful retrieval of a URL.
+	    (cl-letf (((symbol-function 'url-retrieve-synchronously)
+		       (lambda (&rest_)
+			 (with-current-buffer buffer
+			   (insert "HTTP/1.1 404 Not found\n\ndoes not matter"))
+			 buffer)))
+	      (org-file-contents invalid-url :noerror))
+	  (kill-buffer buffer)))))))
+
 
 (provide 'test-org)
 
diff --git a/testing/lisp/test-ox.el b/testing/lisp/test-ox.el
index 69a778bbb3..72b6c8ccd1 100644
--- a/testing/lisp/test-ox.el
+++ b/testing/lisp/test-ox.el
@@ -232,6 +232,38 @@ num:2 <:active")))
 		org-test-dir)
       (org-export--get-inbuffer-options))
     '(:language "fr" :select-tags ("a" "b" "c") :title ("a b c"))))
+  ;; Options set through SETUPFILE specified using a URL.
+  (let ((buffer (generate-new-buffer "url-retrieve-output")))
+    (unwind-protect
+	;; Simulate successful retrieval of a setupfile from URL.
+	(cl-letf (((symbol-function 'url-retrieve-synchronously)
+		   (lambda (&rest_)
+		     (with-current-buffer buffer
+		       (insert "HTTP/1.1 200 OK
+
+# Contents of http://link-to-my-setupfile.org
+#+BIND: variable value
+#+DESCRIPTION: l2
+#+LANGUAGE: en
+#+SELECT_TAGS: b
+#+TITLE: b
+#+PROPERTY: a 1
+"))
+		     buffer)))
+	  (should
+	   (equal
+	    (org-test-with-temp-text
+		"#+DESCRIPTION: l1
+#+LANGUAGE: es
+#+SELECT_TAGS: a
+#+TITLE: a
+#+SETUPFILE: \"http://link-to-my-setupfile.org\"
+#+LANGUAGE: fr
+#+SELECT_TAGS: c
+#+TITLE: c"
+	      (org-export--get-inbuffer-options))
+	    '(:language "fr" :select-tags ("a" "b" "c") :title ("a b c")))))
+      (kill-buffer buffer)))
   ;; More than one property can refer to the same buffer keyword.
   (should
    (equal '(:k2 "value" :k1 "value")
-- 
2.13.0