From: akater <nuclearspace@gmail.com>
To: Kyle Meyer <kyle@kyleam.com>, emacs-orgmode@gnu.org
Subject: Re: [PATCH] org-agenda.el: Complete multiple todo keywords
Date: Thu, 30 Apr 2020 06:50:31 +0000 [thread overview]
Message-ID: <87lfmdxya0.fsf@gmail.com> (raw)
In-Reply-To: <87tv11liug.fsf@kyleam.com>
[-- Attachment #1: Type: text/plain, Size: 5576 bytes --]
Kyle Meyer <kyle@kyleam.com> writes:
> Thanks for the patch. Looks like a nice improvement to me.
>
> akater <nuclearspace@gmail.com> writes:
>
>> * lisp/org-agenda.el (org-todo-list): Use completing-read-multiple
>> instead of completing-read when selecting todo keywords to filter by
>> in Agenda.
>
> This and the rest of the lines were unwrapped. Could you wrap them ~70
> characters? (The Org repo's .dir-locals.el sets fill-column to 70.)
>> * lisp/org-agenda.el (org-todo-list): Fix a typo in the prompt.
>
> Thanks for spotting that typo. I think it'd be more common to append
> this description to the entry above rather than adding another
> org-todo-list entry.
Done. New patch is attached.
>> There is minor UX cost to Helm users: while candidates list used to
>> appear immediately to Helm users, now Helm users have to hit TAB to
>> see the list.
>
> Just the opinion of one Helm user, but needing to hit tab for crm-based
> completion has never bothered me too much. But if it did, Helm allows
> specifying that certain commands should go through the built-in
> completion.
>
> Out of curiosity I tried with the latest ivy (9e0803c), and I also
> needed to hit tab before seeing anything.
>
>> This inconsistency is not present in vanilla Emacs
>> completion.
>
> I'm confused by this. When I try with no customization (Emacs 26.3), I
> need to hit tab to see any of the candidates.
Yes; my point is, in vanilla Emacs completing-read-multiple and
completing-read behave similarly while in Helm, singleton completion
does not require hitting TAB initially but multiple-candidates
completion does. I remember initially thinking that I broke completion
altogether when I first introduced crm here. My confusion didn't last
long but still, I found this a little unpleasant and tried my best to
make it go.
> Looks like you stuck with "|" as the separator, which seems like a good
> idea to me.
>
>> However, it is unfortunate that string patterns are strings themselves
>> and are thus indistinguishable from strings; it would be better if crm
>> exposed separator (the string) on its own in its interface.
>
> I'm not quite sure I follow what you're suggesting with the last bit.
> Could you rephrase the point in a way that is a bit more connected with
> the code change? This patch sticks with the same separator, so aside
> from being able to complete multiple things, there's no change in
> behavior/added restriction here, right?
Well, this likely shouldn't be in the patch either. I removed it and
it's fine to forget about it. The rest of the message elaborates on
this.
What type is crm-separator of? According to crm.el, its default value is
that of the constant crm-default-separator which is regex, i.e. a string
pattern. That means crm-separator is a string pattern. It is employed
as a string pattern in (the function) completing-read-multiple to clean
some string from unneeded whitespace.
And yet, using string pattern modelled on the default one in my patch
would break things for Helm users (besides the fact that using it would
simply make the code unnecessary cluttered). If you substitute said
direct equivalent, activate Helm, eval
(let ((crm-separator "[ \t]*\|[ \t]*"))
(completing-read-multiple "kwds: "
(mapcar #'list '("TODO" "DONE")) nil nil))
, press TAB C-SPC C-SPC RET | TAB C-SPC C-SPC RET, you'll get corrupted
result: TODO,DONE|TODO|DONE. This is due to the fact that to make Helm
support non-standard crm-separator, Helm developer had to resort to a
fairly ugly hack trying to distinguish between a string pattern and an
actual separator, otherwise results like TODO,DONE|TODO|DONE would
appear in minibuffer.
The gist of the problem is, Helm needs a separator to insert into
buffer, and it now tries do detect whether crm-separator is regex or
not, only uses it as insertion material in the latter case and reverts
to the default comma otherwise. Detection is based on string length:
https://github.com/emacs-helm/helm/commit/52e1d74f9ec6647c039012626f96596f0eb4140a
which is of course very unreliable.
This might be considered a low-quality decision in Helm but I'd say it
is natural for a user
- of multiple-candidates completion interface (such as Helm)
- that has to transform collections of candidates into strings on its
own (this would be true for any Emacs application, I guess)
to need both the separator to search for and the separator to
intersperse a string with.
Some collection types, like this collection type “a collection of Org
todo keywords”, come bundled with specific separator-the-string. It is
this string that I'm using in mapconcat here. This string is a natural
part of the interface of many collections of candidates, due to
string-orientedness of everything that Emacs has to deal with, but this
separator-the-string is now inaccessible to users of crm because crm de
facto only exposes regex separator as part of its interface.
These two separators are different objects of different types: one of
them is a pattern, another is a string, and the former cannot be
reliably used in the role of the latter. Maybe the former (regex, or
several such) can always be computed from the latter (the string), and
if true, crm.el could provide just separator-the-string for the
interface; relevant regexes may be constructed from it when needed. But
I did not study all the use cases of crm-separator the regexp so this is
merely a guess.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Complete multiple todo keywords in org-agenda --]
[-- Type: text/x-diff, Size: 1019 bytes --]
org-agenda.el: Complete multiple todo keywords
* lisp/org-agenda.el (org-todo-list): Use completing-read-multiple
instead of completing-read when selecting todo keywords to filter by
in Agenda. Fix a typo in the prompt.
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -4790,8 +4790,12 @@
(nth (1- arg) kwds))))
(when (equal arg '(4))
(setq org-select-this-todo-keyword
- (completing-read "Keyword (or KWD1|K2D2|...): "
- (mapcar #'list kwds) nil nil)))
+ (mapconcat #'identity
+ (let ((crm-separator "|"))
+ (completing-read-multiple
+ "Keyword (or KWD1|KWD2|...): "
+ (mapcar #'list kwds) nil nil))
+ "|")))
(and (equal 0 arg) (setq org-select-this-todo-keyword nil))
(org-compile-prefix-format 'todo)
(org-set-sorting-strategy 'todo)
next prev parent reply other threads:[~2020-04-30 7:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-29 16:27 [PATCH] org-agenda.el: Complete multiple todo keywords akater
2020-04-30 4:04 ` Kyle Meyer
2020-04-30 6:50 ` akater [this message]
2020-04-30 22:19 ` Kyle Meyer
2020-05-01 20:58 ` akater
2020-05-02 3:37 ` Kyle Meyer
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=87lfmdxya0.fsf@gmail.com \
--to=nuclearspace@gmail.com \
--cc=emacs-orgmode@gnu.org \
--cc=kyle@kyleam.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).