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

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

Kaushal Modi <kaushal.modi@gmail.com> writes:

> I have attached an updated and rebased patch with most of your suggestions
> implemented.

Thank you.

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

I understand the use case for `org-file-clear-cache'. My suggestion is
to remove that need by clearing cache automatically. Of course, the
drawback is the cache is cleared more often than necessary. Users could
get very surprising results if they forget to call this function. So it
might be a good idea to call it from time to time, on user's behalf.

> I grepped org.texi but found no reference of org-file-contents. So may be
> we need to add a section for that, and there I can explain
> org-file-clear-cache in more detail. What would be a good node for
> that?

`org-file-contents' is a developer-facing function. I don't really see
a good place in the manual for it. That's the problem of
`org-file-clear-cache', which is really an implementation detail users
shouldn't care about.

Here's another idea: call it from `org-mode-restart'. So cache is reset
whenever `C-c C-c' is pressed on a keyword. So we don't need to document
the function anymore. Instead, we could drop a note about the cache in
(info "(org) The very busy C-c C-c key").

>>           (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)
>>
>>
> I have integrated most of your refactored version except for this portion.
> Above will do a false match if that "HTTP.." string is present in the FILE
> body too! I have retained my version of only that part where the search
> happens only inside the url-retrieve header. The search is also faster in
> the case of failure as it does not have to search through the whole file
> before declaring a fail.. as only the header is searched.

OK. Then the following at least doesn't have the overhead of creating
a string:


(with-current-buffer (url-retrieve-synchronously file)
  (goto-char (point-min))
  ;; Move point to after the url-retrieve header.
  (search-forward "\n\n" nil 'move)
  ;; Search for the success code only in the url-retrieve header.
  (if (save-excursion (re-search-backward "HTTP.*\\s-+200\\s-OK" nil t))
      ;; Update the cache `org--file-cache' and return contents.
      (puthash file
               (buffer-substring-no-properties (point) (point-max))
               org--file-cache)
    (funcall (if noerror #'message #'user-error)
             "Unable to fetch file from %S"
             file)))

Also, mind the full stop at the end of the comments.

Regards,

-- 
Nicolas Goaziou