emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Allow #+SETUPFILE to point to an URL for the org file
@ 2016-12-03 17:23 Kaushal Modi
  2016-12-08 11:51 ` Kaushal Modi
  0 siblings, 1 reply; 25+ messages in thread
From: Kaushal Modi @ 2016-12-03 17:23 UTC (permalink / raw)
  To: emacs-org list

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

Hello,

I would like to put my common org config in github so that I can access it
from anywhere/any machine.

Currently I have

#+SETUPFILE: ~/org/common/config.org

I would like to do something like

#+SETUPFILE: https://cdn.rawgit.com/path/to/config.org

I can probably add an async shell execution to wget that link, save it to
/tmp/config.org and then replace

#+SETUPFILE: https://cdn.rawgit.com/path/to/config.org

with

#+SETUPFILE: /tmp/config.org

in, may be, org-export-before-processing-hook?

But has someone already implemented something like this?

Thanks.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1338 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2016-12-03 17:23 Allow #+SETUPFILE to point to an URL for the org file Kaushal Modi
@ 2016-12-08 11:51 ` Kaushal Modi
  2016-12-08 14:22   ` John Kitchin
  2016-12-08 14:31   ` Nicolas Goaziou
  0 siblings, 2 replies; 25+ messages in thread
From: Kaushal Modi @ 2016-12-08 11:51 UTC (permalink / raw)
  To: emacs-org list

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

Anyone?

On Sat, Dec 3, 2016, 12:23 PM Kaushal Modi <kaushal.modi@gmail.com> wrote:

> Hello,
>
> I would like to put my common org config in github so that I can access it
> from anywhere/any machine.
>
> Currently I have
>
> #+SETUPFILE: ~/org/common/config.org
>
> I would like to do something like
>
> #+SETUPFILE: https://cdn.rawgit.com/path/to/config.org
>
> I can probably add an async shell execution to wget that link, save it to
> /tmp/config.org and then replace
>
> #+SETUPFILE: https://cdn.rawgit.com/path/to/config.org
>
> with
>
> #+SETUPFILE: /tmp/config.org
>
> in, may be, org-export-before-processing-hook?
>
> But has someone already implemented something like this?
>
> Thanks.
> --
>
> Kaushal Modi
>
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 2829 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2016-12-08 11:51 ` Kaushal Modi
@ 2016-12-08 14:22   ` John Kitchin
  2016-12-08 14:31   ` Nicolas Goaziou
  1 sibling, 0 replies; 25+ messages in thread
From: John Kitchin @ 2016-12-08 14:22 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: emacs-org list

+1. I would use it like an xml dtd if I could ;).


Kaushal Modi writes:

> Anyone?
>
> On Sat, Dec 3, 2016, 12:23 PM Kaushal Modi <kaushal.modi@gmail.com> wrote:
>
>> Hello,
>>
>> I would like to put my common org config in github so that I can access it
>> from anywhere/any machine.
>>
>> Currently I have
>>
>> #+SETUPFILE: ~/org/common/config.org
>>
>> I would like to do something like
>>
>> #+SETUPFILE: https://cdn.rawgit.com/path/to/config.org
>>
>> I can probably add an async shell execution to wget that link, save it to
>> /tmp/config.org and then replace
>>
>> #+SETUPFILE: https://cdn.rawgit.com/path/to/config.org
>>
>> with
>>
>> #+SETUPFILE: /tmp/config.org
>>
>> in, may be, org-export-before-processing-hook?
>>
>> But has someone already implemented something like this?
>>
>> Thanks.
>> --
>>
>> Kaushal Modi
>>


-- 
Professor John Kitchin
Doherty Hall A207F
Department of Chemical Engineering
Carnegie Mellon University
Pittsburgh, PA 15213
412-268-7803
@johnkitchin
http://kitchingroup.cheme.cmu.edu

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2016-12-08 11:51 ` Kaushal Modi
  2016-12-08 14:22   ` John Kitchin
@ 2016-12-08 14:31   ` Nicolas Goaziou
  2016-12-08 14:44     ` Kaushal Modi
  1 sibling, 1 reply; 25+ messages in thread
From: Nicolas Goaziou @ 2016-12-08 14:31 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: emacs-org list

Hello,

Kaushal Modi <kaushal.modi@gmail.com> writes:

> Anyone?

Apparently not.

Note that it could drastically slow down opening buffers with a mediocre
connection since SETUPFILE is read every time buffer set-up is
refreshed, e.g., when the buffer is opened.

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2016-12-08 14:31   ` Nicolas Goaziou
@ 2016-12-08 14:44     ` Kaushal Modi
  2016-12-08 21:48       ` Nicolas Goaziou
  0 siblings, 1 reply; 25+ messages in thread
From: Kaushal Modi @ 2016-12-08 14:44 UTC (permalink / raw)
  To: emacs-org list

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

On Thu, Dec 8, 2016 at 9:31 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Hello,
>
> Kaushal Modi <kaushal.modi@gmail.com> writes:
>
> > Anyone?
>
> Apparently not.
>
> Note that it could drastically slow down opening buffers with a mediocre
> connection since SETUPFILE is read every time buffer set-up is
> refreshed, e.g., when the buffer is opened.
>
>
Here are a couple of ideas:

- Let's assume that if the current file name is /path/to/foo.org, the
SETUPFILE is always downloaded to /tmp/path_to_foo_config.org for brevity.
The function that sets that temp file should be a defcustom.
- Now, the referenced SETUPFILE should be downloaded only if that (1) That
file is being fetched for the first time in that emacs session, or (2) that
temp file does not exist.
- Add a defun to force reload the SETUPFILE from the referenced URL, in
which case the temp file will be deleted and re-downloaded (as the above
condition satisfied).

So under the normal circumstance where that foo.org file buffer is reverted
multiple times in an emacs session, the same SETUPFILE downloaded to /tmp
will be used. If the user updated the file at the referenced URL, they can
do the above mentioned forced reload of SETUPFILE and download the latest
version of SETUPFILE.

Thoughts?
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 2187 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2016-12-08 14:44     ` Kaushal Modi
@ 2016-12-08 21:48       ` Nicolas Goaziou
  2016-12-08 22:07         ` Kaushal Modi
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Goaziou @ 2016-12-08 21:48 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: emacs-org list

Kaushal Modi <kaushal.modi@gmail.com> writes:

> Here are a couple of ideas:
>
> - Let's assume that if the current file name is /path/to/foo.org, the
> SETUPFILE is always downloaded to /tmp/path_to_foo_config.org for brevity.
> The function that sets that temp file should be a defcustom.

What about storing the contents of the file in a variable instead of
cluttering the temp directory?

> - Now, the referenced SETUPFILE should be downloaded only if that (1) That
> file is being fetched for the first time in that emacs session, or (2) that
> temp file does not exist.

And (3) it isn't local?

> - Add a defun to force reload the SETUPFILE from the referenced URL, in
> which case the temp file will be deleted and re-downloaded (as the above
> condition satisfied).
>
> So under the normal circumstance where that foo.org file buffer is reverted
> multiple times in an emacs session, the same SETUPFILE downloaded to /tmp
> will be used. If the user updated the file at the referenced URL, they can
> do the above mentioned forced reload of SETUPFILE and download the latest
> version of SETUPFILE.
>
> Thoughts?

It could work. Do you want to provide an implementation?

Regards,

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2016-12-08 21:48       ` Nicolas Goaziou
@ 2016-12-08 22:07         ` Kaushal Modi
  2016-12-08 22:40           ` Nicolas Goaziou
  0 siblings, 1 reply; 25+ messages in thread
From: Kaushal Modi @ 2016-12-08 22:07 UTC (permalink / raw)
  To: emacs-org list

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

On Thu, Dec 8, 2016 at 4:48 PM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Kaushal Modi <kaushal.modi@gmail.com> writes:
>
> What about storing the contents of the file in a variable instead of
> cluttering the temp directory?
>

Hmm, is there a way to read a file from a URL to a variable directly? Or
did you mean to download the file first, read that into a temp buffer and
then delete the temp file?


>
> > - Now, the referenced SETUPFILE should be downloaded only if that (1)
> That
> > file is being fetched for the first time in that emacs session, or (2)
> that
> > temp file does not exist.
>
> And (3) it isn't local?
>

This proposal was for the case where we have

#+SETUPFILE: http://foo.bar/config.org

So it cannot be local to begin with.

With respect to the point about not having the file in temp, we can have a
flag that if set, will prevent re-downloading of the file. User can choose
to reset that flag and then re-download that file. This will be lieu of the
earlier condition "(2) that temp file does not exist."


