From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kaushal Modi Subject: Re: Allow #+SETUPFILE to point to an URL for the org file Date: Thu, 25 May 2017 11:43:31 +0000 Message-ID: References: <87h96eh4qb.fsf@nicolasgoaziou.fr> <871sxigkhk.fsf@nicolasgoaziou.fr> <87twaef3iq.fsf@nicolasgoaziou.fr> <874lybp5ua.fsf@nicolasgoaziou.fr> <87shjtb5wd.fsf@nicolasgoaziou.fr> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="94eb2c19d22297fda7055057bb72" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:40224) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dDrBC-0003Pz-Qf for emacs-orgmode@gnu.org; Thu, 25 May 2017 07:43:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dDrBB-0005tW-6M for emacs-orgmode@gnu.org; Thu, 25 May 2017 07:43:46 -0400 Received: from mail-lf0-x22e.google.com ([2a00:1450:4010:c07::22e]:34229) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dDrBA-0005tK-Pu for emacs-orgmode@gnu.org; Thu, 25 May 2017 07:43:45 -0400 Received: by mail-lf0-x22e.google.com with SMTP id 99so81977834lfu.1 for ; Thu, 25 May 2017 04:43:44 -0700 (PDT) In-Reply-To: <87shjtb5wd.fsf@nicolasgoaziou.fr> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: Nicolas Goaziou Cc: emacs-org list --94eb2c19d22297fda7055057bb72 Content-Type: text/plain; charset="UTF-8" On Thu, May 25, 2017, 6:15 AM Nicolas Goaziou wrote: > 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. Here is my use case for org-file-clear-cache: Let's say I have a file where I have a SETUPFILE retrieved from a URL. Now the upstream version changes but my cache is still on the older version. So I need to clear the hash. The org-file-clear-cache simply does that. With the function being interactive, I just do - M-x org-file-clear-cache - C-c C-e h h (or whatever I am exporting to) If you suggest a node where I should put that in the manual, I can add that to my updated patch. I'll all add more explanation to the doc-string of that function. Now, if the CLEAR-CACHE argument is added to org-file-clear-cache, how do we control the cache clearing interactively from outside? Also, how do we implement the resetting of the cache only the first time the function is called? Wouldn't that need an extra alist defvar to record the state of whether the function is already called specifically for that file? I think that would unnecessary complicate the logic. Another idea is that we have a defcustom like org-file-never-cache. When non-nil, that will always do a fresh URL download. This will be or'ed with the NOCACHE inside org-file-contents. This, though, makes it a bit inconvenient for the user to use the latest upstream version when they need... They might need to set org-file-never-cache to t momentarily, probably via Local Variables, before an export. Of course the cache doesn't survive to multiple exports, but at least it is > transparent to the user. > Sorry, I didn't follow that. Did you mean that the cache doesn't survive between emacs sessions? Because the cache will actually survive between multiple exports. > 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. > The specific reason is, as you noticed, %S wraps file name within double > quotes. IMO, `...' would be for symbols or key bindings. > I'll replace `%s' with "%S".. I just liked the curly quotes created by ` '. But if that breaks a convention, I rather not do that. `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. > I'll integrate your suggestion. > Do we need to update the code using org-file-contents in these places: > > What do you meant by "update"? > For the listed locations of org-file-contents instances in my earlier email, I made no change as I think that those would work the same as before after this commit is applied. I just wanted you to verify if that's the case. The only "update" required around org-file-contents was where expand-file-name was used or default-directory was set (as my patch shows). Thanks! > -- Kaushal Modi --94eb2c19d22297fda7055057bb72 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Thu, May 25, 2017, 6:15 AM N= icolas Goaziou <mail@nicolasgo= aziou.fr> wrote:
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.<= br>
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.

Here is my use case for org-file-clear-cache:

Let's say I have a file where I have a SETUPFILE= retrieved from a URL. Now the upstream version changes but my cache is sti= ll on the older version. So I need to clear the hash. The org-file-clear-ca= che simply does that.=C2=A0

With the function bein= g interactive, I just do=C2=A0

