emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Kyle Meyer <kyle@kyleam.com>
To: stardiviner <numbchild@gmail.com>
Cc: Org-mode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] I updated patch by deleteing duplicate tags
Date: Sun, 10 Jan 2021 22:10:49 GMT	[thread overview]
Message-ID: <87sg78phnc.fsf@kyleam.com> (raw)
In-Reply-To: <CAL1eYuKfdKeRxU1GfPb90-xpBymzO9Kp1C0riUjwFdE4yVviBw@mail.gmail.com>

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.

>> 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.

> 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/.

> ---
>  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).

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)

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)))


  parent reply	other threads:[~2021-01-10 22:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-02  9:30 [PATCH] [C-c C-q] completing tags from both buffer-local and global alist of tags stardiviner
2020-12-03  2:40 ` [PATCH] I updated patch by deleteing duplicate tags stardiviner
2021-01-07  2:37   ` Christopher Miles
2021-01-10 22:10   ` Kyle Meyer [this message]
2021-01-11  2:24     ` Christopher Miles
2021-01-13  3:26       ` Kyle Meyer
2021-01-13  9:30         ` Christopher Miles
2021-01-14  5:24           ` Kyle Meyer
2021-01-14  6:12             ` [APPLIED] " Christopher Miles
2021-04-25  3:25             ` Timothy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87sg78phnc.fsf@kyleam.com \
    --to=kyle@kyleam.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=numbchild@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).