Re: [PATCH] I updated patch by deleteing duplicate tags

[Christopher Miles <numbchild@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 6765 bytes --]


Kyle Meyer <kyle@kyleam.com> writes:

> Thanks for the patch.
>
> stardiviner writes:
>
>> On Wed, Dec 2, 2020 at 5:30 PM stardiviner <numbchild@gmail.com> wrote:
>>
>>> The default [C-c C-q] completing tags only retrieve tags from current
>>> buffer locally.
>>>
>>> By this patch, will merge both buffer-local tags and user defined global
>>> `org-tags-alist`.
>
> It does a bit more than that.  It uses org-global-tags-completion-table,
> which considers tags in all agenda files by default and takes into
> account org-tag-alist (as well as org-tag-persistent-alist) via the use
> of the org-current-tag-alist variable.
>

That's what I want. Why obviously user pre-defined tags can't be used globally.
Right? It should be.

>>> This is more reasonable.
>
> I'd guess that depends on the user.  I personally wouldn't like to see
> tags from all of my agenda files, and I'm fine not seeing
> org-tag{-persistent}-alist ones that aren't in the current buffer given
> that they have fast selection keys.

Currently tags selection is now slow, even with 100 pre-defined tags. Should be
fine. Tags sometimes is abundant. So if it is slow, then it means the command
=org-set-tags-command= should be optimized.

>
>> Subject: [PATCH] org.el: Complete tags from both global and buffer local
>>
>> * lisp/org.el: (org-fast-tag-selection): merge buffer local tags with
>> global alist of tags.
>
> Convention/consistency nits: spurious ":" after ".el" and
> s/merge/Merge/.

Updated. Thanks

>
>> ---
>>  lisp/org.el | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/lisp/org.el b/lisp/org.el
>> index 0e12e4b15..287b8c407 100644
>> --- a/lisp/org.el
>> +++ b/lisp/org.el
>> @@ -12256,10 +12256,13 @@ (defun org-fast-tag-selection (current inherited table &optional todo-table)
>>  		    (condition-case nil
>>  			(setq tg (completing-read
>>  				  "Tag: "
>> -				  (or buffer-tags
>> -				      (with-current-buffer buf
>> -					(setq buffer-tags
>> -					      (org-get-buffer-tags))))))
>> +				  (delq nil
>> +					(delete-dups
>> +					 (append (or buffer-tags
>> +						     (with-current-buffer buf
>> +						       (setq buffer-tags
>> +							     (org-get-buffer-tags))))
>> +						 (org-global-tags-completion-table))))))
>
> This change in behavior should come with a NEWS entry and a
> documentation update.  What the manual currently says is now stale:
>
>   - {{{kbd(TAB)}}} ::
>   
>     #+kindex: TAB
>     Enter a tag in the minibuffer, even if the tag is not in the
>     predefined list.  You can complete on all tags present in the
>     buffer.  You can also add several tags: just separate them with
>     a comma.
>
> As I mentioned above, though, I'm not sure always adding agenda tags is
> desirable.  However, I think it'd probably be safe to look at
> org-complete-tags-always-offer-all-agenda-tags as an indication of
> whether the user wants this behavior.  org-set-tags-command already
> considers that option when it generates the table that it passes to
> org-fast-tag-selection.  So perhaps we could just consider the table
> when calling completing-read for the tab key (something along the lines
> of the patch at the end of the email).
>

Indeed, I add condition on ~org-complete-tags-always-offer-all-agenda-tags~ now.

