emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@posteo.net>
To: "Christopher M. Miles" <numbchild@gmail.com>
Cc: Org-mode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH v5] Re: Improve the performance of `org-set-tags-command` on large `org-tag-alist`
Date: Sat, 01 Jul 2023 11:34:31 +0000	[thread overview]
Message-ID: <87pm5boofs.fsf@localhost> (raw)
In-Reply-To: <64a000ea.170a0220.250a9.7355@mx.google.com>

"Christopher M. Miles" <numbchild@gmail.com> writes:

> Ok, I update and re-generated the patch, also add new document and Org NEWS entries.

Thanks!
See my comments below.

> * lisp/org.el (org-fast-tag-selection): Filter out only maximum number
> limited tags has fast selection bound when `org-use-fast-tag-selection'
> is 'auto and tags has fast select bound keys.

Why only for 'auto?

> * doc/org-manual.org (org-fast-tag-selection-maximum-tags): Add new
> custom option document.

*documentation.

> --- a/doc/org-manual.org
> +++ b/doc/org-manual.org
> @@ -5073,6 +5073,16 @@ effect: start selection with {{{kbd(C-c C-c C-c)}}} instead of
>  the special window is not even shown for single-key tag selection, it
>  comes up only when you press an extra {{{kbd(C-c)}}}.
>  
> +#+vindex: org-fast-tag-selection-maximum-tags
> +Limit the tags table when you have many tags. You can set the maximum

#+vindex entry will not be visible in the rendered manual. So, the first
 sentence of the paragraph reads awkwardly.

Also, please use double space between sentences. It is the convention
for Org manual, docstrings, and commit messages. See
doc/Documentation_Standards.org

> +(defcustom org-fast-tag-selection-maximum-tags 26
> +  "Set the maximum tags number for fast tag selection."
> +  :group 'org-tags
> +  :type 'number
> +  :safe #'numberp)

Why 26?
We use a-zA-Z{|}~

;; Characters available for auto-assignment.
         (tag-binding-char-list
          (eval-when-compile
            (string-to-list "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ{|}~")))

Also, please add :package-version keyword in the defcustom.

>    (let* (;; Combined alist of all the tags and todo keywords.
> -         (tag-alist (append tag-table todo-table))
> +         (tag-alist (let* ((tags-table (append tag-table todo-table))
> +                           (binding-tags (seq-filter 'cdr tags-table)))
> +                      (cl-case org-use-fast-tag-selection
> +                        (auto (if (length> binding-tags org-fast-tag-selection-maximum-tags)
> +                                  binding-tags
> +                                (seq-take tags-table org-fast-tag-selection-maximum-tags)))
> +                        (t tags-table))))

May you please instead:

1. Store the total number of bound tags, but using a bit more complex
   filter (`(:startgroup "foo")', `(:endgroup "bar")' are also valid alist
   members, but do not correspond to tag bindings). See `(pcase
   tag-binding-spec ...)'
2. Store the total number of tag in tag groups. See `ingroup'.
3. Add these two numbers and compare them with
   `org-fast-tag-selection-maximum-tags', calculating how many extra
   tags can be displayed.
4. Do not filter `tag-alist', but instead plug a code into `(while (setq
   tag-binding-spec (pop tag-alist)) ...)', counting how many tags are in
   There, in `;; Insert the tag.', only insert the tag when (1) tag has
   binding - (cdr tag-binding-spec); (2) tag is in a group - ingroup;
   (3) we still have space for extra tags, according to (3), then also
   decrease the leftover tag count.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


  reply	other threads:[~2023-07-01 11:35 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-13  4:49 Improve the performance of `org-set-tags-command` on large `org-tag-alist` stardiviner
2023-05-13  7:43 ` Ihor Radchenko
2023-05-13  9:39   ` Christopher M. Miles
2023-05-13 11:13   ` stardiviner
2023-05-13 11:26     ` Ihor Radchenko
2023-05-13 14:24       ` [PATCH] " Christopher M. Miles
2023-05-13 18:43         ` Ihor Radchenko
2023-05-14  1:54           ` Christopher M. Miles
2023-05-14  8:09             ` Ihor Radchenko
2023-05-14 14:27               ` Christopher M. Miles
2023-05-14 14:58                 ` Ihor Radchenko
2023-05-14 16:27                   ` Christopher M. Miles
2023-05-14 17:38                     ` Ihor Radchenko
2023-05-14 18:14                       ` [PATCH v2] " Christopher M. Miles
2023-05-15 10:59                         ` Ihor Radchenko
2023-05-15 12:43                           ` Christopher M. Miles
2023-05-15 13:14                             ` Ihor Radchenko
2023-05-15 14:40                           ` [PATCH v3] " Christopher M. Miles
2023-05-15 16:12                             ` [PATCH v3.1] " Christopher M. Miles
2023-05-16  9:31                               ` Ihor Radchenko
2023-05-16 12:12                                 ` [PATCH v4] " Christopher M. Miles
2023-05-16 18:53                                   ` Ihor Radchenko
2023-05-17  5:57                                     ` [PATCH v4.1] " Christopher M. Miles
2023-06-30 12:55                                       ` Ihor Radchenko
2023-07-01 10:31                                         ` [PATCH v5] " Christopher M. Miles
2023-07-01 11:34                                           ` Ihor Radchenko [this message]
2024-01-09 14:12                                             ` [PATCH v6] " Ihor Radchenko
2024-01-10  3:48                                               ` Christopher M. Miles
2024-01-12 12:00                                                 ` Ihor Radchenko
2023-05-16 12:12                                 ` [PATCH v4] " Christopher M. 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://www.orgmode.org/

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

  git send-email \
    --in-reply-to=87pm5boofs.fsf@localhost \
    --to=yantar92@posteo.net \
    --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).