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
