[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#60740: [PATCH 0/2] emoji changes
From: |
Eli Zaretskii |
Subject: |
bug#60740: [PATCH 0/2] emoji changes |
Date: |
Sat, 04 Feb 2023 10:30:36 +0200 |
> Cc: larsi@gnus.org
> From: Jonas Bernoulli <jonas@bernoul.li>
> 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.
Thanks.
Would it make sense to install the first patch on the emacs-29 branch
and the second on master? Or did I misunderstand your explanations
above? AFAIU, the first patch solves the immediate issue in this bug,
right?
- bug#60740: [PATCH 0/2] emoji changes, Jonas Bernoulli, 2023/02/01
- 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 <=
- 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