[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#60740: [PATCH 0/2] emoji changes
From: |
Jonas Bernoulli |
Subject: |
bug#60740: [PATCH 0/2] emoji changes |
Date: |
Thu, 2 Feb 2023 00:09:16 +0100 |
* The first commit changes `isearch-emoji-by-name' to not use
transient. If there are derivations then `completing-read'
is instead used to let the user chose one.
I think we should stick to that. What I mention next, seems
way to risky at this time and I also do not think it would be
worth it. Selecting a derivation using `completing-read' isn't
bad at all.
(The problem with using a transient command inside
`with-isearch-suspended' is that expects to be able to call a
function, which uses `recursive-edit' in some way. Once that
returns, the macro resumes isearch. But transient does not do that
and it returns immediately after the transient prefix command
returns. That happens before the user selects a derivation by
invoking one of the suffix commands. So isearch resumes before
the user has made any choice.
One way around that might be to call `recursive-edit' in
`isearch-emoji-by-name' before calling the code that might then use
transient. But that would also have to automatically invoke a
command inside that recursive edit. I couldn't find a way to do
that. I timer might work, but that seems hacky.
Another approach could be to break up `with-isearch-suspended' up
into two functions, one that suspends and another that resumes.
The suspend function could maybe also responsible for returning
the resume function as a closure. The macro could use these two
functions and existing callers could continue to use that. But
`isearch-emoji-by-name' would only call the suspend function and
would have to somehow pass the resume function to transient, so
that it could call it when the time has come.)
* The second commit changes emoji.el to use transient.el the way I
intended it to be used. (I *think* transient already had support
for "dynamic" transient commands, when emoji.el was created, but
some of the convenience features certainly were missing still.)
The new implementation changes the remaining entry points to be
defined using `define-transient-prefix' but still without adding any
suffix commands to them up front. Previously no *prefix* command
was defined up front. Instead code very similar to what can be
found in `define-transient-prefix' was used to turn certain nodes
inside the tree of emoji into (sub-)prefix commands. These prefix
commands were then invoked using `funcall', which is not how that
is supposed to work; I am surprised that worked at all, but I guess
that just means that emoji doesn't use any of transient's features
that depend on `this-command' "being correct".
The new implementation adds a single, named suffix command,
`emoji-insert-glyph'. Now every suffix of a transient prefix is
either that command or a "recursive" call to the prefix itself, with
its scope narrowed to only part of the original tree of emoji. That
is a big improvement over the old implementation, which had to
define a new command for each and every emoji, and a new command for
every grouping of emoji.
Lars, previously a hash-table was used to track the emoji, that should
not be derived any further. I changed that to a list, because I am
yet to encounter a situation where more than one element has to be
tracked. Are there even any derivations of derivations? If so,
please point me to an example.
If there are any, then the first commit would have to be adjusted to
account for that. Otherwise, I think that is ready. I would still
like to test the second commit a bit more, before applying it.
Cheers,
Jonas
- bug#60740: [PATCH 0/2] emoji changes,
Jonas Bernoulli <=
- bug#60740: [PATCH 2/2] Rewrite emoji's usage of transient, Jonas Bernoulli, 2023/02/01
- bug#60740: [PATCH 1/2] No longer use transient in isearch-emoji-by-name, Jonas Bernoulli, 2023/02/01
- bug#60740: [PATCH 0/2] emoji changes, Eli Zaretskii, 2023/02/04
- bug#60740: [PATCH 0/2] emoji changes, Jonas Bernoulli, 2023/02/04
- bug#60740: [PATCH 0/2] emoji changes, Eli Zaretskii, 2023/02/04
- bug#60740: [PATCH 0/2] emoji changes, Jonas Bernoulli, 2023/02/05
- bug#60740: [PATCH 0/2] emoji changes, Eli Zaretskii, 2023/02/05
- bug#60740: [PATCH 0/2] emoji changes, Eli Zaretskii, 2023/02/05
- bug#60740: [PATCH 0/2] emoji changes, Jonas Bernoulli, 2023/02/05
- bug#60740: [PATCH 0/2] emoji changes, Eli Zaretskii, 2023/02/05