emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] New remote resource download policy
@ 2022-06-12 14:43 Timothy
  2022-06-12 16:18 ` Daniel Fleischer
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Timothy @ 2022-06-12 14:43 UTC (permalink / raw)
  To: emacs-orgmode


[-- Attachment #1.1: Type: text/plain, Size: 492 bytes --]

Hi All,

As was raised in the `#+include: URL' thread
(<https://list.orgmode.org/877d5sd7yu.fsf@gmail.com>), currently Org will
automatically download files without confirmation in various circumstances.

This patch introduces two variables to control Org’s attitude towards
downloading files, and hooks them into the relevant parts of the codebase.

When prompting for downloading, this uses an approach borrowed from file local
variable confirmation.

All the best,
Timothy

[-- Attachment #1.2: Type: text/html, Size: 3186 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-Add-setting-for-remote-file-download-policy.patch --]
[-- Type: text/x-patch, Size: 8990 bytes --]

From 4f3437a2386e2ffdf37c99d476fa5ea3481b8d3c Mon Sep 17 00:00:00 2001
From: TEC <tec@tecosaur.com>
Date: Sun, 12 Jun 2022 22:37:42 +0800
Subject: [PATCH] org: Add setting for remote file download policy

* lisp/org.el (org-download-remote-resources,
org-safe-remote-resources): Two new customisations to configure the
policy for downloading remote resources.
(org--should-fetch-remote-resource-p, org--safe-remote-resource-p,
org--confirm-resource-safe, org-download-remote-resources): Introduce
the new function `org--should-fetch-remote-resource-p' for internal use
determining whether a remote resource should be downloaded according to
the download policy.  This function makes use of two helper functions,
`org--safe-remote-resource-p' and `org--confirm-resource-safe'.
(org-file-contents): Apply `org--safe-remote-resource-p' to file
downloading.

* lisp/org-persist.el (org-persist-write): Apply
`org--safe-remote-resource-p' to url downloading.

* lisp/org-attach.el (org-attach-attach): Apply
`org--safe-remote-resource-p' to url downloading.
---
 lisp/org-attach.el  |   6 ++-
 lisp/org-persist.el |   5 +-
 lisp/org.el         | 115 ++++++++++++++++++++++++++++++++++++++------
 3 files changed, 109 insertions(+), 17 deletions(-)

diff --git a/lisp/org-attach.el b/lisp/org-attach.el
index 5ee2b84b2..6f21ad716 100644
--- a/lisp/org-attach.el
+++ b/lisp/org-attach.el
@@ -525,7 +525,11 @@ (defun org-attach-attach (file &optional visit-dir method)
        ((eq method 'cp) (copy-file file attach-file))
        ((eq method 'ln) (add-name-to-file file attach-file))
        ((eq method 'lns) (make-symbolic-link file attach-file))
-       ((eq method 'url) (url-copy-file file attach-file)))
+       ((eq method 'url)
+        (if (or (not noninteractive) (org--should-fetch-remote-resource-p file))
+            (url-copy-file file attach-file)
+          (error "The remote resource %S is considered unsafe, and will not be downloaded."
+                 file))))
       (run-hook-with-args 'org-attach-after-change-hook attach-dir)
       (org-attach-tag)
       (cond ((eq org-attach-store-link-p 'attached)
diff --git a/lisp/org-persist.el b/lisp/org-persist.el
index 068f58cec..f49abe8cd 100644
--- a/lisp/org-persist.el
+++ b/lisp/org-persist.el
@@ -655,7 +655,10 @@ (defun org-persist-write:url (c collection)
                          (format "%s-%s.%s" persist-file (md5 path) ext))))
         (unless (file-exists-p (file-name-directory file-copy))
           (make-directory (file-name-directory file-copy) t))
-        (url-copy-file path file-copy 'overwrite)
+        (if (org--should-fetch-remote-resource-p path)
+            (url-copy-file path file-copy 'overwrite)
+          (error "The remote resource %S is considered unsafe, and will not be downloaded."
+                 path))
         (format "%s-%s.%s" persist-file (md5 path) ext)))))
 
 (defun org-persist-write:index (container _)
diff --git a/lisp/org.el b/lisp/org.el
index 8e7aadde5..3a8acaa8f 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -1352,6 +1352,32 @@ (defcustom org-file-apps
 			(string :tag "Command")
 			(function :tag "Function")))))
 
+(defcustom org-download-remote-resources 'prompt
+  "The policy applied to requests to obtain remote resources.
+
+This affects keywords like #+setupfile and #+incude on export,
+`org-persist-write:url',and `org-attach-attach' in
+non-interactive Emacs sessions.
+
+This recognises four possible values:
+- t, remote resources should always be downloaded.
+- prompt, you will be prompted to download resources nt considered safe.
+- safe, only resources considered safe will be downloaded.
+- nil, never download remote resources.
+
+A resource is considered safe if it matches one of the patterns
+in `org-safe-remote-resources'."
+  :group 'org
+  :type '(choice (const :tag "Always download remote resources" t)
+                 (const :tag "Prompt before downloading an unsafe resource" prompt)
+                 (const :tag "Only download resources considered safe" safe)
+                 (const :tag "Never download any resources" nil)))
+
+(defcustom org-safe-remote-resources nil
+  "A list of regexp patterns matching safe URIs."
+  :group 'org
+  :type '(list regexp))
+
 (defcustom org-open-non-existing-files nil
   "Non-nil means `org-open-file' opens non-existing files.
 
@@ -4466,21 +4492,25 @@ (defun org-file-contents (file &optional noerror nocache)
     (cond
      (cache)
      (is-url
-      (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 :noerror))
-	    ;; 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)
-	  nil)))
+      (if (org--should-fetch-remote-resource-p file)
+          (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 :noerror))
+                ;; 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)
+              nil))
+        (funcall (if noerror #'message #'user-error)
+                 "The remote resource %S is considered unsafe, and will not be downloaded."
+                 file)))
      (t
       (with-temp-buffer
         (condition-case nil
@@ -4493,6 +4523,61 @@ (defun org-file-contents (file &optional noerror nocache)
 		    file)
 	   nil)))))))
 
