From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Kaushal Modi <kaushal.modi@gmail.com>
Cc: emacs-org list <emacs-orgmode@gnu.org>
Subject: Re: Allow #+SETUPFILE to point to an URL for the org file
Date: Thu, 25 May 2017 12:13:54 +0200 [thread overview]
Message-ID: <87shjtb5wd.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <CAFyQvY0VfUzcnR4+TAXn3fSti_==O2d6suXhn6grqf9CwztLew@mail.gmail.com> (Kaushal Modi's message of "Tue, 23 May 2017 19:07:16 +0000")
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
next prev parent reply other threads:[~2017-05-25 10:15 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87shjtb5wd.fsf@nicolasgoaziou.fr \
--to=mail@nicolasgoaziou.fr \
--cc=emacs-orgmode@gnu.org \
--cc=kaushal.modi@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).