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 <m...@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
0001-Allow-org-file-contents-to-fetch-file-contents-from-.patch
Description: Binary data
