[PATCH] org-insert-link: allow ido usage when inserting links

[PATCH] org-insert-link: allow ido usage when inserting links

[tony day]

This time with patch inlined.

I had a look through and couldn't see an obvious reason why you can't use ido with org-insert-link, so here's a patch to enable it.

I haven't looked at using ido for editing links yet, but I figure org-capture would be a good pattern to do this.  The other thought here is to add an 'org:' link type so you can fire up ido just like org-capture (not sure what non-ido org-capture looks like).

This is my first patch, so please let me know if I'm not doing things right.

Tony

[PATCH] org-insert-link: allow ido usage when inserting links

* lisp/org.el (org-insert-link): added all-links to cleanly create prefix+stored links for use in ido
(org-i-read-file-name): new defun to allow ido to read a file: link if allowed

TINYCHANGE
---
 lisp/org.el | 39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 1c18d70..a918cfc 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -9397,7 +9397,7 @@ be used as the default description."
 	 tmphist ; byte-compile incorrectly complains about this
 	 (link link-location)
 	 (abbrevs org-link-abbrev-alist-local)
-	 entry file all-prefixes auto-desc)
+	 entry file all-links all-prefixes auto-desc)
     (cond
      (link-location) ; specified by arg, just use it.
      ((org-in-regexp org-bracket-link-regexp 1)
@@ -9443,19 +9443,19 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
 				 org-link-types))
       (unwind-protect
 	  (progn
+	    (setq all-links (append
+			     (mapcar 'car org-stored-links)
+			     (mapcar 'cadr org-stored-links)
+			     (mapcar (lambda (x) (concat x ":"))
+				     all-prefixes)))
+	    (setq all-links (delete nil all-links))
 	    (setq link
-		  (let ((org-completion-use-ido nil)
-			(org-completion-use-iswitchb nil))
-		    (org-completing-read
-		     "Link: "
-		     (append
-		      (mapcar (lambda (x) (list (concat x ":")))
-			      all-prefixes)
-		      (mapcar 'car org-stored-links)
-		      (mapcar 'cadr org-stored-links))
-		     nil nil nil
-		     'tmphist
-		     (caar org-stored-links))))
+		  (org-completing-read
+		   "Link: "
+		   all-links
+		   nil nil nil
+		   'tmphist
+		   (caar org-stored-links)))
 	    (if (not (string-match "\\S-" link))
 		(error "No link selected"))
 	    (mapc (lambda(l)
@@ -9542,7 +9542,7 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
 (defun org-file-complete-link (&optional arg)
   "Create a file link using completion."
   (let (file link)
-    (setq file (read-file-name "File: "))
+    (setq file (org-i-read-file-name "File: "))
     (let ((pwd (file-name-as-directory (expand-file-name ".")))
 	  (pwd1 (file-name-as-directory (abbreviate-file-name
 					 (expand-file-name ".")))))
@@ -9560,6 +9560,17 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
        (t (setq link (concat "file:" file)))))
     link))
 
+(defun org-i-read-file-name (&rest args)
+  "Read-file-name using `ido-mode' speedup if available."
+  (org-without-partial-completion
+   (if (and org-completion-use-ido
+            (fboundp 'ido-read-file-name)
+            (boundp 'ido-mode) ido-mode
+            (listp (second args)))
+       (let ((ido-enter-matching-directory nil))
+         (apply 'ido-read-file-name args))
+     (apply 'read-file-name args))))
+
 (defun org-completing-read (&rest args)
   "Completing-read with SPACE being a normal character."
   (let ((enable-recursive-minibuffers t)
-- 
1.7.12

Re: [PATCH] org-insert-link: allow ido usage when inserting links

[Giovanni Ridolfi]

Hi, Tony,

thanks for submitting the patch,


however I suspect it is longer than 20 lines. 
Therefore it could be applied only if you've 
assigned the copyright to the FSF. For more infos please refer to:

http://orgmode.org/worg/org-contribute.html

Would it be possible for you?

Thanks,

Giovanni


----- Messaggio originale -----
Da: tony day <zygomega@gmail.com>
A: emacs-orgmode@gnu.org
Cc: 
Inviato: Venerdì 14 Settembre 2012 11:21
Oggetto: [O] [PATCH] org-insert-link: allow ido usage when inserting links

This time with patch inlined.

I had a look through and couldn't see an obvious reason why you can't use ido with org-insert-link, so here's a patch to enable it.

I haven't looked at using ido for editing links yet, but I figure org-capture would be a good pattern to do this.  The other thought here is to add an 'org:' link type so you can fire up ido just like org-capture (not sure what non-ido org-capture looks like).

This is my first patch, so please let me know if I'm not doing things right.

Tony

[PATCH] org-insert-link: allow ido usage when inserting links

* lisp/org.el (org-insert-link): added all-links to cleanly create prefix+stored links for use in ido
(org-i-read-file-name): new defun to allow ido to read a file: link if allowed

TINYCHANGE
---
lisp/org.el | 39 +++++++++++++++++++++++++--------------
1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 1c18d70..a918cfc 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -9397,7 +9397,7 @@ be used as the default description."
     tmphist ; byte-compile incorrectly complains about this
     (link link-location)
     (abbrevs org-link-abbrev-alist-local)
-     entry file all-prefixes auto-desc)
+     entry file all-links all-prefixes auto-desc)
     (cond
      (link-location) ; specified by arg, just use it.
      ((org-in-regexp org-bracket-link-regexp 1)
@@ -9443,19 +9443,19 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
                 org-link-types))
       (unwind-protect
      (progn
+        (setq all-links (append
+                 (mapcar 'car org-stored-links)
+                 (mapcar 'cadr org-stored-links)
+                 (mapcar (lambda (x) (concat x ":"))
+                     all-prefixes)))
+        (setq all-links (delete nil all-links))
        (setq link
-          (let ((org-completion-use-ido nil)
-            (org-completion-use-iswitchb nil))
-            (org-completing-read
-             "Link: "
-             (append
-              (mapcar (lambda (x) (list (concat x ":")))
-                  all-prefixes)
-              (mapcar 'car org-stored-links)
-              (mapcar 'cadr org-stored-links))
-             nil nil nil
-             'tmphist
-             (caar org-stored-links))))
+          (org-completing-read
+           "Link: "
+           all-links
+           nil nil nil
+           'tmphist
+           (caar org-stored-links)))
        (if (not (string-match "\\S-" link))
        (error "No link selected"))
        (mapc (lambda(l)
@@ -9542,7 +9542,7 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
(defun org-file-complete-link (&optional arg)
   "Create a file link using completion."
   (let (file link)
-    (setq file (read-file-name "File: "))
+    (setq file (org-i-read-file-name "File: "))
     (let ((pwd (file-name-as-directory (expand-file-name ".")))
      (pwd1 (file-name-as-directory (abbreviate-file-name
                     (expand-file-name ".")))))
@@ -9560,6 +9560,17 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
        (t (setq link (concat "file:" file)))))
     link))

+(defun org-i-read-file-name (&rest args)
+  "Read-file-name using `ido-mode' speedup if available."
+  (org-without-partial-completion
+   (if (and org-completion-use-ido
+            (fboundp 'ido-read-file-name)
+            (boundp 'ido-mode) ido-mode
+            (listp (second args)))
+       (let ((ido-enter-matching-directory nil))
+         (apply 'ido-read-file-name args))
+     (apply 'read-file-name args))))
+
(defun org-completing-read (&rest args)
   "Completing-read with SPACE being a normal character."
   (let ((enable-recursive-minibuffers t)
-- 
1.7.12

Re: [PATCH] org-insert-link: allow ido usage when inserting links

[Bastien]

Hi Tony,

tony day <zygomega@gmail.com> writes:

> [PATCH] org-insert-link: allow ido usage when inserting links

I'm now marking this patch as not applicable in the patchwork,
and I've archived it.  Please resend it when the FSF papers are
in order.  

Also double-check the formatting of the ChangeLog entry: use 
the active voice, start sentences with an uppercase letter, end 
it with a "." and use two spaces after each... then you'll have
the perfect patch :)

Thanks!

-- 
 Bastien

[PATCH] org-insert-link: allow ido usage when inserting links

[tony day]

From a8f301277e15bc786fa63bbcce3ba1afb85c46aa Mon Sep 17 00:00:00 2001
From: Tony Day <zygomega@gmail.com>
Date: Mon, 10 Sep 2012 13:54:38 +1000
Subject: [PATCH 41/41] org-insert-link: allow ido usage when inserting links

* lisp/org.el (org-insert-link): added all-links to cleanly create prefix+st
(org-i-read-file-name): new defun to allow ido to read a file: link if allowed
---
 lisp/org.el |   39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 1c18d70..a918cfc 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -9397,7 +9397,7 @@ be used as the default description."
 	 tmphist ; byte-compile incorrectly complains about this
 	 (link link-location)
 	 (abbrevs org-link-abbrev-alist-local)
-	 entry file all-prefixes auto-desc)
+	 entry file all-links all-prefixes auto-desc)
     (cond
      (link-location) ; specified by arg, just use it.
      ((org-in-regexp org-bracket-link-regexp 1)
@@ -9443,19 +9443,19 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
 				 org-link-types))
       (unwind-protect
 	  (progn
+	    (setq all-links (append
+			     (mapcar 'car org-stored-links)
+			     (mapcar 'cadr org-stored-links)
+			     (mapcar (lambda (x) (concat x ":"))
+				     all-prefixes)))
+	    (setq all-links (delete nil all-links))
 	    (setq link
-		  (let ((org-completion-use-ido nil)
-			(org-completion-use-iswitchb nil))
-		    (org-completing-read
-		     "Link: "
-		     (append
-		      (mapcar (lambda (x) (list (concat x ":")))
-			      all-prefixes)
-		      (mapcar 'car org-stored-links)
-		      (mapcar 'cadr org-stored-links))
-		     nil nil nil
-		     'tmphist
-		     (caar org-stored-links))))
+		  (org-completing-read
+		   "Link: "
+		   all-links
+		   nil nil nil
+		   'tmphist
+		   (caar org-stored-links)))
 	    (if (not (string-match "\\S-" link))
 		(error "No link selected"))
 	    (mapc (lambda(l)
@@ -9542,7 +9542,7 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
 (defun org-file-complete-link (&optional arg)
   "Create a file link using completion."
   (let (file link)
-    (setq file (read-file-name "File: "))
+    (setq file (org-i-read-file-name "File: "))
     (let ((pwd (file-name-as-directory (expand-file-name ".")))
 	  (pwd1 (file-name-as-directory (abbreviate-file-name
 					 (expand-file-name ".")))))
@@ -9560,6 +9560,17 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
        (t (setq link (concat "file:" file)))))
     link))
 
+(defun org-i-read-file-name (&rest args)
+  "Read-file-name using `ido-mode' speedup if available."
+  (org-without-partial-completion
+   (if (and org-completion-use-ido
+            (fboundp 'ido-read-file-name)
+            (boundp 'ido-mode) ido-mode
+            (listp (second args)))
+       (let ((ido-enter-matching-directory nil))
+         (apply 'ido-read-file-name args))
+     (apply 'read-file-name args))))
+
 (defun org-completing-read (&rest args)
   "Completing-read with SPACE being a normal character."
   (let ((enable-recursive-minibuffers t)
-- 
1.7.9.6 (Apple Git-31.1)

Re: [PATCH] org-insert-link: allow ido usage when inserting links

[Nicolas Goaziou]

Hello,

tony day <zygomega@gmail.com> writes:

Thanks for submitting a patch. Here are a few comments.

> From a8f301277e15bc786fa63bbcce3ba1afb85c46aa Mon Sep 17 00:00:00 2001
> From: Tony Day <zygomega@gmail.com>
> Date: Mon, 10 Sep 2012 13:54:38 +1000
> Subject: [PATCH 41/41] org-insert-link: allow ido usage when inserting
> links
> * lisp/org.el (org-insert-link): added all-links to cleanly create prefix+st
> (org-i-read-file-name): new defun to allow ido to read a file: link if
> allowed

Entries should end with a period (not the title, though). Also, if you
haven't signed FSF papers yet, you should append "TINYCHANGE" on a line
on its own.

> ---
>  lisp/org.el |   39 +++++++++++++++++++++++++--------------
>  1 file changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 1c18d70..a918cfc 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -9397,7 +9397,7 @@ be used as the default description."
>  	 tmphist ; byte-compile incorrectly complains about this
>  	 (link link-location)
>  	 (abbrevs org-link-abbrev-alist-local)
> -	 entry file all-prefixes auto-desc)
> +	 entry file all-links all-prefixes auto-desc)
>      (cond
>       (link-location) ; specified by arg, just use it.
>       ((org-in-regexp org-bracket-link-regexp 1)
> @@ -9443,19 +9443,19 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
>  				 org-link-types))
>        (unwind-protect
>  	  (progn
> +	    (setq all-links (append
> +			     (mapcar 'car org-stored-links)
> +			     (mapcar 'cadr org-stored-links)
> +			     (mapcar (lambda (x) (concat x ":"))
> +				     all-prefixes)))
> +	    (setq all-links (delete nil all-links))

This should be (delq nil all-links).

>  	    (setq link
> -		  (let ((org-completion-use-ido nil)
> -			(org-completion-use-iswitchb nil))
> -		    (org-completing-read
> -		     "Link: "
> -		     (append
> -		      (mapcar (lambda (x) (list (concat x ":")))
> -			      all-prefixes)
> -		      (mapcar 'car org-stored-links)
> -		      (mapcar 'cadr org-stored-links))
> -		     nil nil nil
> -		     'tmphist
> -		     (caar org-stored-links))))
> +		  (org-completing-read
> +		   "Link: "
> +		   all-links
> +		   nil nil nil
> +		   'tmphist
> +		   (caar org-stored-links)))

I don't see the interest of this change nor how it is related to
allowing ido usage to insert links. Can

  (append
    (mapcar (lambda (x) (list (concat x ":"))) all-prefixes)
            (mapcar 'car org-stored-links)
            (mapcar 'cadr org-stored-links))

contain nil values?

If so, adding a (delq nil (append ...)) should be enough. This should be
a separate patch anyway.

> +(defun org-i-read-file-name (&rest args)
> +  "Read-file-name using `ido-mode' speedup if available."
> +  (org-without-partial-completion
> +   (if (and org-completion-use-ido
> +            (fboundp 'ido-read-file-name)
> +            (boundp 'ido-mode) ido-mode
> +            (listp (second args)))
> +       (let ((ido-enter-matching-directory nil))
> +         (apply 'ido-read-file-name args))
> +     (apply 'read-file-name args))))

Ok. There are a couple of places where this could be used
(`org-file-complete-link' for example). You should describe ARGS in the
docstring, though (writing, at least, that they refer to arguments from
`read-file-name').

Also, I'm not sure about the name. `completing-read' became
`org-icompleting-read'. Shouldn't `read-file-name' become
`org-iread-file-name'? 


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] org-insert-link: allow ido usage when inserting links

[tony day]

On 11 Oct 2012, at 23:23, Nicolas Goaziou <n.goaziou@gmail.com> wrote:

> Thanks for submitting a patch. Here are a few comments.

Hi Nicolas, thanks for taking the time to go through the code.  I will resubmit the patch in a separate mail (I didn't know whether I could respond to your suggestions and submit a new patch in the same mail).

> Entries should end with a period (not the title, though). Also, if you
> haven't signed FSF papers yet, you should append "TINYCHANGE" on a line
> on its own.

I have signed the FSF papers and they have been processed.

> I don't see the interest of this change nor how it is related to
> allowing ido usage to insert links. Can
> 
>  (append
>    (mapcar (lambda (x) (list (concat x ":"))) all-prefixes)
>            (mapcar 'car org-stored-links)
>            (mapcar 'cadr org-stored-links))
> 
> contain nil values?
> 
> If so, adding a (delq nil (append ...)) should be enough. This should be
> a separate patch anyway.

The problem is actualy the list bit, which causes a bug when using ido (but not when using normal completion).

Having gone through it again, I don't think the append can contain nil values, so I removed that bit.

> Shouldn't `read-file-name' become
> `org-iread-file-name'? 

Agreed and changed.

I don't think the patch can be split into two - the bug from list is only a bug if ido is used.

Here's some test code it case it helps:

- unit test
    #+begin_src emacs-lisp
      ;;(setq org-stored-links nil)
      (setq org-stored-links 
            '((#("file:~/stuff/org/bugz.org::*current debugging" 28 35 (fontified t org-category "bugz" line-prefix #("*" 0 1 (face org-hide)) wrap-prefix #("    " 0 4 (face org-indent)) face org-level-2) 36 45 (fontified t org-category "bugz" line-prefix #("*" 0 1 (face org-hide)) wrap-prefix #("    " 0 4 (face org-indent)) face org-level-2)) "current debugging")))
      ;;(setq org-stored-links
      ;;      '((#("file:~/stuff/org/bugz.org::*test link 2" 28 32 (fontified t line-prefix #("**" 0 2 (face org-hide)) wrap-prefix #("      " 0 6 (face org-indent)) face org-level-3) 33 37 (fontified t line-prefix #("**" 0 2 (face org-hide)) wrap-prefix #("      " 0 6 (face org-indent)) face org-level-3) 38 39 (fontified t line-prefix #("**" 0 2 (face org-hide)) wrap-prefix #("      " 0 6 (face org-indent)) face org-level-3)) "test link 2") (#("file:~/stuff/org/bugz.org::*test link 1" 28 32 (fontified t line-prefix #("**" 0 2 (face org-hide)) wrap-prefix #("      " 0 6 (face org-indent)) face org-level-3) 33 37 (fontified t line-prefix #("**" 0 2 (face org-hide)) wrap-prefix #("      " 0 6 (face org-indent)) face org-level-3) 38 39 (fontified t line-prefix #("**" 0 2 (face org-hide)) wrap-prefix #("      " 0 6 (face org-indent)) face org-level-3)) "test link 1")))
      (setq abbrevs org-link-abbrev-alist-local)
      (setq all-prefixes (append org-link-types
                                 (mapcar 'car abbrevs)
                                 (mapcar 'car org-link-abbrev-alist)))
      (setq all-links (append
                       (mapcar 'cadr org-stored-links)
                       (mapcar (lambda (x) (concat x ":"))
                               all-prefixes)
                       (mapcar 'car org-stored-links)))
      ;;(setq all-links (delete nil all-links))
      (print (loop for link in all-links
                   collect
                   (list link)))  
       
  #+end_src


Tony

Re: [PATCH] org-insert-link: allow ido usage when inserting links

[Samuel Wales]

Slighly different from this, I wonder if anybody has anything similar:

I have blog entries with a :Live-URL: property.  What I'd like is to
use ido to complete on the header that contains that property, and
create a link with the value of that property as the URL and the
header offered as default for the description.

Maybe this is too different?

-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com