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

2012-10-11 Thread Nicolas Goaziou
Hello,

tony day zygom...@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 zygom...@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: [O] [PATCH] org-insert-link: allow ido usage when inserting links

2012-10-11 Thread 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



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

2012-10-11 Thread tony day


On 11 Oct 2012, at 23:23, Nicolas Goaziou n.goaz...@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








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

2012-10-10 Thread tony day

From a8f301277e15bc786fa63bbcce3ba1afb85c46aa Mon Sep 17 00:00:00 2001
From: Tony Day zygom...@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: [O] [PATCH] org-insert-link: allow ido usage when inserting links

2012-09-22 Thread Bastien
Hi Tony,

tony day zygom...@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



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

2012-09-14 Thread 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: [O] [PATCH] org-insert-link: allow ido usage when inserting links

2012-09-14 Thread 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 zygom...@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