[PATCH] Stable key assignment for org-fast-tag-selection

[PATCH] Stable key assignment for org-fast-tag-selection

[emacs-orgmode]

I noticed that the code to "automatically assign a character" in the
org-fast-tag-selection function assigns a different character depending
on whether or not the tag exists in the current heading.

To see the problem, do the following steps:

- Eval (setq org-use-fast-tag-selection t)
- Eval (setq org-fast-tag-selection-single-key "expert")
- Create a heading with tags :a1:a2:
- Create another heading with tags :a1:a2:
- Type "C-c a", "C-c a", "C-c a", "C-c a"
- Notice how it changes from a1 to a2

Below is a patch to make the character assignment stable, given that all
of the same tags exist in the file each time it's run.

(The tabs messed up the indentation in the git-diff output.)

Regards,
Jason


diff --git a/lisp/org.el b/lisp/org.el
index b2b08ae..fd49f3e 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -12947,7 +12947,8 @@ possibly with grouping information.  TODO-TABLE is a similar table with
 TODO keywords, should these have keys assigned to them.
 If the keys are nil, a-z are automatically assigned.
 Returns the new tags string, or nil to not change the current settings."
-  (let* ((fulltable (append table todo-table))
+  (let* ((table (sort table (lambda (a b) (string< (car a) (car b)))))
+        (fulltable (append table todo-table))
         (maxlen (apply 'max (mapcar
                              (lambda (x)
                                (if (stringp (car x)) (string-width (car x)) 0))

Re: [PATCH] Stable key assignment for org-fast-tag-selection

[Bastien]

Hi Jason,

emacs-orgmode@dunsmor.com writes:

> - Eval (setq org-use-fast-tag-selection t)
> - Eval (setq org-fast-tag-selection-single-key "expert")
> - Create a heading with tags :a1:a2:
> - Create another heading with tags :a1:a2:
> - Type "C-c a", "C-c a", "C-c a", "C-c a"
> - Notice how it changes from a1 to a2

I reproduced this, well spotted.

> Below is a patch to make the character assignment stable, given that all
> of the same tags exist in the file each time it's run.

Thanks for the patch!

I was not able to apply it using the pw utility, so I made the change
myself -- I forgot to do a "git commit --amend --author" tho, sorry!

-- 
 Bastien

Re: [PATCH] Stable key assignment for org-fast-tag-selection

[Matt Lundin]

Bastien <bastien.guerry@wikimedia.fr> writes:

> emacs-orgmode@dunsmor.com writes:
>
>> - Eval (setq org-use-fast-tag-selection t)
>> - Eval (setq org-fast-tag-selection-single-key "expert")
>> - Create a heading with tags :a1:a2:
>> - Create another heading with tags :a1:a2:
>> - Type "C-c a", "C-c a", "C-c a", "C-c a"
>> - Notice how it changes from a1 to a2
>
> I reproduced this, well spotted.
>
>> Below is a patch to make the character assignment stable, given that all
>> of the same tags exist in the file each time it's run.
>
> Thanks for the patch!
>
> I was not able to apply it using the pw utility, so I made the change
> myself -- I forgot to do a "git commit --amend --author" tho, sorry!

This patch breaks grouping in fast tag selection, since the sort
function ignores the grouping information provided in org-tag-alist.

Here's a sample org-tag-alist:

(setq org-tag-alist '((:startgroup . nil)
		      ("desk" . ?d)
		      ("email" . ?m)
		      ("hack" . ?k)
		      ("net" . ?n)
		      ("phone" . ?p)
		      (:endgroup . nil)
		      (:startgroup . nil)
		      ("home" . ?h)
		      ("yard" . ?y)
		      ("errands" . ?e)
		      (:endgroup . nil)
		      (:startgroup . nil)
		      ("read" . ?r)
		      ("script" .?s)
		      ("think" . ?t)
		      ("travel" . ?j)
		      (:endgroup . nil)))

And here's the prompt for org-tag-alist:

--8<---------------cut here---------------start------------->8---
Inherited:  inbox per prof
Current:    
							   Next change exits
}
}
}
{ { { [d] desk      [m] email     [e] errands   [k] hack      [h] home      
  [n] net       [p] phone     [r] read      [s] script    [t] think     
  [j] travel    [y] yard      
--8<---------------cut here---------------end--------------->8---

Best,
Matt

Re: Re: [PATCH] Stable key assignment for org-fast-tag-selection

[Jason Dunsmore]

Matt Lundin <mdl@imapmail.org> writes:

> Bastien <bastien.guerry@wikimedia.fr> writes:
>
>> emacs-orgmode@dunsmor.com writes:
>>
>>> - Eval (setq org-use-fast-tag-selection t)
>>> - Eval (setq org-fast-tag-selection-single-key "expert")
>>> - Create a heading with tags :a1:a2:
>>> - Create another heading with tags :a1:a2:
>>> - Type "C-c a", "C-c a", "C-c a", "C-c a"
>>> - Notice how it changes from a1 to a2
>>
>> I reproduced this, well spotted.
>>
>>> Below is a patch to make the character assignment stable, given that all
>>> of the same tags exist in the file each time it's run.
>>
>> Thanks for the patch!
>>
>> I was not able to apply it using the pw utility, so I made the change
>> myself -- I forgot to do a "git commit --amend --author" tho, sorry!
>
> This patch breaks grouping in fast tag selection, since the sort
> function ignores the grouping information provided in org-tag-alist.
>
> Here's a sample org-tag-alist:
>
> (setq org-tag-alist '((:startgroup . nil)
> 		      ("desk" . ?d)
> 		      ("email" . ?m)
> 		      ("hack" . ?k)
> 		      ("net" . ?n)
> 		      ("phone" . ?p)
> 		      (:endgroup . nil)
> 		      (:startgroup . nil)
> 		      ("home" . ?h)
> 		      ("yard" . ?y)
> 		      ("errands" . ?e)
> 		      (:endgroup . nil)
> 		      (:startgroup . nil)
> 		      ("read" . ?r)
> 		      ("script" .?s)
> 		      ("think" . ?t)
> 		      ("travel" . ?j)
> 		      (:endgroup . nil)))
>
> And here's the prompt for org-tag-alist:
>
> Inherited:  inbox per prof
> Current:    
> 							   Next change exits
> }
> }
> }
> { { { [d] desk      [m] email     [e] errands   [k] hack      [h] home      
>   [n] net       [p] phone     [r] read      [s] script    [t] think     
>   [j] travel    [y] yard      

Good catch.  I see the problem.

--8<---------------cut here---------------start------------->8---
(sort org-tag-alist (lambda (a b) (string< (car a) (car b))))
--8<---------------cut here---------------end--------------->8---

Yields:

--8<---------------cut here---------------start------------->8---
((:endgroup)
 (:endgroup)
 (:endgroup)
 (:startgroup)
 (:startgroup)
 (:startgroup)
 ("desk" . 100)
 ("email" . 109)
 ("errands" . 101)
 ("hack" . 107)
 ("home" . 104)
 ("net" . 110)
 ("phone" . 112)
 ("read" . 114)
 ("script" . 115)
 ("think" . 116)
 ("travel" . 106)
 ("yard" . 121))
--8<---------------cut here---------------end--------------->8---

And a bunch of code relies on the position of the :startgroup
and :endgroup dummy tags.

I wonder why this design was chosen instead of using a list of alists
for grouping.  For example:

--8<---------------cut here---------------start------------->8---
(setq org-tag-alist '((("desk" . ?d)
                       ("email" . ?m)
                       ("hack" . ?k)
                       ("net" . ?n)
                       ("phone" . ?p))
                      (("home" . ?h)
                       ("yard" . ?y)
                       ("errands" . ?e))
		      (("read" . ?r)
                       ("script" .?s)
                       ("think" . ?t)
                       ("travel" . ?j))))
--8<---------------cut here---------------end--------------->8---

Maybe org-tag-alist could either be an alist or a list of alists, and a
simple (listp (caar org-tag-alist)) test could be used to see what form
org-tag-alist is in and then handle accordingly.  Does anybody see a
problem with that approach?

This doesn't seem like a trivial change, so my original patch should
probably be reverted until this gets implemented properly.

Regards,
Jason