>
> > - Add a defun to force reload the SETUPFILE from the referenced URL, in
> > which case the temp file will be deleted and re-downloaded (as the above
> > condition satisfied).
> >
> > So under the normal circumstance where that foo.org file buffer is
> reverted
> > multiple times in an emacs session, the same SETUPFILE downloaded to /tmp
> > will be used. If the user updated the file at the referenced URL, they
> can
> > do the above mentioned forced reload of SETUPFILE and download the latest
> > version of SETUPFILE.
> >
> > Thoughts?
>
> It could work. Do you want to provide an implementation?
>

I would like to work on this. But I will be away from my computer for about
a month starting tomorrow. Will get back to this once I am back from my
vacation.

Thank you for the feedback.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 3361 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2016-12-08 22:07         ` Kaushal Modi
@ 2016-12-08 22:40           ` Nicolas Goaziou
  2017-03-13 17:37             ` Kaushal Modi
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Goaziou @ 2016-12-08 22:40 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: emacs-org list

Kaushal Modi <kaushal.modi@gmail.com> writes:

> Hmm, is there a way to read a file from a URL to a variable directly? Or
> did you mean to download the file first, read that into a temp buffer and
> then delete the temp file?

You can use something like `url-insert' and `url-retrieve' or
`url-retrieve-synchronously'.

> This proposal was for the case where we have
>
> #+SETUPFILE: http://foo.bar/config.org
>
> So it cannot be local to begin with.

Sure, but SETUPFILE still accepts local file names. So I was pointing
out that you need to check if the URL is a local file name before
proceeding. In particular, this check needs to happen when using "C-c '"
(which may display URL contents in a read-only buffer, BTW).

> With respect to the point about not having the file in temp, we can have a
> flag that if set, will prevent re-downloading of the file. User can choose
> to reset that flag and then re-download that file. This will be lieu of the
> earlier condition "(2) that temp file does not exist."

Is it necessary? File contents could be stored in, e.g., a hash table,
url being the key. The file is downloaded only if the entry doesn't
exist in the table and the user didn't force download.-

> I would like to work on this. But I will be away from my computer for about
> a month starting tomorrow. Will get back to this once I am back from my
> vacation.

Great. Thank you.

Regards,

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2016-12-08 22:40           ` Nicolas Goaziou
@ 2017-03-13 17:37             ` Kaushal Modi
  2017-03-30  7:43               ` Nicolas Goaziou
  0 siblings, 1 reply; 25+ messages in thread
From: Kaushal Modi @ 2017-03-13 17:37 UTC (permalink / raw)
  To: emacs-org list, Nicolas Goaziou

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

On Thu, Dec 8, 2016 at 5:40 PM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Kaushal Modi <kaushal.modi@gmail.com> writes:
>
> You can use something like `url-insert' and `url-retrieve' or
> `url-retrieve-synchronously'.
>

Thanks. I am using url-retrieve-synchronously.

>
> Sure, but SETUPFILE still accepts local file names. So I was pointing
> out that you need to check if the URL is a local file name before
> proceeding. In particular, this check needs to happen when using "C-c '"
> (which may display URL contents in a read-only buffer, BTW).
>

Understood. I might need some help when baking this into org.el,
org-macros.el, etc.


>
> Is it necessary? File contents could be stored in, e.g., a hash table,
> url being the key. The file is downloaded only if the entry doesn't
> exist in the table and the user didn't force download.-
>

Correct. Thanks for the idea. I am now using hash table for this.

Here is my implementation.. it is still not baked into org.el, etc and
provided as a complete patch; I have some questions..

=====
(defvar org-setupfile-ht (make-hash-table :test 'equal)
  "Hash table to store the org SETUPFILE.
This acts as a cache of setup files read using
`org-insert-setupfile-contents'.")

(defun org-setupfile-clear-cache ()
  "Clear the SETUPFILE cache stored in `org-setupfile-ht'."
  (interactive)
  (clrhash org-setupfile-ht))

