GNU bug report logs - #60740
29.0.60; Broken isearch-emoji-by-name

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jonas Bernoulli <jonas <at> bernoul.li>
Cc: 60740 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: bug#60740: [PATCH 0/2] emoji changes
Date: Sat, 04 Feb 2023 10:30:36 +0200

> Cc: larsi <at> gnus.org
> From: Jonas Bernoulli <jonas <at> 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?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.