emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Kaushal Modi
Subject: Re: [O] Allow #+SETUPFILE to point to an URL for the org file
Date: Tue, 23 May 2017 19:07:16 +0000

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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]