Re: [O] [PATCH] * org-insert-link: use ido when inserting links
Hello, tony day zygom...@gmail.com writes: And a revised patch for your reviewing pleasure :) Ok. I have applied it and added you to the list of contributors. Thank you again for the patch. Regards, -- Nicolas Goaziou
Re: [O] [PATCH] * org-insert-link: use ido when inserting links
Hello, Thanks for considering my suggestions. So here are new ones: tony day zygom...@gmail.com writes: org.el (org-insert-link): removed a list within the list of link creation that was causing a bug when using ido. Nitpick: Removed a list Judging by your test case, it's the (mapcar 'cadr org-stored-links) part that cause problems, isn't it? I'm not sure why it should belong to the provided collection. Wouldn't dropping it solve the ido problem? Removed the hard coded iswitch and ido switches. Just wondering: what will happen if an user wants to use iswitchb? Changed the order of prefixes so http came up first. Please do not add unrelated features during a patch review. By the way, I'm not sure to agree with you: it /is/ meaningful to have user-defined link abbrevs before default types. (org-iread-file-name): created a function that can use ido-read-file-name if flagged as ok. Nitpick: Created... (org-file-complete-link): referenced org-iread-file-name. Nitpick: Referenced... --- lisp/org.el | 44 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/lisp/org.el b/lisp/org.el index bdfc919..41c2572 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -9311,24 +9311,22 @@ Use TAB to complete link prefixes, then RET for type-specific completion support ;; Fake a link history, containing the stored links. (setq tmphist (append (mapcar 'car org-stored-links) org-insert-link-history)) - (setq all-prefixes (append (mapcar 'car abbrevs) - (mapcar 'car org-link-abbrev-alist) - org-link-types)) + (setq all-prefixes (append org-link-types + (mapcar 'car abbrevs) + (mapcar 'car org-link-abbrev-alist))) For now, you can remove that part, unless it is tied to the current patch. But having http first doesn't sound convincing. (unwind-protect (progn (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: +(append + (mapcar 'cadr org-stored-links) + (mapcar (lambda (x) (concat x :)) + all-prefixes) Why do you need to change that order? + (mapcar 'car org-stored-links)) +nil nil nil + 'tmphist + (cadr org-stored-links))) It means the default value will be the second stored link, if any? Is it intended? (if (not (string-match \\S- link)) (error No link selected)) (mapc (lambda(l) @@ -9422,7 +9420,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-iread-file-name File: )) (let ((pwd (file-name-as-directory (expand-file-name .))) (pwd1 (file-name-as-directory (abbreviate-file-name (expand-file-name .) @@ -9440,6 +9438,20 @@ Use TAB to complete link prefixes, then RET for type-specific completion support (t (setq link (concat file: file) link)) +(defun org-iread-file-name (rest args) + Read-file-name using `ido-mode' speedup if available. +ARGS are arguments that may be passed to `ido-read-file-name' or `read-file-name'. +See `read-file-name' for a description of parameters. + + (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) It looks good. You can answer to this mail, no need to post the patch on another thread. Regards, -- Nicolas Goaziou
[O] [PATCH] * org-insert-link: use ido when inserting links
On 12 Oct 2012, at 23:50, Nicolas Goaziou n.goaz...@gmail.com wrote: Thanks for considering my suggestions. So here are new ones: And a revised patch for your reviewing pleasure :) Judging by your test case, it's the (mapcar 'cadr org-stored-links) part that cause problems, isn't it? I'm not sure why it should belong to the provided collection. Wouldn't dropping it solve the ido problem? No, but the combination of removing this and removing the list wrapper solves the ordering issues I was trying to fix. I wasn't sure why the cadr was there either. Removed the hard coded iswitch and ido switches. Just wondering: what will happen if an user wants to use iswitchb? Works for the first part and reverts to no assist on file: selection. Changed the order of prefixes so http came up first. Please do not add unrelated features during a patch review. By the way, I'm not sure to agree with you: it /is/ meaningful to have user-defined link abbrevs before default types. Excellent advice (yes, even the nitpicks :) From 31c9855ca6db95d10ca09611f749d74074b19b08 Mon Sep 17 00:00:00 2001 From: Tony Day zygom...@gmail.com Date: Fri, 12 Oct 2012 14:39:53 +1100 Subject: [PATCH] * org-insert-link: use ido when inserting links org.el (org-insert-link): remove a list within the list of link creation that causes a bug when using ido. Remove the hard coded iswitch and ido switches. (org-iread-file-name): create a function that can use ido-read-file-name if flagged as ok. (org-file-complete-link): reference org-iread-file-name. --- lisp/org.el | 37 - 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/lisp/org.el b/lisp/org.el index bdfc919..89acb81 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -9317,18 +9317,15 @@ Use TAB to complete link prefixes, then RET for type-specific completion support (unwind-protect (progn (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: + (append + (mapcar (lambda (x) (concat x :)) + all-prefixes) + (mapcar 'car org-stored-links)) + nil nil nil + 'tmphist + (caar org-stored-links))) (if (not (string-match \\S- link)) (error No link selected)) (mapc (lambda(l) @@ -9422,7 +9419,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-iread-file-name File: )) (let ((pwd (file-name-as-directory (expand-file-name .))) (pwd1 (file-name-as-directory (abbreviate-file-name (expand-file-name .) @@ -9440,6 +9437,20 @@ Use TAB to complete link prefixes, then RET for type-specific completion support (t (setq link (concat file: file) link)) +(defun org-iread-file-name (rest args) + Read-file-name using `ido-mode' speedup if available. +ARGS are arguments that may be passed to `ido-read-file-name' or `read-file-name'. +See `read-file-name' for a description of parameters. + + (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)
[O] [PATCH] * org-insert-link: use ido when inserting links
org.el (org-insert-link): removed a list within the list of link creation that was causing a bug when using ido. Removed the hard coded iswitch and ido switches. Changed the order of prefixes so http came up first. (org-iread-file-name): created a function that can use ido-read-file-name if flagged as ok. (org-file-complete-link): referenced org-iread-file-name. --- lisp/org.el | 44 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/lisp/org.el b/lisp/org.el index bdfc919..41c2572 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -9311,24 +9311,22 @@ Use TAB to complete link prefixes, then RET for type-specific completion support ;; Fake a link history, containing the stored links. (setq tmphist (append (mapcar 'car org-stored-links) org-insert-link-history)) - (setq all-prefixes (append (mapcar 'car abbrevs) -(mapcar 'car org-link-abbrev-alist) -org-link-types)) + (setq all-prefixes (append org-link-types + (mapcar 'car abbrevs) + (mapcar 'car org-link-abbrev-alist))) (unwind-protect (progn (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: + (append + (mapcar 'cadr org-stored-links) + (mapcar (lambda (x) (concat x :)) + all-prefixes) + (mapcar 'car org-stored-links)) + nil nil nil + 'tmphist + (cadr org-stored-links))) (if (not (string-match \\S- link)) (error No link selected)) (mapc (lambda(l) @@ -9422,7 +9420,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-iread-file-name File: )) (let ((pwd (file-name-as-directory (expand-file-name .))) (pwd1 (file-name-as-directory (abbreviate-file-name (expand-file-name .) @@ -9440,6 +9438,20 @@ Use TAB to complete link prefixes, then RET for type-specific completion support (t (setq link (concat file: file) link)) +(defun org-iread-file-name (rest args) + Read-file-name using `ido-mode' speedup if available. +ARGS are arguments that may be passed to `ido-read-file-name' or `read-file-name'. +See `read-file-name' for a description of parameters. + + (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)
