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 SETUPFILE contents.
`org--setupfile-clear-cache' or `org-setupfile--clear-cache' depending
on the location of the function.
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?
Mind the double spaces at the end of sentences.
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)))
> (re-search-forward "\n\n") ; 2 consecutive new-line chars
`re-search-forward' -> `search-forward'
> (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.
Kaushal Modi