(defun org-insert-setupfile-contents (setupfile &optional nocache noerror)
  "Insert the contents of SETUPFILE.
SETUPFILE can be a file path or URL.

If SETUPFILE is a file path, use `org-file-contents' to get the file
contents.
If it is a URL instead, download the contents. If the URL contents are
already
cached in the `org-setupfile-ht' hash table, the download step is skipped.

If NOCACHE is non-nil, do a fresh fetch of SETUPFILE even if cached version
is
available. This option applies only if SETUPFILE is a URL.

If NOERROR is non-nil, ignore the error when unable to read the SETUPFILE
from
file or URL."
  (require 'ffap)                       ;for `ffap-url-regexp'
  (let* ((is-url (string-match-p ffap-url-regexp setupfile))
         (cache (when (and is-url (not nocache))
                  (gethash setupfile org-setupfile-ht)))
         (contents (when (and is-url cache) cache)))
    (if is-url
        (unless cache
          (let (url-retrieve-header)
            (with-current-buffer (url-retrieve-synchronously setupfile)
              (goto-char (point-min))
              ;; Take point to after the url-retrieve header
              (re-search-forward "\n\n") ; 2 consecutive new-line chars
              (setq url-retrieve-header (buffer-substring-no-properties
                                         (point-min) (point)))
              (message url-retrieve-header) ;Dump the URL retrieve header
to *Messages*
              (if (string-match-p "HTTP.*\\s-+200\\s-OK"
url-retrieve-header) ;URL retrieved correctly
                  (progn
                    (setq contents (buffer-substring-no-properties (point)
(point-max)))
                    ;; Update the cache `org-setupfile-ht'
                    (puthash setupfile contents org-setupfile-ht))
                (funcall (if noerror 'message 'error)
                         "Unable to fetch SETUPFILE from `%s'"
setupfile)))))
      (setq contents (org-file-contents setupfile noerror)))
    (when contents
      (save-excursion
        (insert contents)))))
=====

Question:

- All the places where the content of SETUPFILE is inserted in a temp
buffer, it is assumed that the file is retrieved from disk and not from URL.

Example in ox.el:

 ((equal key "SETUPFILE")
  (let ((file
 (expand-file-name
  (org-unbracket-string "\"" "\"" (org-trim val)))))
    ;; Avoid circular dependencies.
    (unless (member file files)
      (with-temp-buffer
(setq default-directory
  (file-name-directory file))
(insert (org-file-contents file 'noerror))
(let ((org-inhibit-startup t)) (org-mode))
(funcall get-options (cons file files))))))


Note the use of expand-file-name, (member file files), default-directory,
(funcall get-options (cons file files)).

How do we deal with those parts of the code when the 'file' is a URL.

Using my implementation above,

    (org-insert-setupfile-contents "/file/path/or/url" nil :noerror)

works the same way as

    (insert (org-file-contents "/file/path" 'noerror))

So org-insert-setupfile-contents can replace (insert (org-file-contents
..)) easily. The unknown is how to deal with the surrounding code that
deals with

> expand-file-name, (member file files), default-directory, (funcall
get-options (cons file files)).

Here's a similar code snippet around setupfile insertion in ox.el again, in
org-export--list-bound-variables:

=====
    ;; Enter setup file.
    (let ((file (expand-file-name
 (org-unbracket-string "\"" "\"" val))))
      (unless (member file files)
(with-temp-buffer
  (setq default-directory
(file-name-directory file))
  (let ((org-inhibit-startup t)) (org-mode))
  (insert (org-file-contents file 'noerror))
  (setq alist
(funcall collect-bind
 (cons file files)
 alist))))))))))
=====




-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 9864 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-03-13 17:37             ` Kaushal Modi
@ 2017-03-30  7:43               ` Nicolas Goaziou
  2017-05-23 19:07                 ` Kaushal Modi
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Goaziou @ 2017-03-30  7:43 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: emacs-org list

Hello,

Kaushal Modi <kaushal.modi@gmail.com> writes:

> Here is my implementation.. it is still not baked into org.el, etc and
> provided as a complete patch; I have some questions..

Thank you.

> (defvar org-setupfile-ht (make-hash-table :test 'equal)

org-setupfile-ht -> org--setupfile-cache

if it is meant to be inserted in "org.el" proper, or
`org-setupfile--cache' if you want to create a new "org-setupfile.el"
library.

Nitpick : 'equal -> #'equal

>   "Hash table to store the org SETUPFILE.

Hash table to store SETUPFILE contents.

> This acts as a cache of setup files read using
> `org-insert-setupfile-contents'.")
>
> (defun org-setupfile-clear-cache ()

`org--setupfile-clear-cache' or `org-setupfile--clear-cache' depending
on the location of the function.

>   "Clear the SETUPFILE cache stored in `org-setupfile-ht'."
>   (interactive)
>   (clrhash org-setupfile-ht))
>
> (defun org-insert-setupfile-contents (setupfile &optional nocache noerror)
>   "Insert the contents of SETUPFILE.
> SETUPFILE can be a file path or URL.

file path -> file name

> If SETUPFILE is a file path, use `org-file-contents' to get the file
> contents.

Then, we might want to generalize `org-file-contents' instead (i.e., let
`org-file-contents' handle remote locations). WDYT?

> If it is a URL instead, download the contents. If the URL contents are
> already
> cached in the `org-setupfile-ht' hash table, the download step is
> skipped.

Mind the double spaces at the end of sentences.

> If NOCACHE is non-nil, do a fresh fetch of SETUPFILE even if cached version
> is
> available. This option applies only if SETUPFILE is a URL.
>
> If NOERROR is non-nil, ignore the error when unable to read the SETUPFILE
> from
> file or URL."
>   (require 'ffap)                       ;for `ffap-url-regexp'
>   (let* ((is-url (string-match-p ffap-url-regexp setupfile))

They are not equivalent, but could `org-file-remote-p', or
`file-remote-p' be used instead?

>          (cache (when (and is-url (not nocache))
>                   (gethash setupfile org-setupfile-ht)))

  (cache (and is-url (not nocache) (gethash setupfile org-setupfile-ht)))

>          (contents (when (and is-url cache) cache)))
>     (if is-url
>         (unless cache
>           (let (url-retrieve-header)
>             (with-current-buffer (url-retrieve-synchronously setupfile)
>               (goto-char (point-min))
>               ;; Take point to after the url-retrieve header
>               (re-search-forward "\n\n") ; 2 consecutive new-line chars

`re-search-forward' -> `search-forward'

>               (setq url-retrieve-header (buffer-substring-no-properties
>                                          (point-min) (point)))
>               (message url-retrieve-header) ;Dump the URL retrieve header
> to *Messages*
>               (if (string-match-p "HTTP.*\\s-+200\\s-OK"
> url-retrieve-header) ;URL retrieved correctly
>                   (progn
>                     (setq contents (buffer-substring-no-properties (point)
> (point-max)))
>                     ;; Update the cache `org-setupfile-ht'
>                     (puthash setupfile contents org-setupfile-ht))
>                 (funcall (if noerror 'message 'error)

  (if noerror #'message #'error)

>                          "Unable to fetch SETUPFILE from `%s'"

  `%s' -> %S

> setupfile)))))
>       (setq contents (org-file-contents setupfile noerror)))

I think it is clearer if wrapped like this:

  (contents
   (cond (cache)
         (is-url
          (let (url-retrieve-header)
            ...))
         (t (org-file-contents setupfile noerror))))

>     (when contents
>       (save-excursion
>         (insert contents)))))

This may not be necessary at this point if we merge `org-file-contents'
with the above.

> Question:
>
> - All the places where the content of SETUPFILE is inserted in a temp
> buffer, it is assumed that the file is retrieved from disk and not from URL.
>
> Example in ox.el:
>
>  ((equal key "SETUPFILE")
>   (let ((file
>  (expand-file-name
>   (org-unbracket-string "\"" "\"" (org-trim val)))))
>     ;; Avoid circular dependencies.
>     (unless (member file files)
>       (with-temp-buffer
> (setq default-directory
>   (file-name-directory file))
> (insert (org-file-contents file 'noerror))
> (let ((org-inhibit-startup t)) (org-mode))
> (funcall get-options (cons file files))))))
>
>
> Note the use of expand-file-name, (member file files), default-directory,
> (funcall get-options (cons file files)).

(member file files), (cons file files) and `expand-file-name' are used
to avoid reading over and over the same setup file. In particular, they
prevent circular dependencies.

You can ignore `expand-file-name' and replace `file' with `uri', i.e.,
it is straightforward to extend the code to remote file names.

`default-directory' is slightly more tricky, as it is used to properly
read recursively setup files with relative file names. I think our best
bet is to to check if current file name is local or remote, and ignore
`default-directory' setting in the latter case.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-03-30  7:43               ` Nicolas Goaziou
@ 2017-05-23 19:07                 ` Kaushal Modi
  2017-05-25 10:13                   ` Nicolas Goaziou
  0 siblings, 1 reply; 25+ messages in thread
From: Kaushal Modi @ 2017-05-23 19:07 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-org list


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

Thanks for the detailed review.

I have attached a patch following your recommendation. Please review it.

Here's a MWE to show the use of this new feature:

=====
#+SETUPFILE:
https://cdn.rawgit.com/kaushalmodi/.emacs.d/master/misc/org-setupfile.org
#+TITLE: Heading{{{NEWLINE}}}Sub-heading
=====

(Note that NEWLINE is not an inbuilt macro.)

On Thu, Mar 30, 2017 at 5:06 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> org-setupfile-ht -> org--setupfile-cache
>
> if it is meant to be inserted in "org.el" proper, or
> `org-setupfile--cache' if you want to create a new "org-setupfile.el"
> library.
>

I ended up updating org-file-contents. So now that is org--file-cache.


> Nitpick : 'equal -> #'equal
>

Done.


> Hash table to store SETUPFILE contents.
>

Done.


> `org--setupfile-clear-cache' or `org-setupfile--clear-cache' depending
> on the location of the function.
>

This is an interactive, a user-facing function; do we want to add
double-dashes in that name? The function is now called org-file-clear-cache.


> file path -> file name
>

Done.


> > If SETUPFILE is a file path, use `org-file-contents' to get the file
> > contents.
>
> Then, we might want to generalize `org-file-contents' instead (i.e., let
> `org-file-contents' handle remote locations). WDYT?
>

That was my first thought, but was leery on modifying the
org-file-contents. I have now done that.


> Mind the double spaces at the end of sentences.
>

Done.


> They are not equivalent, but could `org-file-remote-p', or
> `file-remote-p' be used instead?
>

I tried (org-file-remote-p "http://foo.bar") but it returned nil. Looks
like both org-file-remote-p and file-remote-p will not work for URLs.


> >          (cache (when (and is-url (not nocache))
> >                   (gethash setupfile org-setupfile-ht)))
>
>   (cache (and is-url (not nocache) (gethash setupfile org-setupfile-ht)))
>

Of course :)


> >               (re-search-forward "\n\n") ; 2 consecutive new-line chars
> `re-search-forward' -> `search-forward'
>

Correct, regexp is not needed in that case.


> >                 (funcall (if noerror 'message 'error)
>   (if noerror #'message #'error)
>

Done.


>
> >                          "Unable to fetch SETUPFILE from `%s'"
>
>   `%s' -> %S
>

setupfile is always a string so I used %s. If setupfile is not a string
(may be nil?), then the very first string-match-p will through an error. Is
there a specific reason for using %S? I did not use %S because I did not
want the double-quotes to be printed around the string in the echo area.


> > setupfile)))))
> >       (setq contents (org-file-contents setupfile noerror)))
>
> I think it is clearer if wrapped like this:
>
>   (contents
>    (cond (cache)
>          (is-url
>           (let (url-retrieve-header)
>             ...))
>          (t (org-file-contents setupfile noerror))))
>

That indeed is pretty sweet. I have made that change.


> >     (when contents
> >       (save-excursion
> >         (insert contents)))))
>
> This may not be necessary at this point if we merge `org-file-contents'
> with the above.
>

Correct. The attached patch has everything integrated in org-file-contents.


> > Question:
> >
> > - All the places where the content of SETUPFILE is inserted in a temp
> > buffer, it is assumed that the file is retrieved from disk and not from
> URL.
> >
> > Example in ox.el:
> >
> >  ((equal key "SETUPFILE")
> >   (let ((file
> >  (expand-file-name
> >   (org-unbracket-string "\"" "\"" (org-trim val)))))
> >     ;; Avoid circular dependencies.
> >     (unless (member file files)
> >       (with-temp-buffer
> > (setq default-directory
> >   (file-name-directory file))
> > (insert (org-file-contents file 'noerror))
> > (let ((org-inhibit-startup t)) (org-mode))
> > (funcall get-options (cons file files))))))
> >
> >
> > Note the use of expand-file-name, (member file files), default-directory,
> > (funcall get-options (cons file files)).
>
> (member file files), (cons file files) and `expand-file-name' are used
> to avoid reading over and over the same setup file. In particular, they
> prevent circular dependencies.
>
> You can ignore `expand-file-name' and replace `file' with `uri', i.e.,
> it is straightforward to extend the code to remote file names.
>
> `default-directory' is slightly more tricky, as it is used to properly
> read recursively setup files with relative file names. I think our best
> bet is to to check if current file name is local or remote, and ignore
> `default-directory' setting in the latter case.
>

Do we need to update the code using org-file-contents in these places:

lisp/org-capture.el
692:      (setq txt (org-file-contents file))

lisp/ox-man.el
519:              (setq code-block  (org-file-contents out-file))
764:        (setq code-block  (org-file-contents out-file))

contrib/lisp/ox-groff.el
1084:              (setq code-block  (org-file-contents out-file))
1521:          (setq code-block  (org-file-contents out-file))
-- 

Kaushal Modi

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

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

From 93fdbc96527b63b3ceb9c25708958b9accd66c06 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Tue, 23 May 2017 13:40:20 -0400
Subject: [PATCH] Allow org-file-contents to fetch file contents from a URL

* lisp/org.el (org--file-cache): New internal variable to store
downloaded files' cache.

* lisp/org.el (org-file-clear-cache): New interactive function to
clear the above file cache.

* lisp/org.el (org-file-url-p): New function to test if the input
argument is a URL.

* 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/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.
---
 etc/ORG-NEWS      |  5 ++++
 lisp/org-macro.el | 22 ++++++++++------
 lisp/org.el       | 77 +++++++++++++++++++++++++++++++++++++++++++++++--------
 lisp/ox.el        | 38 ++++++++++++++++-----------
 4 files changed, 108 insertions(+), 34 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 23e8a1db7..4ae4f57eb 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -231,6 +231,11 @@ 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.  A new optional argument ~NOCACHE~ is added to
+~org-file-contents~.
+
 ** Removed functions
 
 *** Org Timeline
diff --git a/lisp/org-macro.el b/lisp/org-macro.el
index 71e917b71..590ee57bd 100644
--- a/lisp/org-macro.el
+++ b/lisp/org-macro.el
@@ -56,7 +56,8 @@
 		  (&optional granularity visible-only))
 (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-url-p "org" (file))
+(declare-function org-file-contents "org" (file &optional noerror nocache))
 (declare-function org-mode "org" ())
 (declare-function vc-backend "vc-hooks" (f))
 (declare-function vc-call "vc-hooks" (fun file &rest args) t)
@@ -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 6a15e801c..8dcfd1676 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -5273,17 +5273,72 @@ 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))
-      (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 "\")") "")))))
+(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-file-clear-cache ()
+  "Clear the file cache stored in `org--file-cache'."
+  (interactive)
+  (clrhash org--file-cache))
+
+(defun org-file-url-p (file)
+  "Test whether FILE is a URL.
+Return non-nil if it is."
+  (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)))
+         err)
+    (prog1
+        (cond
+         (cache)           ;Set contents to cache if cache is non-nil
+         (is-url
+          (let (url-retrieve-header
+		cont)
+            (with-current-buffer (url-retrieve-synchronously file)
+              (goto-char (point-min))
+              ;; Take point to after the url-retrieve header
+              (search-forward "\n\n") ;2 consecutive new-line chars
+              (setq url-retrieve-header (buffer-substring-no-properties
+                                         (point-min) (point)))
+              (message url-retrieve-header) ;Dump the URL retrieve header to *Messages*
+              (if (string-match-p "HTTP.*\\s-+200\\s-OK" url-retrieve-header) ;URL retrieved correctly
+                  (progn
+                    (setq cont (buffer-substring-no-properties (point) (point-max)))
+                    ;; Update the cache `org--file-cache'
+                    (puthash file cont org--file-cache))
+                (setq err t)))
+	    cont))
+         (t (if (and file (file-readable-p file))
+                (with-temp-buffer
+	          (insert-file-contents file)
+	          (buffer-string))
+              (setq err t))))
+      (when err
+        (let ((msg (if is-url
+                       "Unable to fetch file from"
+                     "Cannot read file"))
+              (from (buffer-file-name (buffer-base-buffer))))
+          (funcall (if noerror #'message #'user-error)
+                   (format "%s `%s' (referenced in file `%s')" msg file from)))))))
 
 (defun org-extract-log-state-settings (x)
   "Extract the log state setting from a TODO keyword string.
diff --git a/lisp/ox.el b/lisp/ox.el
index ac8d8ce68..7d1012974 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.
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-05-23 19:07                 ` Kaushal Modi
@ 2017-05-25 10:13                   ` Nicolas Goaziou
  2017-05-25 10:18                     ` Nicolas Goaziou
  2017-05-25 11:43                     ` Kaushal Modi
  0 siblings, 2 replies; 25+ messages in thread
From: Nicolas Goaziou @ 2017-05-25 10:13 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: emacs-org list

Hello,

Kaushal Modi <kaushal.modi@gmail.com> writes:

> I have attached a patch following your recommendation. Please review
> it.

Thank you. Comments follow.

>> `org--setupfile-clear-cache' or `org-setupfile--clear-cache' depending
>> on the location of the function.
>
> This is an interactive, a user-facing function; do we want to add
> double-dashes in that name? The function is now called org-file-clear-cache.

Interactive functions do not have double-dashes in their names. However,
I have concerns about this interactive status. Given than the function
is not properly documented in the manual, there is little chance it will
be actually used. And if it isn't, it could return surprising results.

Another idea would be to replace NOCACHE with CLEAR-CACHE. When this is
non-nil, the cache is reset at the beginning of the function. The point
is to reset the cache the first time the function is called, but not on
recursive calls, which ensures any file is retrieved only once. Of
course the cache doesn't survive to multiple exports, but at least it is
transparent to the user.

Yet another idea is to add a time-to-live to cached values and remove
them from cache past it. However, I prefer the idea above.

> I tried (org-file-remote-p "http://foo.bar") but it returned nil. Looks
> like both org-file-remote-p and file-remote-p will not work for URLs.

OK.

> setupfile is always a string so I used %s. If setupfile is not a string
> (may be nil?), then the very first string-match-p will through an error. Is
> there a specific reason for using %S? I did not use %S because I did not
> want the double-quotes to be printed around the string in the echo
> area.

The specific reason is, as you noticed, %S wraps file name within double
quotes. IMO, `...' would be for symbols or key bindings.

> Correct. The attached patch has everything integrated in
> org-file-contents.

`prog1' is difficult to read when the first SEXP is large. Also, we
should avoid splitting error messages and constructing them back, for
hypothetical i18n considerations.

I suggest the following re-factoring, where I limited the number of
bindings and remove some trivial comments.

  (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))
          (if (re-search-forward "HTTP.*\\s-+200\\s-OK" nil t)
              ;; URL retrieved correctly.  Move point to after the
              ;; url-retrieve header, update the cache `org--file-cache'
              ;; and return contents.
              (progn
                (search-forward "\n\n" nil 'move)
                (puthash file
                         (buffer-substring-no-properties (point) (point-max))
                         org--file-cache))
            (funcall (if noerror #'message #'user-error)
                     (format "Unable to fetch file from %S" file)))))
       (t
        (with-temp-buffer
          (condition-case err
              (progn (insert-file-contents file) (buffer-string))
            (file-error
             (funcall (if noerror #'message #'user-error)
                      (error-message-string err)))))))))

I'm not sure about the return value when NOERROR is non-nil, but it may
not matter, per the suggestion above.

> Do we need to update the code using org-file-contents in these places:

What do you meant by "update"?

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-05-25 10:13                   ` Nicolas Goaziou
@ 2017-05-25 10:18                     ` Nicolas Goaziou
  2017-05-25 11:43                     ` Kaushal Modi
  1 sibling, 0 replies; 25+ messages in thread
From: Nicolas Goaziou @ 2017-05-25 10:18 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: emacs-org list

Correcting myself,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>       (cond
>        (cache)
>        (is-url
>         (with-current-buffer (url-retrieve-synchronously file)
>           (goto-char (point-min))
>           (if (re-search-forward "HTTP.*\\s-+200\\s-OK" nil t)
>               ;; URL retrieved correctly.  Move point to after the
>               ;; url-retrieve header, update the cache `org--file-cache'
>               ;; and return contents.
>               (progn
>                 (search-forward "\n\n" nil 'move)
>                 (puthash file
>                          (buffer-substring-no-properties (point) (point-max))
>                          org--file-cache))
>             (funcall (if noerror #'message #'user-error)
>                      (format "Unable to fetch file from %S" file)))))

Err.

  (funcall (if noerror #'message #'user-error)
           "Unable to fetch file from %S"
           file)

Also, in your patch,

    "Test whether FILE is a URL.
  Return non-nil if it is."

  ->

    "Non-nil if FILE is a URL."

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-05-25 10:13                   ` Nicolas Goaziou
  2017-05-25 10:18                     ` Nicolas Goaziou
@ 2017-05-25 11:43                     ` Kaushal Modi
  2017-05-25 15:15                       ` Kaushal Modi
  1 sibling, 1 reply; 25+ messages in thread
From: Kaushal Modi @ 2017-05-25 11:43 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-org list

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

On Thu, May 25, 2017, 6:15 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Interactive functions do not have double-dashes in their names. However,
> I have concerns about this interactive status. Given than the function
> is not properly documented in the manual, there is little chance it will
> be actually used. And if it isn't, it could return surprising results.
>
> Another idea would be to replace NOCACHE with CLEAR-CACHE. When this is
> non-nil, the cache is reset at the beginning of the function. The point
> is to reset the cache the first time the function is called, but not on
> recursive calls, which ensures any file is retrieved only once.


Here is my use case for org-file-clear-cache:

Let's say I have a file where I have a SETUPFILE retrieved from a URL. Now
the upstream version changes but my cache is still on the older version. So
I need to clear the hash. The org-file-clear-cache simply does that.

With the function being interactive, I just do

- M-x org-file-clear-cache
- C-c C-e h h (or whatever I am exporting to)

If you suggest a node where I should put that in the manual, I can add that
to my updated patch. I'll all add more explanation to the doc-string of
that function.

Now, if the CLEAR-CACHE argument is added to org-file-clear-cache, how do
we control the cache clearing interactively from outside?

Also, how do we implement the resetting of the cache only the first time
the function is called? Wouldn't that need an extra alist defvar to record
the state of whether the function is already called specifically for that
file? I think that would unnecessary complicate the logic.

Another idea is that we have a defcustom like org-file-never-cache. When
non-nil, that will always do a fresh URL download. This will be or'ed with
the NOCACHE inside org-file-contents. This, though, makes it a bit
inconvenient for the user to use the latest upstream version when they
need... They might need to set org-file-never-cache to t momentarily,
probably via Local Variables, before an export.

 Of

course the cache doesn't survive to multiple exports, but at least it is
> transparent to the user.
>

Sorry, I didn't follow that. Did you mean that the cache doesn't survive
between emacs sessions? Because the cache will actually survive between
multiple exports.

>
Yet another idea is to add a time-to-live to cached values and remove
> them from cache past it. However, I prefer the idea above.
>


The specific reason is, as you noticed, %S wraps file name within double
> quotes. IMO, `...' would be for symbols or key bindings.
>

I'll replace `%s' with "%S".. I just liked the curly quotes created by ` '.
But if that breaks a convention, I rather not do that.

`prog1' is difficult to read when the first SEXP is large. Also, we
> should avoid splitting error messages and constructing them back, for
> hypothetical i18n considerations.
>
> I suggest the following re-factoring, where I limited the number of
> bindings and remove some trivial comments.
>
>   (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))
>           (if (re-search-forward "HTTP.*\\s-+200\\s-OK" nil t)
>               ;; URL retrieved correctly.  Move point to after the
>               ;; url-retrieve header, update the cache `org--file-cache'
>               ;; and return contents.
>               (progn
>                 (search-forward "\n\n" nil 'move)
>                 (puthash file
>                          (buffer-substring-no-properties (point)
> (point-max))
>                          org--file-cache))
>             (funcall (if noerror #'message #'user-error)
>                      (format "Unable to fetch file from %S" file)))))
>        (t
>         (with-temp-buffer
>           (condition-case err
>               (progn (insert-file-contents file) (buffer-string))
>             (file-error
>              (funcall (if noerror #'message #'user-error)
>                       (error-message-string err)))))))))
>
> I'm not sure about the return value when NOERROR is non-nil, but it may
> not matter, per the suggestion above.
>

I'll integrate your suggestion.

> Do we need to update the code using org-file-contents in these places:
>
> What do you meant by "update"?
>

For the listed locations of org-file-contents instances in my earlier
email, I made no change as I think that those would work the same as before
after this commit is applied. I just wanted you to verify if that's the
case. The only "update" required around org-file-contents was where
expand-file-name was used or default-directory was set (as my patch shows).

Thanks!

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 7940 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-05-25 11:43                     ` Kaushal Modi
@ 2017-05-25 15:15                       ` Kaushal Modi
  2017-05-26  7:47                         ` Nicolas Goaziou
  0 siblings, 1 reply; 25+ messages in thread
From: Kaushal Modi @ 2017-05-25 15:15 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-org list


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

I have attached an updated and rebased patch with most of your suggestions
implemented.

Comments below.

On Thu, May 25, 2017 at 7:43 AM Kaushal Modi <kaushal.modi@gmail.com> wrote:

> On Thu, May 25, 2017, 6:15 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
> wrote:
>
>> Interactive functions do not have double-dashes in their names. However,
>> I have concerns about this interactive status. Given than the function
>> is not properly documented in the manual, there is little chance it will
>> be actually used. And if it isn't, it could return surprising results.
>>
>> Another idea would be to replace NOCACHE with CLEAR-CACHE. When this is
>> non-nil, the cache is reset at the beginning of the function. The point
>> is to reset the cache the first time the function is called, but not on
>> recursive calls, which ensures any file is retrieved only once.
>
>
> Here is my use case for org-file-clear-cache:
>
> Let's say I have a file where I have a SETUPFILE retrieved from a URL. Now
> the upstream version changes but my cache is still on the older version. So
> I need to clear the hash. The org-file-clear-cache simply does that.
>
> With the function being interactive, I just do
>
> - M-x org-file-clear-cache
> - C-c C-e h h (or whatever I am exporting to)
>
> If you suggest a node where I should put that in the manual, I can add
> that to my updated patch. I'll all add more explanation to the doc-string
> of that function.
>
> Now, if the CLEAR-CACHE argument is added to org-file-clear-cache, how do
> we control the cache clearing interactively from outside?
>
> Also, how do we implement the resetting of the cache only the first time
> the function is called? Wouldn't that need an extra alist defvar to record
> the state of whether the function is already called specifically for that
> file? I think that would unnecessary complicate the logic.
>
> Another idea is that we have a defcustom like org-file-never-cache. When
> non-nil, that will always do a fresh URL download. This will be or'ed with
> the NOCACHE inside org-file-contents. This, though, makes it a bit
> inconvenient for the user to use the latest upstream version when they
> need... They might need to set org-file-never-cache to t momentarily,
> probably via Local Variables, before an export.
>
>  Of
>
> course the cache doesn't survive to multiple exports, but at least it is
>> transparent to the user.
>>
>
> Sorry, I didn't follow that. Did you mean that the cache doesn't survive
> between emacs sessions? Because the cache will actually survive between
> multiple exports.
>

This issue is still open. I have just added a bit more info to the
docstring of org-file-clear-cache.

I grepped org.texi but found no reference of org-file-contents. So may be
we need to add a section for that, and there I can explain
org-file-clear-cache in more detail. What would be a good node for that?


>           (if (re-search-forward "HTTP.*\\s-+200\\s-OK" nil t)
>>               ;; URL retrieved correctly.  Move point to after the
>>               ;; url-retrieve header, update the cache `org--file-cache'
>>               ;; and return contents.
>>               (progn
>>                 (search-forward "\n\n" nil 'move)
>
>
I have integrated most of your refactored version except for this portion.
Above will do a false match if that "HTTP.." string is present in the FILE
body too! I have retained my version of only that part where the search
happens only inside the url-retrieve header. The search is also faster in
the case of failure as it does not have to search through the whole file
before declaring a fail.. as only the header is searched.

> --

Kaushal Modi

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

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

From b9877ae43bb8e83e1cecfdf0dab5065574c8dc85 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Thu, 25 May 2017 11:08:20 -0400
Subject: [PATCH] Allow org-file-contents to fetch file contents from a URL

* lisp/org.el (org--file-cache): New internal variable to store
downloaded files' cache.

* lisp/org.el (org-file-clear-cache): New interactive function to
clear the above file cache.

* lisp/org.el (org-file-url-p): New function to test if the input
argument is a URL.

* 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/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.
---
 etc/ORG-NEWS      |  5 ++++
 lisp/org-macro.el | 22 ++++++++++-------
 lisp/org.el       | 72 +++++++++++++++++++++++++++++++++++++++++++++++--------
 lisp/ox.el        | 38 +++++++++++++++++------------
 4 files changed, 104 insertions(+), 33 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 044f167ce..6e24d408f 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -234,6 +234,11 @@ 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.  A new optional argument ~NOCACHE~ is added to
+~org-file-contents~.
+
 ** Removed functions
 
 *** Org Timeline
diff --git a/lisp/org-macro.el b/lisp/org-macro.el
index f5ddb92e4..9f6e0ebaf 100644
--- a/lisp/org-macro.el
+++ b/lisp/org-macro.el
@@ -59,7 +59,8 @@
 		  (&optional granularity visible-only))
 (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-url-p "org" (file))
+(declare-function org-file-contents "org" (file &optional noerror nocache))
 (declare-function org-mode "org" ())
 (declare-function vc-backend "vc-hooks" (f))
 (declare-function vc-call "vc-hooks" (fun file &rest args) t)
@@ -105,16 +106,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 946d8af8c..ebd1f4792 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -5273,17 +5273,69 @@ 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-file-clear-cache ()
+  "Clear the file cache stored in `org--file-cache'.
+
+By default, if the FILE argument of `org-file-contents' is a URL, the
+URL download will be skipped if it was already downloaded and cached
+in `org--file-cache'.  If you need to force-download the URL again,
+call this function to clear the cache first."
+  (interactive)
+  (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 (string-match-p "HTTP.*\\s-+200\\s-OK"
+			    (buffer-substring-no-properties
+                             (point-min) (point)))
+	    ;; 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.
diff --git a/lisp/ox.el b/lisp/ox.el
index ac8d8ce68..7d1012974 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.
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-05-25 15:15                       ` Kaushal Modi
@ 2017-05-26  7:47                         ` Nicolas Goaziou
  2017-05-26 20:24                           ` Kaushal Modi
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Goaziou @ 2017-05-26  7:47 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: emacs-org list

Hello,

Kaushal Modi <kaushal.modi@gmail.com> writes:

> I have attached an updated and rebased patch with most of your suggestions
> implemented.

Thank you.

>> Here is my use case for org-file-clear-cache:
>>
>> Let's say I have a file where I have a SETUPFILE retrieved from a URL. Now
>> the upstream version changes but my cache is still on the older version. So
>> I need to clear the hash. The org-file-clear-cache simply does that.

I understand the use case for `org-file-clear-cache'. My suggestion is
to remove that need by clearing cache automatically. Of course, the
drawback is the cache is cleared more often than necessary. Users could
get very surprising results if they forget to call this function. So it
might be a good idea to call it from time to time, on user's behalf.

> I grepped org.texi but found no reference of org-file-contents. So may be
> we need to add a section for that, and there I can explain
> org-file-clear-cache in more detail. What would be a good node for
> that?

`org-file-contents' is a developer-facing function. I don't really see
a good place in the manual for it. That's the problem of
`org-file-clear-cache', which is really an implementation detail users
shouldn't care about.

Here's another idea: call it from `org-mode-restart'. So cache is reset
whenever `C-c C-c' is pressed on a keyword. So we don't need to document
the function anymore. Instead, we could drop a note about the cache in
(info "(org) The very busy C-c C-c key").

>>           (if (re-search-forward "HTTP.*\\s-+200\\s-OK" nil t)
>>>               ;; URL retrieved correctly.  Move point to after the
>>>               ;; url-retrieve header, update the cache `org--file-cache'
>>>               ;; and return contents.
>>>               (progn
>>>                 (search-forward "\n\n" nil 'move)
>>
>>
> I have integrated most of your refactored version except for this portion.
> Above will do a false match if that "HTTP.." string is present in the FILE
> body too! I have retained my version of only that part where the search
> happens only inside the url-retrieve header. The search is also faster in
> the case of failure as it does not have to search through the whole file
> before declaring a fail.. as only the header is searched.

OK. Then the following at least doesn't have the overhead of creating
a string:


(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 t))
      ;; 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)))

Also, mind the full stop at the end of the comments.

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-05-26  7:47                         ` Nicolas Goaziou
@ 2017-05-26 20:24                           ` Kaushal Modi
  2017-05-28  7:35                             ` Nicolas Goaziou
  0 siblings, 1 reply; 25+ messages in thread
From: Kaushal Modi @ 2017-05-26 20:24 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-org list


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

Thanks for another round of review.

I have attached a patch, rebased to master and with all suggestions
implemented.

There are some additional changes in the patch this time:
- Prevent org-edit-special from attempting to open the "file" for editing
if the value is a URL.
- Silence byte-compiler for ffap-url-regexp. We do a require, but the
byte-compiler still complains of a free variable.
- Update org.texi at few places (first time editing a texi)

On Fri, May 26, 2017 at 3:47 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> I understand the use case for `org-file-clear-cache'. My suggestion is
> to remove that need by clearing cache automatically. Of course, the
> drawback is the cache is cleared more often than necessary. Users could
> get very surprising results if they forget to call this function. So it
> might be a good idea to call it from time to time, on user's behalf.
>

I liked your C-c C-c idea below! :)


> `org-file-contents' is a developer-facing function. I don't really see
> a good place in the manual for it. That's the problem of
> `org-file-clear-cache', which is really an implementation detail users
> shouldn't care about.
>

Understood.


> Here's another idea: call it from `org-mode-restart'. So cache is reset
> whenever `C-c C-c' is pressed on a keyword. So we don't need to document
> the function anymore. Instead, we could drop a note about the cache in
> (info "(org) The very busy C-c C-c key").
>

Done!


> OK. Then the following at least doesn't have the overhead of creating
> a string:
>

Integrated. Thanks!

Also, mind the full stop at the end of the comments.
>

Will do.
-- 

Kaushal Modi

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

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

From 80bb0f81fc54e2bdd7f00c03f0d0cae789204877 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Thu, 25 May 2017 11:08:20 -0400
Subject: [PATCH] Allow org-file-contents to fetch file contents from a URL

* lisp/org.el (org--file-cache): New internal variable to store
downloaded files' cache.

* lisp/org.el (org-mode-restart): Now also clears the above file
  cache.

* lisp/org.el (org-file-url-p): New function to test if the input
argument is a URL.

* 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.
---
 doc/org.texi      | 37 ++++++++++++++++-------------
 etc/ORG-NEWS      | 12 +++++++++-
 lisp/org-macro.el | 22 ++++++++++-------
 lisp/org.el       | 70 +++++++++++++++++++++++++++++++++++++++++++++----------
 lisp/ox.el        | 38 ++++++++++++++++++------------
 5 files changed, 127 insertions(+), 52 deletions(-)

diff --git a/doc/org.texi b/doc/org.texi
index 142fa9627..3ebc01c0c 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/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:
@@ -17147,13 +17147,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
+@item #+SETUPFILE: file/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 cache.  @kbd{C-c C-c} on the settings line will
+also parse and load.  @kbd{C-c C-c} on the @code{#+SETUPFILE:} line will also
+reset the temporary file cache.  Org also parses and loads the file/URL
+during normal exporting process.  Org parses the contents of this file/URL 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
@@ -17395,7 +17398,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 044f167ce..a84eb98c1 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
@@ -234,6 +234,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 f5ddb92e4..9f6e0ebaf 100644
--- a/lisp/org-macro.el
+++ b/lisp/org-macro.el
@@ -59,7 +59,8 @@
 		  (&optional granularity visible-only))
 (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-url-p "org" (file))
+(declare-function org-file-contents "org" (file &optional noerror nocache))
 (declare-function org-mode "org" ())
 (declare-function vc-backend "vc-hooks" (f))
 (declare-function vc-call "vc-hooks" (fun file &rest args) t)
@@ -105,16 +106,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 102a9b265..171b10cd7 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -5273,17 +5273,59 @@ 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'.")
+
+(defvar ffap-url-regexp)		;Silence byte-compiler
+(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.
@@ -20685,7 +20727,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))
@@ -20949,7 +20993,9 @@ 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))
+    ;; Reset the cache of files downloaded by `org-file-contents'.
+    (clrhash org--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 ac8d8ce68..7d1012974 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.
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-05-26 20:24                           ` Kaushal Modi
@ 2017-05-28  7:35                             ` Nicolas Goaziou
  2017-05-28 10:04                               ` Kaushal Modi
  2017-06-09 16:59                               ` Kaushal Modi
  0 siblings, 2 replies; 25+ messages in thread
From: Nicolas Goaziou @ 2017-05-28  7:35 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: emacs-org list

Hello,

Kaushal Modi <kaushal.modi@gmail.com> writes:

> I have attached a patch, rebased to master and with all suggestions
> implemented.

Thank you.

> There are some additional changes in the patch this time:
> - Prevent org-edit-special from attempting to open the "file" for editing
> if the value is a URL.

This is a good idea. We could even display a read-only buffer with the
contents of the document, using new `org-file-contents'.

Anyway, this can be done in a different patch.

> I liked your C-c C-c idea below! :)

Great!

> +indirectly through a file included using @samp{#+SETUPFILE: filename/URL}

filename/URL > file name or URL

> +@item #+SETUPFILE: file/URL

file/URL > file name 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 cache.  @kbd{C-c C-c} on the settings line will
> +also parse and load.  @kbd{C-c C-c} on the @code{#+SETUPFILE:} line will also
> +reset the temporary file cache.  Org also parses and loads the file/URL

file/URL > document

> +during normal exporting process.  Org parses the contents of this
> file/URL as

file/URL document

> +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
> @@ -17395,7 +17398,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

org > Org

> +(defvar ffap-url-regexp)		;Silence byte-compiler

I think this can go at the beginning of the "org.el".

> +    ;; Reset the cache of files downloaded by `org-file-contents'.
> +    (clrhash org--file-cache))

Note that we can still implement a non-interactive
`org-reset-setupfile-cache' function and call it here instead. (clrhash
org--file-cache) may be a bit low level at this point.

Otherwise, LGTM. Would you want to throw in some tests? You can use
cl-letf to bind `url-retrieve-synchronously' to a function returning
a buffer containing some dumb (valid or invalid) output.

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-05-28  7:35                             ` Nicolas Goaziou
@ 2017-05-28 10:04                               ` Kaushal Modi
  2017-06-09 16:59                               ` Kaushal Modi
  1 sibling, 0 replies; 25+ messages in thread
From: Kaushal Modi @ 2017-05-28 10:04 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-org list

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

On Sun, May 28, 2017, 3:36 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Hello,
>
> Kaushal Modi <kaushal.modi@gmail.com> writes:
>
> > I have attached a patch, rebased to master and with all suggestions
> > implemented.
>
> Thank you.
>
> > There are some additional changes in the patch this time:
> > - Prevent org-edit-special from attempting to open the "file" for editing
> > if the value is a URL.
>
> This is a good idea. We could even display a read-only buffer with the
> contents of the document, using new `org-file-contents'.
>
> Anyway, this can be done in a different patch.
>
> > I liked your C-c C-c idea below! :)
>
> Great!
>
> > +indirectly through a file included using @samp{#+SETUPFILE:
> filename/URL}
>
> filename/URL > file name or URL
>
> > +@item #+SETUPFILE: file/URL
>
> file/URL > file name 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 cache.  @kbd{C-c C-c} on the settings line
> will
> > +also parse and load.  @kbd{C-c C-c} on the @code{#+SETUPFILE:} line
> will also
> > +reset the temporary file cache.  Org also parses and loads the file/URL
>
> file/URL > document
>
> > +during normal exporting process.  Org parses the contents of this
> > file/URL as
>
> file/URL document
>
> > +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
> > @@ -17395,7 +17398,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
>
> org > Org
>
> > +(defvar ffap-url-regexp)             ;Silence byte-compiler
>
> I think this can go at the beginning of the "org.el".
>
> > +    ;; Reset the cache of files downloaded by `org-file-contents'.
> > +    (clrhash org--file-cache))
>
> Note that we can still implement a non-interactive
> `org-reset-setupfile-cache' function and call it here instead. (clrhash
> org--file-cache) may be a bit low level at this point.
>
> Otherwise, LGTM. Would you want to throw in some tests? You can use
> cl-letf to bind `url-retrieve-synchronously' to a function returning
> a buffer containing some dumb (valid or invalid) output.
>

Thanks. I'll get back to this in 8 days; I'm going offline (vacation).

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 3737 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-05-28  7:35                             ` Nicolas Goaziou
  2017-05-28 10:04                               ` Kaushal Modi
@ 2017-06-09 16:59                               ` Kaushal Modi
  2017-06-12 19:32                                 ` Kaushal Modi
  1 sibling, 1 reply; 25+ messages in thread
From: Kaushal Modi @ 2017-06-09 16:59 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-org list


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

I have attached the updated patch, rebased to master.


On Sun, May 28, 2017 at 3:36 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> This is a good idea. We could even display a read-only buffer with the
> contents of the document, using new `org-file-contents'.
>
> Anyway, this can be done in a different patch.
>

Thanks. I was on the fence about whether I should open the cached file in a
read-only buffer.. but then the function was named `org-edit-special'.. the
"edit" part. But if you are fine, I can work on that in a different commit.


> > +indirectly through a file included using @samp{#+SETUPFILE:
> filename/URL}
>
> filename/URL > file name or URL
>
> > +@item #+SETUPFILE: file/URL
>
> file/URL > file name or URL
>
> > +reset the temporary file cache.  Org also parses and loads the file/URL
>
> file/URL > document
>
> > +during normal exporting process.  Org parses the contents of this
> > file/URL as
>
> file/URL document
>
> > +buffer for these lines and update the information.  Also reset the org
> file
>
> org > Org
>

Done.


> > +(defvar ffap-url-regexp)             ;Silence byte-compiler
>
> I think this can go at the beginning of the "org.el".
>

Done.


> > +    ;; Reset the cache of files downloaded by `org-file-contents'.
> > +    (clrhash org--file-cache))
>
> Note that we can still implement a non-interactive
> `org-reset-setupfile-cache' function and call it here instead. (clrhash
> org--file-cache) may be a bit low level at this point.
>

Done, except that I named the function org-reset-file-cache as the file
cache can be updated by org-file-contents function in general for any file.


> Otherwise, LGTM. Would you want to throw in some tests? You can use
> cl-letf to bind `url-retrieve-synchronously' to a function returning
> a buffer containing some dumb (valid or invalid) output.
>

Done! I, though used invalid URLs in the test. (If they become valid at
some point in distant future (I doubt), then we can update the test :))
-- 

Kaushal Modi

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

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

From 711d71895cc3fec23ee4665299df665842fe9065 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Fri, 9 Jun 2017 12:56:11 -0400
Subject: [PATCH] Allow org-file-contents to fetch file contents from a URL

* lisp/org.el (org--file-cache): New internal variable to store
downloaded files' cache.

* lisp/org.el (org-reset-file-cache): New function to clear the above
file cache.

* lisp/org.el (org-mode-restart): Use org-reset-file-cache to clear
the file cache.

* lisp/org.el (org-file-url-p): New function to test if the input
argument is a URL.

* 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.
---
 doc/org.texi             | 37 +++++++++++++-----------
 etc/ORG-NEWS             | 12 +++++++-
 lisp/org-macro.el        | 22 ++++++++------
 lisp/org.el              | 74 ++++++++++++++++++++++++++++++++++++++++--------
 lisp/ox.el               | 38 +++++++++++++++----------
 testing/lisp/test-org.el | 41 +++++++++++++++++++++++++++
 6 files changed, 172 insertions(+), 52 deletions(-)

diff --git a/doc/org.texi b/doc/org.texi
index dee46d1054..195e37ad9f 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,13 +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
+@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 cache.  @kbd{C-c C-c} on the settings line will
+also parse and load.  @kbd{C-c C-c} on the @code{#+SETUPFILE:} line will 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
@@ -17422,7 +17425,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 b984103e04..7a1c68ca2e 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 96461f9a95..ffbfa91add 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-url-p "org" (file))
+(declare-function org-file-contents "org" (file &optional noerror nocache))
 (declare-function org-mode "org" ())
 (declare-function vc-backend "vc-hooks" (f))
 (declare-function vc-call "vc-hooks" (fun file &rest args) t)
@@ -101,16 +102,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..31bda86073 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -6498,6 +6498,47 @@ 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= "#+BIND: variable value
+#+DESCRIPTION: l2
+#+LANGUAGE: en
+#+SELECT_TAGS: b
+#+TITLE: b
+#+PROPERTY: a 1
+" (org-file-contents "http://orgmode.org/cgit.cgi/org-mode.git/plain/testing/examples/setupfile3.org")))
+
+  (let ((invalid-url "http://this-url-must-not-exist"))
+    ;; Throw error when trying to access an invalid URL.
+    (should-error
+     (org-file-contents invalid-url))
+    ;; Try to access an invalid URL, but do not throw an error.
+    (should
+     (string= (format "Unable to fetch file from \"%s\"" invalid-url)
+	      (org-file-contents invalid-url :noerror)))))
+
 
 (provide 'test-org)
 
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-06-09 16:59                               ` Kaushal Modi
@ 2017-06-12 19:32                                 ` Kaushal Modi
  2017-06-13 12:43                                   ` Nicolas Goaziou
  0 siblings, 1 reply; 25+ messages in thread
From: Kaushal Modi @ 2017-06-12 19:32 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-org list


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

I have attached the same patch with one more test; rebased to master.

On Fri, Jun 9, 2017 at 12:59 PM Kaushal Modi <kaushal.modi@gmail.com> wrote:

> I have attached the updated patch, rebased to master.
>
-- 

Kaushal Modi

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

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

From e6441d48c970abd769509220156d2c59028bac24 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Mon, 12 Jun 2017 12:10:25 -0400
Subject: [PATCH] Allow org-file-contents to fetch file contents from a URL

* lisp/org.el (org--file-cache): New internal variable to store
downloaded files' cache.

* lisp/org.el (org-reset-file-cache): New function to clear the above
file cache.

* lisp/org.el (org-mode-restart): Use org-reset-file-cache to clear
the file cache.

* lisp/org.el (org-file-url-p): New function to test if the input
argument is a URL.

* 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             | 37 +++++++++++++-----------
 etc/ORG-NEWS             | 12 +++++++-
 lisp/org-macro.el        | 22 ++++++++------
 lisp/org.el              | 74 ++++++++++++++++++++++++++++++++++++++++--------
 lisp/ox.el               | 38 +++++++++++++++----------
 testing/lisp/test-org.el | 41 +++++++++++++++++++++++++++
 testing/lisp/test-ox.el  | 15 ++++++++++
 7 files changed, 187 insertions(+), 52 deletions(-)

diff --git a/doc/org.texi b/doc/org.texi
index 83016282ca..a625fac8fb 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,13 +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
+@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 cache.  @kbd{C-c C-c} on the settings line will
+also parse and load.  @kbd{C-c C-c} on the @code{#+SETUPFILE:} line will 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
@@ -17422,7 +17425,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..31bda86073 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -6498,6 +6498,47 @@ 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= "#+BIND: variable value
+#+DESCRIPTION: l2
+#+LANGUAGE: en
+#+SELECT_TAGS: b
+#+TITLE: b
+#+PROPERTY: a 1
+" (org-file-contents "http://orgmode.org/cgit.cgi/org-mode.git/plain/testing/examples/setupfile3.org")))
+
+  (let ((invalid-url "http://this-url-must-not-exist"))
+    ;; Throw error when trying to access an invalid URL.
+    (should-error
+     (org-file-contents invalid-url))
+    ;; Try to access an invalid URL, but do not throw an error.
+    (should
+     (string= (format "Unable to fetch file from \"%s\"" invalid-url)
+	      (org-file-contents invalid-url :noerror)))))
+
 
 (provide 'test-org)
 
diff --git a/testing/lisp/test-ox.el b/testing/lisp/test-ox.el
index 69a778bbb3..e439651129 100644
--- a/testing/lisp/test-ox.el
+++ b/testing/lisp/test-ox.el
@@ -228,6 +228,21 @@ num:2 <:active")))
 #+SETUPFILE: \"%s/examples/setupfile.org\"
 #+LANGUAGE: fr
 #+SELECT_TAGS: c
+#+TITLE: c"
+		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.
+  (should
+   (equal
+    (org-test-with-temp-text
+	(format "#+DESCRIPTION: l1
+#+LANGUAGE: es
+#+SELECT_TAGS: a
+#+TITLE: a
+#+SETUPFILE: \"http://orgmode.org/cgit.cgi/org-mode.git/plain/testing/examples/setupfile3.org\"
+#+LANGUAGE: fr
+#+SELECT_TAGS: c
 #+TITLE: c"
 		org-test-dir)
       (org-export--get-inbuffer-options))
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-06-12 19:32                                 ` Kaushal Modi
@ 2017-06-13 12:43                                   ` Nicolas Goaziou
  2017-06-13 15:45                                     ` Kaushal Modi
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Goaziou @ 2017-06-13 12:43 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: emacs-org list

Hello,

Kaushal Modi <kaushal.modi@gmail.com> writes:

> I have attached the same patch with one more test; rebased to master.

Great. Thank you.

> * lisp/org.el (org--file-cache): New internal variable to store
> downloaded files' cache.
>
> * lisp/org.el (org-reset-file-cache): New function to clear the above
> file cache.
>
> * lisp/org.el (org-mode-restart): Use org-reset-file-cache to clear
> the file cache.
>
> * lisp/org.el (org-file-url-p): New function to test if the input
> argument is a URL.

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.

> +Org opens the main file.  If URL is specified, the contents are downloaded
> +and stored in a temporary cache.  @kbd{C-c C-c} on the settings line will
> +also parse and load.  @kbd{C-c C-c} on the @code{#+SETUPFILE:} line will also
> +reset the temporary file cache.

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

> +(ert-deftest test-org/org-file-contents-url ()
> +  "Test `org-file-contents' with a URL as input."
> +  (should
> +   (string= "#+BIND: variable value
> +#+DESCRIPTION: l2
> +#+LANGUAGE: en
> +#+SELECT_TAGS: b
> +#+TITLE: b
> +#+PROPERTY: a 1
> +" (org-file-contents "http://orgmode.org/cgit.cgi/org-mode.git/plain/testing/examples/setupfile3.org")))

I'm worried about this test, and some other below. They require a proper
internet access. 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)))))


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-06-13 12:43                                   ` Nicolas Goaziou
@ 2017-06-13 15:45                                     ` Kaushal Modi
  2017-06-13 21:32                                       ` Nicolas Goaziou
  0 siblings, 1 reply; 25+ messages in thread
From: Kaushal Modi @ 2017-06-13 15:45 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-org list


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


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-06-13 15:45                                     ` Kaushal Modi
@ 2017-06-13 21:32                                       ` Nicolas Goaziou
  2017-06-13 21:42                                         ` Kaushal Modi
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Goaziou @ 2017-06-13 21:32 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: emacs-org list

> I have attached the revised patch.

LGTM. Thank you for the work!

Regards,

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Allow #+SETUPFILE to point to an URL for the org file
  2017-06-13 21:32                                       ` Nicolas Goaziou
@ 2017-06-13 21:42                                         ` Kaushal Modi
  0 siblings, 0 replies; 25+ messages in thread
From: Kaushal Modi @ 2017-06-13 21:42 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-org list

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

On Tue, Jun 13, 2017 at 5:32 PM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> > I have attached the revised patch.
>
> LGTM. Thank you for the work!
>

Yay! Pushed to master.

Thanks for the super reviews. It was a great learning experience.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 651 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2017-06-13 21:42 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-03 17:23 Allow #+SETUPFILE to point to an URL for the org file Kaushal Modi
2016-12-08 11:51 ` Kaushal Modi
2016-12-08 14:22   ` John Kitchin
2016-12-08 14:31   ` Nicolas Goaziou
2016-12-08 14:44     ` Kaushal Modi
2016-12-08 21:48       ` Nicolas Goaziou
2016-12-08 22:07         ` Kaushal Modi
2016-12-08 22:40           ` Nicolas Goaziou
2017-03-13 17:37             ` Kaushal Modi
2017-03-30  7:43               ` Nicolas Goaziou
2017-05-23 19:07                 ` Kaushal Modi
2017-05-25 10:13                   ` Nicolas Goaziou
2017-05-25 10:18                     ` Nicolas Goaziou
2017-05-25 11:43                     ` Kaushal Modi
2017-05-25 15:15                       ` Kaushal Modi
2017-05-26  7:47                         ` Nicolas Goaziou
2017-05-26 20:24                           ` Kaushal Modi
2017-05-28  7:35                             ` Nicolas Goaziou
2017-05-28 10:04                               ` Kaushal Modi
2017-06-09 16:59                               ` Kaushal Modi
2017-06-12 19:32                                 ` Kaushal Modi
2017-06-13 12:43                                   ` Nicolas Goaziou
2017-06-13 15:45                                     ` Kaushal Modi
2017-06-13 21:32                                       ` Nicolas Goaziou
2017-06-13 21:42                                         ` Kaushal Modi

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).