From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [PATCH] org-insert-link: allow ido usage when inserting links Date: Thu, 11 Oct 2012 14:23:28 +0200 Message-ID: <87k3ux42pr.fsf@gmail.com> References: <9264AF9E-715F-45FF-88D1-8EAB5E94E751@gmail.com> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([208.118.235.92]:44640) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMHrd-00037F-2N for emacs-orgmode@gnu.org; Thu, 11 Oct 2012 08:27:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMHrU-0002Fs-B5 for emacs-orgmode@gnu.org; Thu, 11 Oct 2012 08:27:45 -0400 Received: from mail-wi0-f177.google.com ([209.85.212.177]:35412) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMHrU-0002Fl-0g for emacs-orgmode@gnu.org; Thu, 11 Oct 2012 08:27:36 -0400 Received: by mail-wi0-f177.google.com with SMTP id hj13so1714326wib.12 for ; Thu, 11 Oct 2012 05:27:34 -0700 (PDT) In-Reply-To: <9264AF9E-715F-45FF-88D1-8EAB5E94E751@gmail.com> (tony day's message of "Thu, 11 Oct 2012 15:19:19 +1100") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: tony day Cc: emacs-orgmode@gnu.org Hello, tony day writes: Thanks for submitting a patch. Here are a few comments. > From a8f301277e15bc786fa63bbcce3ba1afb85c46aa Mon Sep 17 00:00:00 2001 > From: Tony Day > 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