emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Christopher Miles
Subject: Re: [PATCH] I updated patch by deleteing duplicate tags
Date: Wed, 13 Jan 2021 09:30:56 +0000

Kyle Meyer <kyle@kyleam.com> writes:

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

Applied.

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

Applied.

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

Applied.

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

Maybe our recognition is different I personally like to markup command as
verbatim, and option as code to display as two different face color. I forget to
drop my habit.... Now fixed to use "~code~".

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

Added back now.

> Thanks.


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

Attachment: 0001-org.el-Complete-tags-from-both-global-and-buffer-loc.patch
Description: latest patch

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]