Re: [O] Is it possible to remove org-completing-read and org-completing-read-no-i?

2015-08-04 Thread Bastien Guerry
Hi,

Rasmus  writes:

> I think we had a discussion following another thread on this topic
> (probably a thread you started), and I believe switching to
> `completing-read' is on the table after 8.3 has been released...

Yes.  Can someone take this?

-- 
 Bastien



Re: [O] Is it possible to remove org-completing-read and org-completing-read-no-i?

2015-07-28 Thread Rasmus
Hi,

Oleh Krehel  writes:

> I'd like to remove them in favor of using `org-icompleting-read'
> everywhere (or better yet, `completing-read').

I think we had a discussion following another thread on this topic
(probably a thread you started), and I believe switching to
`completing-read' is on the table after 8.3 has been released...

> An even better thing to do would be to use "lexical-binding: t".

Of course.  AFAIK lexical binding is not supported by Emacs 24.3.  Thus,
it can only happen after 8.3 has been released.

Rasmus

-- 
Dung makes an excellent fertilizer




[O] Is it possible to remove org-completing-read and org-completing-read-no-i?

2015-07-28 Thread Oleh Krehel

Hi all,

I'd like to remove them in favor of using `org-icompleting-read'
everywhere (or better yet, `completing-read').

`org-completing-read-no-i' doesn't do much, is called twice and can be
replaced with a let binding wrapper.

`org-completing-read' could be updated this way:

(defmacro with-org-minibuffer-keys (&rest body)
  "Minibuffer read with SPACE being a normal character."
  `(let ((enable-recursive-minibuffers t)
 (minibuffer-local-completion-map
  (copy-keymap minibuffer-local-completion-map)))
 (org-defkey minibuffer-local-completion-map " " 'self-insert-command)
 (org-defkey minibuffer-local-completion-map "?" 'self-insert-command)
 (org-defkey minibuffer-local-completion-map (kbd "C-c !") 
'org-time-stamp-inactive)
 ,@body))

(with-org-minibuffer-keys
(org-icompleting-read args))

This change will simplify the code nicely.  One example that bothers me
is `org-tags-completion-function': it's essentially a hacky closure that
relies on several dynamic variables being set:
`org-last-tags-completion-table' and
`org-add-colon-after-tag-completion' are implicit arguments to this
function.

With the proposed change, each use of `org-tags-completion-function'
will be in conjunction with `org-icompleting-read'. That means a new
function can be written:

(defun org-read-tags (tag-list &optional colon)
  (let ((org-last-tags-completion-table tag-list)
(org-add-colon-after-tag-completion colon))
(org-icompleting-read
 "Tag: " 'org-tags-completion-function
 ;; ...
 )))

I think it's a better interface, since the coupling is now made
explicit, instead of having to remember to set
`org-last-tags-completion-table' and
`org-add-colon-after-tag-completion' each time
`org-tags-completion-function' is to be used.

An even better thing to do would be to use "lexical-binding: t", remove
`org-tags-completion-function' and have it be a real closure inside
`org-read-tags'. Is there any wish or effort to move Org to
lexical-binding? Adding it would allow us to get rid of those dynamic
variables and `org-tags-completion-function' altogether and have the
lambda enclose on tag-list and colon instead.

regards,
Oleh