Org-mode mailing list
 help / color / mirror / Atom feed
From: Kyle Meyer <kyle@kyleam.com>
To: Christopher Miles <numbchild@gmail.com>
Cc: Org-mode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] I updated patch by deleteing duplicate tags
Date: Wed, 13 Jan 2021 03:26:46 GMT
Message-ID: <87sg7534az.fsf@kyleam.com> (raw)
In-Reply-To: <VI1PR1001MB107016828BB82C14468190A6A3AB0@VI1PR1001MB1070.EURPRD10.PROD.OUTLOOK.COM>

Christopher Miles writes:

> Kyle Meyer <kyle@kyleam.com> writes:
>
>> stardiviner writes:
>>>> 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.

My point was that it's not just adding these pre-defined lists (#1);
it's also adding tags from agenda files (#2).  I think we certainly
don't want to do #2 unconditionally.  On the other hand, #1 is probably
okay, but as I said in my last email, I could see not wanting that
either.

Anyway, the patch I proposed (which you've adapted for your latest
round), avoids #2 by guarding the behavior behind the existing
org-complete-tags-always-offer-all-agenda-tags option, and I think
that's a good path forward.

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

s/obey/&s/

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

Please spell these two options out: ~org-tag-alist~ and
~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~).

Same note about avoiding shorthand for the option names.

Also, please rewrite this to make it clear that this is relevant for TAB
in the fast completion interface.  It's probably also worth mentioning
that org-complete-tags-always-offer-all-agenda-tags is now honored.

And, very minor, but any reason for the mix of =...= and ~...~ markup
here?

> 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

Unlike the patch I sent, you've dropped the condition-case here, which I
don't think was intentional.

Thanks.


  reply	other threads:[~2021-01-13  3:27 UTC|newest]

Thread overview: 9+ 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
2021-01-11  2:24     ` Christopher Miles
2021-01-13  3:26       ` Kyle Meyer [this message]
2021-01-13  9:30         ` Christopher Miles
2021-01-14  5:24           ` Kyle Meyer
2021-01-14  6:12             ` [APPLIED] " Christopher Miles

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://orgmode.org

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

  git send-email \
    --in-reply-to=87sg7534az.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

Org-mode mailing list

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://orgmode.org/list/0 list/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 list list/ https://orgmode.org/list \
		emacs-orgmode@gnu.org
	public-inbox-index list

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.emacs.orgmode
	nntp://news.gmane.io/gmane.emacs.orgmode


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git