unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Jonas Bernoulli <jonas@bernoul.li>
To: 60740@debbugs.gnu.org
Cc: larsi@gnus.org
Subject: bug#60740: [PATCH 0/2] emoji changes
Date: Thu,  2 Feb 2023 00:09:16 +0100	[thread overview]
Message-ID: <20230201230918.550-1-jonas@bernoul.li> (raw)
In-Reply-To: <83leli3nly.fsf@gnu.org>

* 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	other threads:[~2023-02-01 23:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <83leli3nly.fsf@gnu.org>
2023-02-01 23:09 ` Jonas Bernoulli [this message]
2023-02-01 23:09   ` bug#60740: [PATCH 1/2] No longer use transient in isearch-emoji-by-name Jonas Bernoulli
2023-02-01 23:09   ` bug#60740: [PATCH 2/2] Rewrite emoji's usage of transient Jonas Bernoulli
2023-02-04  8:30   ` bug#60740: [PATCH 0/2] emoji changes Eli Zaretskii
2023-02-04 12:05     ` Jonas Bernoulli
2023-02-04 12:29       ` Eli Zaretskii
2023-02-05 14:40         ` Jonas Bernoulli
2023-02-05 14:53           ` Eli Zaretskii
2023-02-05 15:02           ` Eli Zaretskii
2023-02-05 16:29             ` Jonas Bernoulli
2023-02-05 16:53               ` Eli Zaretskii

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.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230201230918.550-1-jonas@bernoul.li \
    --to=jonas@bernoul.li \
    --cc=60740@debbugs.gnu.org \
    --cc=larsi@gnus.org \
    /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.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).