[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: |
Mon, 11 Jan 2021 02:24:49 +0000 |
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
0001-org.el-Complete-tags-from-both-global-and-buffer-loc.patch
Description: new patch
signature.asc
Description: PGP signature