bug-gnu-emacs
[Top][All Lists]
Advanced

[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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]