> Conceptually that's been discussed/tried before, but it was then backed
> out of:
>
>   * https://orgmode.org/list/F753E612-2D5D-4BA7-AF0C-D49C7A8DDA24@pobox.com/T/#u
>   * 647396464 (org.el: Include tags from `org-tag-alist' when completing
>     with the TAB key, 2012-03-27)
>   * d4ddcbb8b (Revert "org.el: Include tags from `org-tag-alist' when
>     completing with the TAB key.", 2012-04-10)
>   * acc7a0b2b (org.el: Include `org-tag-alist' in the list for tag
>     completions, 2012-03-27)
>

I checked this mailing list thread, the original commit use ~(mapcar 'car table)~.
It's more simple. I updated my code to this.

My patch only changed the ~completing-read~ part. And I found ido support option
is been removed in Org. I think this is right decision.

> At a quick glance, I think the patch below avoids the problems that led
> to 647396464 being reverted, but that'd need to be checked more
> carefully.
>
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 5b0ae389c..9383719e3 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -12139,7 +12139,7 @@ (defun org-fast-tag-selection (current inherited table &optional todo-table)
>  				  fulltable))))
>  	 (buf (current-buffer))
>  	 (expert (eq org-fast-tag-selection-single-key 'expert))
> -	 (buffer-tags nil)
> +	 (tab-tags nil)
>  	 (fwidth (+ maxlen 3 1 3))
>  	 (ncol (/ (- (window-width) 4) fwidth))
>  	 (i-face 'org-done)
> @@ -12274,16 +12274,22 @@ (defun org-fast-tag-selection (current inherited table &optional todo-table)
>  		    (setq current nil)
>  		    (when exit-after-next (setq exit-after-next 'now)))
>  		   ((= c ?\t)
> +                    (unless tab-tags
> +                      (setq tab-tags
> +                            (delq nil
> +                                  (mapcar (lambda (x)
> +                                            (let ((item (car-safe x)))
> +                                              (and (stringp item)
> +                                                   (list item))))
> +                                          (org--tag-add-to-alist
> +                                           (with-current-buffer buf
> +                                             (org-get-buffer-tags))
> +                                           table)))))
>  		    (condition-case nil
> -			(setq tg (completing-read
> -				  "Tag: "
> -				  (or buffer-tags
> -				      (with-current-buffer buf
> -					(setq buffer-tags
> -					      (org-get-buffer-tags))))))
> +                        (setq tg (completing-read "Tag: " tab-tags))
>  		      (quit (setq tg "")))
>  		    (when (string-match "\\S-" tg)
> -		      (cl-pushnew (list tg) buffer-tags :test #'equal)
> +		      (cl-pushnew (list tg) tab-tags :test #'equal)
>  		      (if (member tg current)
>  			  (setq current (delete tg current))
>  			(push tg current)))

Hmm, I tested your upper patch, works same. But looks safer. So I applied your
patch in my patch. Actually it's totally your code.... Hahaha

Anyway, I re-generated patch. Bother you to review it. :smile:

-- 
[ stardiviner ]
       I try to make every word tell the meaning that I want to express.

       Blog: https://stardiviner.github.io/
       IRC(freenode): stardiviner, Matrix: stardiviner
       GPG: F09F650D7D674819892591401B5DF1C95AE89AC3

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: new patch --]
[-- Type: text/x-patch, Size: 3844 bytes --]

From 85d11170985a612a8e6c847f56a5c5bf121b97d2 Mon Sep 17 00:00:00 2001
From: stardiviner <numbchild@gmail.com>
Date: Wed, 2 Dec 2020 17:24:29 +0800
Subject: [PATCH] org.el: Complete tags from both global and buffer local

* lisp/org.el (org-fast-tag-selection): Merge buffer local tags with
global alist of tags. And it obey the option
org-complete-tags-always-offer-all-agenda-tags.

* doc/org-manual.org: Update the TAB key doc in tags selection UI.

* etc/ORG-NEWS: Mention the change in org-set-tags-command.
---
 doc/org-manual.org |  6 +++---
 etc/ORG-NEWS       |  5 +++++
 lisp/org.el        | 24 ++++++++++++++----------
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index b015b502c..01cec4b8d 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -4860,9 +4860,9 @@ In this interface, you can also use the following special keys:
 
   #+kindex: TAB
   Enter a tag in the minibuffer, even if the tag is not in the
-  predefined list.  You can complete on all tags present in the
-  buffer.  You can also add several tags: just separate them with
-  a comma.
+  predefined list.  You can complete on all tags present in the buffer
+  and globally pre-defined tags from ~org-tag{-persistent}-alist~.
+  You can also add several tags: just separate them with a comma.
 
 - {{{kbd(SPC)}}} ::
 
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 5e5f1954d..5e68d27c0 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -149,6 +149,11 @@ Example:
 A new =u= mode flag for Calc formulas in Org tables has been added to
 enable Calc units simplification mode.
 
+*** =org-set-tags-command= select tags from ~org-global-tags-completion-table~
+
+Let =org-set-tags-command= complete tags from global tags list (both
+buffer-local tags and ~org-tag{-persistent}-alist~).
+
 ** Miscellaneous
 *** =org-goto-first-child= now works before first heading
 
diff --git a/lisp/org.el b/lisp/org.el
index 5b0ae389c..ba816dfa6 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -12139,7 +12139,7 @@ (defun org-fast-tag-selection (current inherited table &optional todo-table)
 				  fulltable))))
 	 (buf (current-buffer))
 	 (expert (eq org-fast-tag-selection-single-key 'expert))
-	 (buffer-tags nil)
+	 (tab-tags nil)
 	 (fwidth (+ maxlen 3 1 3))
 	 (ncol (/ (- (window-width) 4) fwidth))
 	 (i-face 'org-done)
@@ -12274,16 +12274,20 @@ (defun org-fast-tag-selection (current inherited table &optional todo-table)
 		    (setq current nil)
 		    (when exit-after-next (setq exit-after-next 'now)))
 		   ((= c ?\t)
-		    (condition-case nil
-			(setq tg (completing-read
-				  "Tag: "
-				  (or buffer-tags
-				      (with-current-buffer buf
-					(setq buffer-tags
-					      (org-get-buffer-tags))))))
-		      (quit (setq tg "")))
+                    (unless tab-tags
+                      (setq tab-tags
+                            (delq nil
+                                  (mapcar (lambda (x)
+                                            (let ((item (car-safe x)))
+                                              (and (stringp item)
+                                                   (list item))))
+                                          (org--tag-add-to-alist
+                                           (with-current-buffer buf
+                                             (org-get-buffer-tags))
+                                           table)))))
+                    (setq tg (completing-read "Tag: " tab-tags))
 		    (when (string-match "\\S-" tg)
-		      (cl-pushnew (list tg) buffer-tags :test #'equal)
+		      (cl-pushnew (list tg) tab-tags :test #'equal)
 		      (if (member tg current)
 			  (setq current (delete tg current))
 			(push tg current)))
-- 
2.29.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]