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