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