Re: [QUESTION] Re: [PATCH] make org-attach-url download function as an option
> Indeed, as long as org-attach introduced new async actions. Those hooks will > have problem if they requires files is downloading finished. Also > `make-thread' > does not have process sentinel. That's also an problem. > > Does anyone have better idea? You can wrap the call to org-attach-url-function into with-mutex and run hooks in another thread ustilising the same mutex. Note that the heading contents (including :DIR: property), point, current buffer, and window might be changed at the time the hook thread will be called. You will need to perform some sanity checks and potentially cleanup after org-attach-url-function if the heading was modified in some undesired way. The above assumes that org-attach-url-function returns after the file is attached. This might not be the case if org-attach-url-function calls async shell command. This should be either documented or also handled in some way. Best, Ihor stardiviner writes: > Ihor Radchenko writes: > >> I do not know an answer to your question, but would like to point out >> that make-thread will return immidietealy and all the following code >> will run before the download finishes: >> >> (run-hook-with-args 'org-attach-after-change-hook attach-dir) >> (org-attach-tag) >> (cond ((eq org-attach-store-link-p 'attached)... >> >> At least the hooks and org-attach-tag would expect that the file is >> attached already. > > Indeed, as long as org-attach introduced new async actions. Those hooks will > have problem if they requires files is downloading finished. Also > `make-thread' > does not have process sentinel. That's also an problem. > > Does anyone have better idea? > >> >> Best, >> Ihor >> >> stardiviner writes: >> >>> I got solution for async org-attach-url now. Use `make-thread` for async >>> downloading is simple. >>> >>> Here is the code prototype, but it has a problem, seems `apply` part code >>> does >>> not really downloading file. I don't know why. Does anybody knows the >>> reason? >>> >>> #+begin_src diff >>> modified lisp/org-attach.el >>> @@ -110,6 +110,12 @@ (defcustom org-attach-method 'cp >>> (const :tag "Hard Link" ln) >>> (const :tag "Symbol Link" lns))) >>> >>> +(defcustom org-attach-url-function 'url-copy-file >>> + "The download file function to use in org-attach-url." >>> + :type '(choice (const 'url-copy-file)) >>> + :safe #'functionp >>> + :group 'org-attach) >>> + >>> (defcustom org-attach-expert nil >>>"Non-nil means do not show the splash buffer with the attach dispatcher." >>>:group 'org-attach >>> @@ -503,7 +509,12 @@ (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) (make-thread >>> + (lambda () >>> + ;; (url-copy-file file attach-file) >>> + ;; FIXME This seems does not really download file. >>> Don't know why. >>> + (apply org-attach-url-function '(file attach-file))) >>> + "org-attach-url downloading"))) >>>(run-hook-with-args 'org-attach-after-change-hook attach-dir) >>>(org-attach-tag) >>>(cond ((eq org-attach-store-link-p 'attached) >>> #+end_src >>> >>> Bastien writes: >>> Hi, stardiviner writes: > I found when network is bad and slow, or the download file is big, the > org-attach-url will suspend Emacs for a long time. User might have to > cancel > downloading, and start again later. Indeed, this might be annoying. At the same time, it is not unreasonable to expect the user to know what size is the contents he is willing to attach to an Org node. > I hope to make "org-attach-url" download file asynchronously. But function > org-attach-attach hardcoded this function for 'url method. Here is a > patch to > make it into an option. (FWIW, I could not find the patch.) I think you are on the right track when trying to enhance the 'url package. Maybe url-copy-file should be asynchronous and url could provide url-copy-file-synchronously (to mimic the url-retrieve and url-retrieve-synchronously pair)? Until Emacs has a function to copy a URL's contents asynchronously, I'd rather not add this functionality in Org. >>> >>> >>> -- >>> [ stardiviner ] >>>I try to make every word tell the meaning that I want to express. >>> >>>Blog: https://stardiviner.github.io/ >>>IRC(freenode): stardiviner, Matrix: stardiviner >>>GPG: F09F650D7D674819892591401B5DF1C95AE89AC3 >>> > > > -- > [ stardiviner ] >I try to make every word tell the meaning that I want to express. > >Blog: https://stardiviner.
Re: [QUESTION] Re: [PATCH] make org-attach-url download function as an option
Ihor Radchenko writes: > I do not know an answer to your question, but would like to point out > that make-thread will return immidietealy and all the following code > will run before the download finishes: > > (run-hook-with-args 'org-attach-after-change-hook attach-dir) > (org-attach-tag) > (cond ((eq org-attach-store-link-p 'attached)... > > At least the hooks and org-attach-tag would expect that the file is > attached already. Indeed, as long as org-attach introduced new async actions. Those hooks will have problem if they requires files is downloading finished. Also `make-thread' does not have process sentinel. That's also an problem. Does anyone have better idea? > > Best, > Ihor > > stardiviner writes: > >> I got solution for async org-attach-url now. Use `make-thread` for async >> downloading is simple. >> >> Here is the code prototype, but it has a problem, seems `apply` part code >> does >> not really downloading file. I don't know why. Does anybody knows the reason? >> >> #+begin_src diff >> modified lisp/org-attach.el >> @@ -110,6 +110,12 @@ (defcustom org-attach-method 'cp >>(const :tag "Hard Link" ln) >>(const :tag "Symbol Link" lns))) >> >> +(defcustom org-attach-url-function 'url-copy-file >> + "The download file function to use in org-attach-url." >> + :type '(choice (const 'url-copy-file)) >> + :safe #'functionp >> + :group 'org-attach) >> + >> (defcustom org-attach-expert nil >>"Non-nil means do not show the splash buffer with the attach dispatcher." >>:group 'org-attach >> @@ -503,7 +509,12 @@ (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) (make-thread >> + (lambda () >> +;; (url-copy-file file attach-file) >> +;; FIXME This seems does not really download file. >> Don't know why. >> +(apply org-attach-url-function '(file attach-file))) >> + "org-attach-url downloading"))) >>(run-hook-with-args 'org-attach-after-change-hook attach-dir) >>(org-attach-tag) >>(cond ((eq org-attach-store-link-p 'attached) >> #+end_src >> >> Bastien writes: >> >>> Hi, >>> >>> stardiviner writes: >>> I found when network is bad and slow, or the download file is big, the org-attach-url will suspend Emacs for a long time. User might have to cancel downloading, and start again later. >>> >>> Indeed, this might be annoying. At the same time, it is not >>> unreasonable to expect the user to know what size is the contents he >>> is willing to attach to an Org node. >>> I hope to make "org-attach-url" download file asynchronously. But function org-attach-attach hardcoded this function for 'url method. Here is a patch to make it into an option. >>> >>> (FWIW, I could not find the patch.) >>> >>> I think you are on the right track when trying to enhance the 'url >>> package. Maybe url-copy-file should be asynchronous and url could >>> provide url-copy-file-synchronously (to mimic the url-retrieve and >>> url-retrieve-synchronously pair)? >>> >>> Until Emacs has a function to copy a URL's contents asynchronously, >>> I'd rather not add this functionality in Org. >> >> >> -- >> [ stardiviner ] >>I try to make every word tell the meaning that I want to express. >> >>Blog: https://stardiviner.github.io/ >>IRC(freenode): stardiviner, Matrix: stardiviner >>GPG: F09F650D7D674819892591401B5DF1C95AE89AC3 >> -- [ stardiviner ] I try to make every word tell the meaning that I want to express. Blog: https://stardiviner.github.io/ IRC(freenode): stardiviner, Matrix: stardiviner GPG: F09F650D7D674819892591401B5DF1C95AE89AC3
Re: [QUESTION] Re: [PATCH] make org-attach-url download function as an option
I do not know an answer to your question, but would like to point out that make-thread will return immidietealy and all the following code will run before the download finishes: (run-hook-with-args 'org-attach-after-change-hook attach-dir) (org-attach-tag) (cond ((eq org-attach-store-link-p 'attached)... At least the hooks and org-attach-tag would expect that the file is attached already. Best, Ihor stardiviner writes: > I got solution for async org-attach-url now. Use `make-thread` for async > downloading is simple. > > Here is the code prototype, but it has a problem, seems `apply` part code does > not really downloading file. I don't know why. Does anybody knows the reason? > > #+begin_src diff > modified lisp/org-attach.el > @@ -110,6 +110,12 @@ (defcustom org-attach-method 'cp > (const :tag "Hard Link" ln) > (const :tag "Symbol Link" lns))) > > +(defcustom org-attach-url-function 'url-copy-file > + "The download file function to use in org-attach-url." > + :type '(choice (const 'url-copy-file)) > + :safe #'functionp > + :group 'org-attach) > + > (defcustom org-attach-expert nil >"Non-nil means do not show the splash buffer with the attach dispatcher." >:group 'org-attach > @@ -503,7 +509,12 @@ (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) (make-thread > + (lambda () > + ;; (url-copy-file file attach-file) > + ;; FIXME This seems does not really download file. > Don't know why. > + (apply org-attach-url-function '(file attach-file))) > + "org-attach-url downloading"))) >(run-hook-with-args 'org-attach-after-change-hook attach-dir) >(org-attach-tag) >(cond ((eq org-attach-store-link-p 'attached) > #+end_src > > Bastien writes: > >> Hi, >> >> stardiviner writes: >> >>> I found when network is bad and slow, or the download file is big, the >>> org-attach-url will suspend Emacs for a long time. User might have to cancel >>> downloading, and start again later. >> >> Indeed, this might be annoying. At the same time, it is not >> unreasonable to expect the user to know what size is the contents he >> is willing to attach to an Org node. >> >>> I hope to make "org-attach-url" download file asynchronously. But function >>> org-attach-attach hardcoded this function for 'url method. Here is a patch >>> to >>> make it into an option. >> >> (FWIW, I could not find the patch.) >> >> I think you are on the right track when trying to enhance the 'url >> package. Maybe url-copy-file should be asynchronous and url could >> provide url-copy-file-synchronously (to mimic the url-retrieve and >> url-retrieve-synchronously pair)? >> >> Until Emacs has a function to copy a URL's contents asynchronously, >> I'd rather not add this functionality in Org. > > > -- > [ stardiviner ] >I try to make every word tell the meaning that I want to express. > >Blog: https://stardiviner.github.io/ >IRC(freenode): stardiviner, Matrix: stardiviner >GPG: F09F650D7D674819892591401B5DF1C95AE89AC3 > -- Ihor Radchenko, PhD, Center for Advancing Materials Performance from the Nanoscale (CAMP-nano) State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China Email: yanta...@gmail.com, ihor_radche...@alumni.sutd.edu.sg
[QUESTION] Re: [PATCH] make org-attach-url download function as an option
I got solution for async org-attach-url now. Use `make-thread` for async downloading is simple. Here is the code prototype, but it has a problem, seems `apply` part code does not really downloading file. I don't know why. Does anybody knows the reason? #+begin_src diff modified lisp/org-attach.el @@ -110,6 +110,12 @@ (defcustom org-attach-method 'cp (const :tag "Hard Link" ln) (const :tag "Symbol Link" lns))) +(defcustom org-attach-url-function 'url-copy-file + "The download file function to use in org-attach-url." + :type '(choice (const 'url-copy-file)) + :safe #'functionp + :group 'org-attach) + (defcustom org-attach-expert nil "Non-nil means do not show the splash buffer with the attach dispatcher." :group 'org-attach @@ -503,7 +509,12 @@ (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) (make-thread + (lambda () + ;; (url-copy-file file attach-file) + ;; FIXME This seems does not really download file. Don't know why. + (apply org-attach-url-function '(file attach-file))) + "org-attach-url downloading"))) (run-hook-with-args 'org-attach-after-change-hook attach-dir) (org-attach-tag) (cond ((eq org-attach-store-link-p 'attached) #+end_src Bastien writes: > Hi, > > stardiviner writes: > >> I found when network is bad and slow, or the download file is big, the >> org-attach-url will suspend Emacs for a long time. User might have to cancel >> downloading, and start again later. > > Indeed, this might be annoying. At the same time, it is not > unreasonable to expect the user to know what size is the contents he > is willing to attach to an Org node. > >> I hope to make "org-attach-url" download file asynchronously. But function >> org-attach-attach hardcoded this function for 'url method. Here is a patch to >> make it into an option. > > (FWIW, I could not find the patch.) > > I think you are on the right track when trying to enhance the 'url > package. Maybe url-copy-file should be asynchronous and url could > provide url-copy-file-synchronously (to mimic the url-retrieve and > url-retrieve-synchronously pair)? > > Until Emacs has a function to copy a URL's contents asynchronously, > I'd rather not add this functionality in Org. -- [ stardiviner ] I try to make every word tell the meaning that I want to express. Blog: https://stardiviner.github.io/ IRC(freenode): stardiviner, Matrix: stardiviner GPG: F09F650D7D674819892591401B5DF1C95AE89AC3
Re: [PATCH] make org-attach-url download function as an option
Bastien writes: > Hi, > > stardiviner writes: > >> I found when network is bad and slow, or the download file is big, the >> org-attach-url will suspend Emacs for a long time. User might have to cancel >> downloading, and start again later. > > Indeed, this might be annoying. At the same time, it is not > unreasonable to expect the user to know what size is the contents he > is willing to attach to an Org node. It's not the URL file size problem, sometime network is bad. I meet some situations like downloading 1M file might use 5 minutes. Not 1M file or 1G file difference. > >> I hope to make "org-attach-url" download file asynchronously. But function >> org-attach-attach hardcoded this function for 'url method. Here is a patch to >> make it into an option. > > (FWIW, I could not find the patch.) Aha, I forgot the patch. I attached now. The patch does not provide an async function to download. Just provide an easy way for user to use other async functions. > > I think you are on the right track when trying to enhance the 'url > package. Maybe url-copy-file should be asynchronous and url could > provide url-copy-file-synchronously (to mimic the url-retrieve and > url-retrieve-synchronously pair)? Actually I did check out url-copy-file-synchronously source code, try to mimic an async version function. But seems I can't implement it. I will post an email to Emacs-dev mailing list whether this can be improved. > > Until Emacs has a function to copy a URL's contents asynchronously, > I'd rather not add this functionality in Org. Emacs async functionality is always bad. Waiting for Emacs get better async support might need a very long time. I still think simply provide an simple entry for user to change downloading function is a simpler option. WDYT? -- [ stardiviner ] I try to make every word tell the meaning that I want to express. Blog: https://stardiviner.github.io/ IRC(freenode): stardiviner, Matrix: stardiviner GPG: F09F650D7D674819892591401B5DF1C95AE89AC3 From 61e26e148e68deb03f26dddb2a88beae5b6869f7 Mon Sep 17 00:00:00 2001 From: stardiviner Date: Wed, 29 Apr 2020 21:38:34 +0800 Subject: [PATCH] lisp/org-attach.el: support custom url file downloading function lisp/org-attach.el (org-attach-url-function): make download function as a defcustom option. So that user can use async function or other functions instead. --- lisp/org-attach.el | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lisp/org-attach.el b/lisp/org-attach.el index 57d1360fc..e33551f40 100644 --- a/lisp/org-attach.el +++ b/lisp/org-attach.el @@ -110,6 +110,12 @@ (defcustom org-attach-method 'cp (const :tag "Hard Link" ln) (const :tag "Symbol Link" lns))) +(defcustom org-attach-url-function 'url-copy-file + "The download file function to use in org-attach-url." + :type '(choice (const 'url-copy-file)) + :safe #'functionp + :group 'org-attach) + (defcustom org-attach-expert nil "Non-nil means do not show the splash buffer with the attach dispatcher." :group 'org-attach @@ -504,7 +510,7 @@ (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) (apply org-attach-url-function file (list attach-file (run-hook-with-args 'org-attach-after-change-hook attach-dir) (org-attach-tag) (cond ((eq org-attach-store-link-p 'attached) -- 2.26.2 signature.asc Description: PGP signature
Re: [PATCH] make org-attach-url download function as an option
Hi, stardiviner writes: > I found when network is bad and slow, or the download file is big, the > org-attach-url will suspend Emacs for a long time. User might have to cancel > downloading, and start again later. Indeed, this might be annoying. At the same time, it is not unreasonable to expect the user to know what size is the contents he is willing to attach to an Org node. > I hope to make "org-attach-url" download file asynchronously. But function > org-attach-attach hardcoded this function for 'url method. Here is a patch to > make it into an option. (FWIW, I could not find the patch.) I think you are on the right track when trying to enhance the 'url package. Maybe url-copy-file should be asynchronous and url could provide url-copy-file-synchronously (to mimic the url-retrieve and url-retrieve-synchronously pair)? Until Emacs has a function to copy a URL's contents asynchronously, I'd rather not add this functionality in Org. -- Bastien
[PATCH] make org-attach-url download function as an option
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 I found when network is bad and slow, or the download file is big, the org-attach-url will suspend Emacs for a long time. User might have to cancel downloading, and start again later. I hope to make "org-attach-url" download file asynchronously. But function org-attach-attach hardcoded this function for 'url method. Here is a patch to make it into an option. So user can specify a custom function to download file. # == BTW, I'm wandering why function "url-retrieve" download file is not complete and failed. Bellowing code snippets are my try: Use function ~url-copy-file~ works fine: #+begin_src emacs-lisp :eval no (url-copy-file "https://img1.doubanio.com/view/photo/s_ratio_poster/public/p2592521688.webp"; (expand-file-name "~/kk.webp")) #+end_src I check out function ~url-copy-file~ implement, try to implement an async version like this: #+begin_src emacs-lisp (defun url-copy-file-asynchronous (url newname &optional ok-if-already-exists &rest _ignored) "Copy URL to NEWNAME. Both arguments must be strings. Signal a `file-already-exists' error if file NEWNAME already exists, unless a third argument OK-IF-ALREADY-EXISTS is supplied and non-nil. An integer as third argument means request confirmation if NEWNAME already exists." (and (file-exists-p newname) (or (not ok-if-already-exists) (and (integerp ok-if-already-exists) (not (yes-or-no-p (format "File %s already exists; copy to it anyway? " newname) (signal 'file-already-exists (list "File already exists" newname))) (let* ((buffer (or (url-retrieve url (lambda (&rest ignored))) (signal 'file-missing (list "Opening URL" "No such file or directory" url (handle (with-current-buffer buffer (mm-dissect-buffer t (let ((mm-attachment-file-modes (default-file-modes))) (mm-save-part-to-file handle newname)) (kill-buffer buffer) (mm-destroy-parts handle))) (put 'copy-file 'url-file-handlers #'url-copy-file-asynchronous) #+end_src #+begin_src emacs-lisp (url-copy-file-asynchronous "https://img1.doubanio.com/view/photo/s_ratio_poster/public/p2592521688.webp"; (expand-file-name "~/kk.webp")) #+end_src But it download seems incomplete and failed. Don't know why. Also there are extra async libraries support download file asynchronously. Like "deferred", "async" etc. Those libraries can be used for user define their own functions. Is you have better idea, please tell me, thanks in advance. - -- [ stardiviner ] I try to make every word tell the meaning that I want to express. Blog: https://stardiviner.github.io/ IRC(freenode): stardiviner, Matrix: stardiviner GPG: F09F650D7D674819892591401B5DF1C95AE89AC3 -BEGIN PGP SIGNATURE- iQFIBAEBCAAyFiEE8J9lDX1nSBmJJZFAG13xyVromsMFAl6phdUUHG51bWJjaGls ZEBnbWFpbC5jb20ACgkQG13xyVromsNswQgAjuFtM/+WU0EXdpQOx3C4kymgUTex jEZgw96JlfHSP3JcoOuA5Ll2F+nXShWwIeGJAe1aRgjLTy9O5XSZO6BdlxoN2QRB RfrZvl5X5cJu9NIyiwhSYbIMM8+DJKHSp/nBGIimXwDoNU/ye43E4OmApVKfWjf0 TmMXUzLnXWSxdSubFe/M7SEGUaarFI5y2bNwytBMOQr7q1yY6UgynlB6Vda06mqb BO/AnJgYtP69XvghK14NJ6/X8nol5oFijLtt67QUNKBCdUlp3tx/G+6lFebGruHS kZ6PjTzoAwZOk5l7lTx9mG+dle1g/7rNce1TRVFP+cT1+qS5+7UHhwfnwg== =pYDj -END PGP SIGNATURE-