- M-x org-file-clea= r-cache
- C-c C-e h h (or whatever I am exporting to)
<= br>
If you suggest a node where I should put that in the manual, = I can add that to my updated patch. I'll all add more explanation to th= e doc-string of that function. =C2=A0

Now, if = the CLEAR-CACHE argument is added to org-file-clear-cache, how do we contro= l the cache clearing interactively from outside?=C2=A0

=
Also, how do we implement the resetting of the cache only the first ti= me the function is called? Wouldn't that need an extra alist defvar to = record the state of whether the function is already called specifically for= that file? I think that would unnecessary complicate the logic.=C2=A0

Another idea is that we have a defcustom like org-file= -never-cache. When non-nil, that will always do a fresh URL download. This = will be or'ed with the NOCACHE inside org-file-contents. This, though, = makes it a bit inconvenient for the user to use the latest upstream version= when they need... They might need to set org-file-never-cache to t momenta= rily, probably via Local Variables, before an export.=C2=A0

<= /div>
=C2=A0Of
course the cache doesn't survive to multiple exports, but at least it i= s
transparent to the user.

Sorry, I= didn't follow that. Did you mean that the cache doesn't survive be= tween emacs sessions? Because the cache will actually survive between multi= ple exports.=C2=A0

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.
=


The specific reason is, as you noticed, %S wraps file na= me within double
quotes. IMO, `...' would be for symbols or key bindings.

I'll replace `%s' with "%S"..= I just liked the curly quotes created by ` '. But if that breaks a con= vention, I rather not do that.=C2=A0

`prog1' is difficult to read wh= en 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.

=C2=A0 (defun org-file-contents (file &optional noerror nocache)
=C2=A0 =C2=A0 "Return the contents of FILE, as a string.

=C2=A0 FILE can be a file name or URL.

=C2=A0 If FILE is a URL, download the contents.=C2=A0 If the URL contents a= re
=C2=A0 already cached in the `org--file-cache' hash table, the download= step
=C2=A0 is skipped.

=C2=A0 If NOERROR is non-nil, ignore the error when unable to read the FILE=
=C2=A0 from file or URL.

=C2=A0 If NOCACHE is non-nil, do a fresh fetch of FILE even if cached versi= on
=C2=A0 is available.=C2=A0 This option applies only if FILE is a URL."=
=C2=A0 =C2=A0 (let* ((is-url (org-file-url-p file))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(cache (and is-url
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0(not nocache)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0(gethash file org--file-cache))))
=C2=A0 =C2=A0 =C2=A0 (cond
=C2=A0 =C2=A0 =C2=A0 =C2=A0(cache)
=C2=A0 =C2=A0 =C2=A0 =C2=A0(is-url
=C2=A0 =C2=A0 =C2=A0 =C2=A0 (with-current-buffer (url-retrieve-synchronousl= y file)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (goto-char (point-min))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (if (re-search-forward "HTTP.*\\s-+= 200\\s-OK" nil t)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ;; URL retrieved correctly= .=C2=A0 Move point to after the
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ;; url-retrieve header, up= date the cache `org--file-cache'
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ;; and return contents. =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (progn
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (search-forward &qu= ot;\n\n" nil 'move)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (puthash file
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0(buffer-substring-no-properties (point) (point-max))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0org--file-cache))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (funcall (if noerror #'messag= e #'user-error)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0(format "Unable to fetch file from %S" file)))))
=C2=A0 =C2=A0 =C2=A0 =C2=A0(t
=C2=A0 =C2=A0 =C2=A0 =C2=A0 (with-temp-buffer
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (condition-case err
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (progn (insert-file-conten= ts file) (buffer-string))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (file-error
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(funcall (if noerror #'= message #'user-error)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 (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.

=
I'll integrate your suggestion.

> Do we need to update the c= ode using org-file-contents in these places:

What do you meant by "update"?

For the listed locations of org-file-contents instances in my earli= er email, I made no change as I think that those would work the same as bef= ore after this commit is applied. I just wanted you to verify if that's= the case. The only "update" required around org-file-contents wa= s where expand-file-name was used or default-directory was set (as my patch= shows).=C2=A0

Thanks!
--

Kaushal Modi

--94eb2c19d22297fda7055057bb72--