+(defun org--should-fetch-remote-resource-p (uri)
+  "Return non-nil if the URI should be fetched."
+  (or (eq org-download-remote-resources t)
+      (org--safe-remote-resource-p uri)
+      (and (eq org-download-remote-resources 'prompt)
+           (org--confirm-resource-safe uri))))
+
+(defun org--safe-remote-resource-p (uri)
+  "Return non-nil if URI is considered safe.
+This checks every pattern in `org-safe-remote-resources', and
+returns non-nil if any of them match."
+  (let (match-p (uri-patterns org-safe-remote-resources))
+    (while (and (not match-p) uri-patterns)
+      (setq match-p (string-match-p (car uri-patterns) uri)
+            uri-patterns (cdr uri-patterns)))
+    match-p))
+
+(defun org--confirm-resource-safe (uri)
+  "Ask the user if URI should be considered safe, returning non-nil if so."
+    (unless noninteractive
+      (let ((buf (get-buffer-create "*Org Remote Resource*")))
+        ;; Set up the contents of the *Local Variables* buffer.
+        (with-current-buffer buf
+          (erase-buffer)
+          (insert "An org-mode document would like to download "
+                  (propertize uri 'face '(:inherit org-link :weight normal))
+                  ", which is not considered safe.\n\n"
+                  "Do you want to download this?  You can type\n "
+                  (propertize "!" 'face 'success)
+                  " to download this resource, and permanantly mark it as safe.\n "
+                  (propertize "y" 'face 'warning)
+                  " to download this resource, just this once.\n "
+                  (propertize "n" 'face 'error)
+                  " to skip this resource.\n")
+          (setq-local cursor-type nil)
+          (set-buffer-modified-p nil)
+          (goto-char (point-min)))
+        ;; Display the buffer and read a choice.
+        (save-window-excursion
+          (pop-to-buffer buf)
+          (let* ((exit-chars '(?y ?n ?! ?\s))
+                 (prompt (format "Please type y, n, or !%s: "
+                                 (if (< (line-number-at-pos (point-max))
+                                        (window-body-height))
+                                     ""
+                                   ", or C-v/M-v to scroll")))
+                 char)
+            (setq char (read-char-choice prompt exit-chars))
+            (when (= char ?!)
+              (customize-push-and-save
+               'org-safe-remote-resources
+               (regexp-quote uri)))
+	    (prog1 (memq char '(?! ?\s ?y))
+	      (quit-window t)))))))
+
 (defun org-extract-log-state-settings (x)
   "Extract the log state setting from a TODO keyword string.
 This will extract info from a string like \"WAIT(w@/!)\"."
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] New remote resource download policy
  2022-06-12 14:43 [PATCH] New remote resource download policy Timothy
@ 2022-06-12 16:18 ` Daniel Fleischer
  2022-06-14  9:40 ` Robert Pluim
  2022-06-15 12:35 ` Max Nikulin
  2 siblings, 0 replies; 11+ messages in thread
From: Daniel Fleischer @ 2022-06-12 16:18 UTC (permalink / raw)
  To: Timothy; +Cc: emacs-orgmode

Timothy [2022-06-12 Sun 22:43] wrote:

> As was raised in the #+include: URL thread (https://list.orgmode.org/877d5sd7yu.fsf@gmail.com), currently Org will
> automatically download files without confirmation in various circumstances. 
>
> This patch introduces two variables to control Org’s attitude towards downloading files, and hooks them into the
> relevant parts of the codebase. 

Hi Timothy, looks very nice! I was also bothered by it, specifically in
the case of #+setupfile. Having a safe URI regexp list is a nice
addition, similar to `org-link-elisp-skip-confirm-regexp'. I'll
definitely give it a try.

-- 

Daniel Fleischer


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] New remote resource download policy
  2022-06-12 14:43 [PATCH] New remote resource download policy Timothy
  2022-06-12 16:18 ` Daniel Fleischer
@ 2022-06-14  9:40 ` Robert Pluim
  2022-06-22  9:58   ` Timothy
  2022-06-15 12:35 ` Max Nikulin
  2 siblings, 1 reply; 11+ messages in thread
From: Robert Pluim @ 2022-06-14  9:40 UTC (permalink / raw)
  To: Timothy; +Cc: emacs-orgmode

>>>>> On Sun, 12 Jun 2022 22:43:07 +0800, Timothy <tecosaur@gmail.com> said:

    Timothy> Hi All,
    Timothy> As was raised in the `#+include: URL' thread
    Timothy> (<https://list.orgmode.org/877d5sd7yu.fsf@gmail.com>), currently Org will
    Timothy> automatically download files without confirmation in various circumstances.

    Timothy> This patch introduces two variables to control Org’s attitude towards
    Timothy> downloading files, and hooks them into the relevant parts of the codebase.

I think that would be a good addition. 
 
    Timothy> +(defcustom org-download-remote-resources 'prompt
    Timothy> +  "The policy applied to requests to obtain remote resources.
    Timothy> +
    Timothy> +This affects keywords like #+setupfile and #+incude on export,
    Timothy> +`org-persist-write:url',and `org-attach-attach' in
    Timothy> +non-interactive Emacs sessions.
    Timothy> +
    Timothy> +This recognises four possible values:
    Timothy> +- t, remote resources should always be downloaded.
    Timothy> +- prompt, you will be prompted to download resources nt considered safe.

'prompted when downloading unsafe resources'

maybe?

Does this need a 'prompt-always option? I guess thatʼs what you get
with the default value of `org-safe-remote-resources', but maybe it
makes sense to be explicit?

Robert
-- 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] New remote resource download policy
  2022-06-12 14:43 [PATCH] New remote resource download policy Timothy
  2022-06-12 16:18 ` Daniel Fleischer
  2022-06-14  9:40 ` Robert Pluim
@ 2022-06-15 12:35 ` Max Nikulin
  2022-06-22 10:01   ` Timothy
  2 siblings, 1 reply; 11+ messages in thread
From: Max Nikulin @ 2022-06-15 12:35 UTC (permalink / raw)
  To: emacs-orgmode

On 12/06/2022 21:43, Timothy wrote:
> 
> As was raised in the #+include: URL thread 
> (https://list.orgmode.org/877d5sd7yu.fsf@gmail.com), currently Org will 
> automatically download files without confirmation in various circumstances.
> 
> This patch introduces two variables to control Org’s attitude towards 
> downloading files, and hooks them into the relevant parts of the codebase.

Timothy, thank you for efforts in this direction. In some sense you have 
done even more than I asked for. I tried you patch mostly to confirm 
that the protection can not be bypassed using file local variables. 
Since custom variables are not marked as safe, user is asked if values 
should be applied. Such behavior is consistent with my expectation.

> --- a/lisp/org-attach.el
> +++ b/lisp/org-attach.el
> @@ -525,7 +525,11 @@ (defun org-attach-attach (file &optional visit-dir method)
>         ((eq method 'cp) (copy-file file attach-file))
>         ((eq method 'ln) (add-name-to-file file attach-file))
>         ((eq method 'lns) (make-symbolic-link file attach-file))
> -       ((eq method 'url) (url-copy-file file attach-file)))
> +       ((eq method 'url)
> +        (if (or (not noninteractive) (org--should-fetch-remote-resource-p file))

I am confused by (not noninteractive). Does it mean that interactive 
call is enough to bypass protection? It may have sense it at this step 
there is no ambiguity what resources is fetched. On the other hand I am 
unsure concerning a case when `org-attach-attach' is a part of a larger 
command.

> +            (url-copy-file file attach-file)
> +          (error "The remote resource %S is considered unsafe, and will not be downloaded."
> +                 file))))


> +(defcustom org-download-remote-resources 'prompt

The name sounds like some function.

> +(defun org--confirm-resource-safe (uri)
> +  "Ask the user if URI should be considered safe, returning non-nil if so."
> +    (unless noninteractive
> +      (let ((buf (get-buffer-create "*Org Remote Resource*")))

I see your intention to add something fancy to the dialog. May `org-mks' 
be reused instead to avoid proliferation variants of rather similar UI code?

> +        ;; Set up the contents of the *Local Variables* buffer.
> +        (with-current-buffer buf
> +          (erase-buffer)
> +          (insert "An org-mode document would like to download "
> +                  (propertize uri 'face '(:inherit org-link :weight normal))
> +                  ", which is not considered safe.\n\n"
> +                  "Do you want to download this?  You can type\n "
> +                  (propertize "!" 'face 'success)
> +                  " to download this resource, and permanantly mark it as safe.\n "
> +                  (propertize "y" 'face 'warning)
> +                  " to download this resource, just this once.\n "

I am in doubts concerning "once". I tried "y" in a file having to 
"#+include:" of the same file. I did not get question for second 
include. I did not get prompt for this file anymore at all, even during 
next export. I modified the remote file, but stale content appeared 
during export. So the file was really downloaded once, but it is hardly 
in agreement with my expectations. Behavior is unrelated to this patch, 
concerning wording I am not sure, but I have no a better variant.

> +                  (propertize "n" 'face 'error)
> +                  " to skip this resource.\n")

 From "skip" I do not expect aborting of export.

I have an idea but unsure if it should be implemented. Consider 
`org-remote-resources-policy' custom variable that is a list of pairs 
(url-regexp . policy) for fine grain tuning instead of 2 variables. The 
price is more complicated structure, so higher chance of user error.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] New remote resource download policy
  2022-06-14  9:40 ` Robert Pluim
@ 2022-06-22  9:58   ` Timothy
  0 siblings, 0 replies; 11+ messages in thread
From: Timothy @ 2022-06-22  9:58 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 719 bytes --]

Hi Robert,

>     Timothy> +(defcustom org-download-remote-resources ’prompt
>
> ’prompted when downloading unsafe resources’
>
> maybe?
>
> Does this need a ’prompt-always option? I guess thatʼs what you get
> with the default value of `org-safe-remote-resources’, but maybe it
> makes sense to be explicit?

That isn’t the behaviour. `prompt' doesn’t /always/ prompt, just when the resource
in question is not considered safe according to `org--safe-remote-resource-p'.

So, `prompt-always' would be something else, but one can easily make prompt behave
that way by setting `org-safe-remote-resources' to nil, so I don’t think it
warrants an extra option.

All the best,
Timothy

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] New remote resource download policy
  2022-06-15 12:35 ` Max Nikulin
@ 2022-06-22 10:01   ` Timothy
  2022-06-22 16:55     ` Max Nikulin
  2022-06-25  7:50     ` Max Nikulin
  0 siblings, 2 replies; 11+ messages in thread
From: Timothy @ 2022-06-22 10:01 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 3028 bytes --]

Hi Max,

>> — a/lisp/org-attach.el
>> +++ b/lisp/org-attach.el
>> @@ -525,7 +525,11 @@ (defun org-attach-attach (file &optional visit-dir method)
>> [snip]
>> +        (if (or (not noninteractive) (org–should-fetch-remote-resource-p file))
>
> I am confused by (not noninteractive). Does it mean that interactive call is
> enough to bypass protection? It may have sense it at this step there is no
> ambiguity what resources is fetched. On the other hand I am unsure concerning a
> case when `org-attach-attach’ is a part of a larger command.

The idea here is that when this is done interactively the user will be aware of
the URL this is being applied to, and so it isn’t a risk. Let me know if this
assumption doesn’t hold.

>> +(defcustom org-download-remote-resources ’prompt
>
> The name sounds like some function.

Mmm. I could add `-policy' to that variable name perhaps.

>> +(defun org–confirm-resource-safe (uri)
>> +  “Ask the user if URI should be considered safe, returning non-nil if so.”
>> +    (unless noninteractive
>> +      (let ((buf (get-buffer-create “*Org Remote Resource*”)))
>
> I see your intention to add something fancy to the dialog. May `org-mks’ be
> reused instead to avoid proliferation variants of rather similar UI code?

Well, the thing here is that I’m explicitly trying to mimic the
file-local-variable dialog, and since a general form isn’t exposed by Emacs, a
little bit of proliferation seems like the best option to me.

>> +        ;; Set up the contents of the *Local Variables* buffer.
>
> I am in doubts concerning “once”. I tried “y” in a file having to “#+include:”
> of the same file. I did not get question for second include. I did not get
> prompt for this file anymore at all, even during next export. I modified the
> remote file, but stale content appeared during export. So the file was really
> downloaded once, but it is hardly in agreement with my expectations. Behavior is
> unrelated to this patch, concerning wording I am not sure, but I have no a
> better variant.

Ok, that is not the intended behaviour. I’ll see if I can work out what’s going
on here. Oh, and I should change that comment to “Set up the contents of the *Org
Remote Resource* buffer.”.

>> +                  (propertize “n” ’face ’error)
>> +                  “ to skip this resource.”)
>
> From “skip” I do not expect aborting of export.

Hmm, the “skip” action isn’t determined by the “is it safe” functions, but some
wording that doesn’t sound completely safe at least would be good.

> I have an idea but unsure if it should be implemented. Consider
> `org-remote-resources-policy’ custom variable that is a list of pairs
> (url-regexp . policy) for fine grain tuning instead of 2 variables. The price is
> more complicated structure, so higher chance of user error.

My initial reaction is that this feels like overkill.

All the best,
Timothy

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] New remote resource download policy
  2022-06-22 10:01   ` Timothy
@ 2022-06-22 16:55     ` Max Nikulin
  2022-06-29 15:27       ` Timothy
  2022-06-25  7:50     ` Max Nikulin
  1 sibling, 1 reply; 11+ messages in thread
From: Max Nikulin @ 2022-06-22 16:55 UTC (permalink / raw)
  To: emacs-orgmode

On 22/06/2022 17:01, Timothy wrote:
> 
>>> +(defun org–confirm-resource-safe (uri)
>>> +  “Ask the user if URI should be considered safe, returning non-nil if so.”
>>> +    (unless noninteractive
>>> +      (let ((buf (get-buffer-create “*Org Remote Resource*”)))
>>
>> I see your intention to add something fancy to the dialog. May `org-mks’ be
>> reused instead to avoid proliferation variants of rather similar UI code?
> 
> Well, the thing here is that I’m explicitly trying to mimic the
> file-local-variable dialog, and since a general form isn’t exposed by Emacs, a
> little bit of proliferation seems like the best option to me.

 From my point of view the result is rather close (prompt should be 
adjusted):

(let ((uri "https://orgmode.org"))
   (org-mks
    nil
    (format "An org-mode document would like to download %s, which is 
not considered safe.

Do you want to download this?"
	   (propertize uri 'face '(:inherit org-link :weight normal)))
    nil ; prompt
    `((,(propertize "!" 'face 'success)
       "download this resource, and permanantly mark it as safe.")
      (,(propertize "y" 'face 'warning)
       "to download this resource, just this once.")
      (,(propertize "n" 'face 'error)
       "skip this resource."))))



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] New remote resource download policy
  2022-06-22 10:01   ` Timothy
  2022-06-22 16:55     ` Max Nikulin
@ 2022-06-25  7:50     ` Max Nikulin
  1 sibling, 0 replies; 11+ messages in thread
From: Max Nikulin @ 2022-06-25  7:50 UTC (permalink / raw)
  To: emacs-orgmode

On 22/06/2022 17:01, Timothy wrote:
> 
>>> — a/lisp/org-attach.el
>>> +++ b/lisp/org-attach.el
>>> @@ -525,7 +525,11 @@ (defun org-attach-attach (file &optional visit-dir method)
>>> [snip]
>>> +        (if (or (not noninteractive) (org–should-fetch-remote-resource-p file))
>>
>> I am confused by (not noninteractive). Does it mean that interactive call is
>> enough to bypass protection? It may have sense it at this step there is no
>> ambiguity what resources is fetched. On the other hand I am unsure concerning a
>> case when `org-attach-attach’ is a part of a larger command.
> 
> The idea here is that when this is done interactively the user will be aware of
> the URL this is being applied to, and so it isn’t a risk. Let me know if this
> assumption doesn’t hold.

I am not sure what is the best option here. Despite `org-attach-attach' 
is an interactive command, URL code path most likely will be followed 
when this function is called from another one. *Currently* my opinion is 
that it is caller who is responsible for the decision whether the user 
explicitly asked for a particular resource so it may be considered safe. 
For example for `org-attach-url' probability that it is called by user 
directly is higher than it happens from another function.

So I am considering such variant: no heuristics is added to 
`org-attach-attach', but `org-attach-url' temporary adjusts the list of 
safe locations to bypass user prompt. Notice that I am not an active 
user of `org-attach'.

>>> +(defcustom org-download-remote-resources ’prompt
>>
>> The name sounds like some function.
> 
> Mmm. I could add `-policy' to that variable name perhaps.

It will be too long. Maybe org-remote-resources-policy, but I am leaving 
the decision up to you.

>>> +(defun org–confirm-resource-safe (uri)
>>> +  “Ask the user if URI should be considered safe, returning non-nil if so.”
>>> +    (unless noninteractive
>>> +      (let ((buf (get-buffer-create “*Org Remote Resource*”)))
>>
>> I see your intention to add something fancy to the dialog. May `org-mks’ be
>> reused instead to avoid proliferation variants of rather similar UI code?
> 
> Well, the thing here is that I’m explicitly trying to mimic the
> file-local-variable dialog, and since a general form isn’t exposed by Emacs, a
> little bit of proliferation seems like the best option to me.

I do not have strong opinion, I provided an example of `org-mks' reusing 
in another message.

>>> +                  (propertize “n” ’face ’error)
>>> +                  “ to skip this resource.”)
>>
>>  From “skip” I do not expect aborting of export.
> 
> Hmm, the “skip” action isn’t determined by the “is it safe” functions, but some
> wording that doesn’t sound completely safe at least would be good.

At least for export it works more like "Abort".

I have realized that e.g. 7zip, asking if it should overwrite an 
existing file, offers "Always" variant. Almost certainly I saw similar 
buttons in other dialogs on Windows as well. Unsure it it may be easily 
implemented, but the case "Allow all remote URLs for the current file" 
sounds like a valid option during export.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] New remote resource download policy
  2022-06-22 16:55     ` Max Nikulin
@ 2022-06-29 15:27       ` Timothy
  2022-06-30 16:57         ` Max Nikulin
  0 siblings, 1 reply; 11+ messages in thread
From: Timothy @ 2022-06-29 15:27 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 1563 bytes --]

Hi Max,

Max Nikulin <manikulin@gmail.com> writes:

>>> I see your intention to add something fancy to the dialog. May `org-mks’ be
>>> reused instead to avoid proliferation variants of rather similar UI code?
>> Well, the thing here is that I’m explicitly trying to mimic the
>> file-local-variable dialog, and since a general form isn’t exposed by Emacs, a
>> little bit of proliferation seems like the best option to me.
>
> From my point of view the result is rather close (prompt should be adjusted):
>
> (let ((uri “<https://orgmode.org>”))
>   (org-mks
>    nil
>    (format “An org-mode document would like to download %s, which is not
>   considered safe.
>
> Do you want to download this?”
> 	   (propertize uri ’face ’(:inherit org-link :weight normal)))
>    nil ; prompt
>    `((,(propertize “!” ’face ’success)
>       “download this resource, and permanantly mark it as safe.”)
>      (,(propertize “y” ’face ’warning)
>       “to download this resource, just this once.”)
>      (,(propertize “n” ’face ’error)
>       “skip this resource.”))))

I just tried this snippet and it looked quite different to me, that said I have
tweaked `org-mks' a bit in my config… (I initially found org-capture quite
ugly).

I also really don’t like how `org-mks' so forcefully grabs all keyboard input. I
can see it being nice to jump back to the buffer and see where the resource is
being used, which isn’t really possible using `org-mks'.

All the best,
Timothy

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] New remote resource download policy
  2022-06-29 15:27       ` Timothy
@ 2022-06-30 16:57         ` Max Nikulin
  2022-07-16  9:47           ` Timothy
  0 siblings, 1 reply; 11+ messages in thread
From: Max Nikulin @ 2022-06-30 16:57 UTC (permalink / raw)
  To: emacs-orgmode

On 29/06/2022 22:27, Timothy wrote:
> Max Nikulin writes:
> 
> I also really don’t like how `org-mks' so forcefully grabs all keyboard input. I
> can see it being nice to jump back to the buffer and see where the resource is
> being used, which isn’t really possible using `org-mks'.

An idea: a menu entry that displays location in the org file that caused 
the prompt. However it may require enough work to pass context data to 
the function rendering menu.

I see you point concerning blocking modal prompt and I do not like it as 
well. Have you seen the following thread:

Arthur Miller. Proposal: 'executable' org-capture-templaes. Wed, 22 Jun 
2022 14:13:51 +0200.
https://list.orgmode.org/AM9PR09MB49771CF015DAECBF3E5F955E96B29@AM9PR09MB4977.eurprd09.prod.outlook.com

If it were merged already, would it be enough for you to implement the 
dialog or the proposal lacks some features necessary for smooth user 
experience?

The reason why I asked about `org-mks' is expectation of UI consistency 
withing Org. It might help users having customized `display-buffer-alist'

Eric S Fraga. Re: Bug: org-no-popups disregards 
display-buffer-fallback-action Sun, 14 Nov 2021 19:37:30 +0000.
https://list.orgmode.org/87tugeedfp.fsf@ucl.ac.uk

Eric S Fraga. window management for logging and capturing notes. Sun, 10 
Oct 2021 18:01:56 +0100.
https://87zgrgke4b.fsf@ucl.ac.uk

I still do not like code adding a menu close enough to existing one, but 
I will not object any more.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] New remote resource download policy
  2022-06-30 16:57         ` Max Nikulin
@ 2022-07-16  9:47           ` Timothy
  0 siblings, 0 replies; 11+ messages in thread
From: Timothy @ 2022-07-16  9:47 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 1480 bytes --]

Hi Max,

I’ve just made a few more tweaks, tested it, and pushed this as 0583a0c. We may
want to tweak this further before the next Org release, but I think this is a
good enough starting point.

> An idea: a menu entry that displays location in the org file that caused the
> prompt. However it may require enough work to pass context data to the function
> rendering menu.

This could be added in future.

> I see you point concerning blocking modal prompt and I do not like it as well.
> Have you seen the following thread:
>
> Arthur Miller. Proposal: ’executable’ org-capture-templaes. Wed, 22 Jun 2022
> 14:13:51 +0200.
> <https://list.orgmode.org/AM9PR09MB49771CF015DAECBF3E5F955E96B29@AM9PR09MB4977.eurprd09.prod.outlook.com>
>
> If it were merged already, would it be enough for you to implement the dialog or
> the proposal lacks some features necessary for smooth user experience?

I have not read much of this thread, but from a brief glance — possibly.

> The reason why I asked about `org-mks’ is expectation of UI consistency withing
> Org. It might help users having customized `display-buffer-alist’

Here I was more thinking of UI consistency within Emacs.

> I still do not like code adding a menu close enough to existing one, but I will
> not object any more.

Since this is separate to the functionality, I’m going to push this for now, and
we can update the UI side of things later.

All the best,
Timothy

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-07-16  9:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-12 14:43 [PATCH] New remote resource download policy Timothy
2022-06-12 16:18 ` Daniel Fleischer
2022-06-14  9:40 ` Robert Pluim
2022-06-22  9:58   ` Timothy
2022-06-15 12:35 ` Max Nikulin
2022-06-22 10:01   ` Timothy
2022-06-22 16:55     ` Max Nikulin
2022-06-29 15:27       ` Timothy
2022-06-30 16:57         ` Max Nikulin
2022-07-16  9:47           ` Timothy
2022-06-25  7:50     ` Max Nikulin

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