all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Jonas Bernoulli via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: "Eli Zaretskii" <eliz@gnu.org>, "João Távora" <joaotavora@gmail.com>
Cc: 68568@debbugs.gnu.org
Subject: bug#68568: transient.el interns potentially enormous symbols as commands
Date: Sat, 27 Jan 2024 19:18:22 +0100	[thread overview]
Message-ID: <87mssqr66p.fsf@bernoul.li> (raw)
In-Reply-To: <86v87f87ds.fsf@gnu.org>

Eli Zaretskii <eliz@gnu.org> writes:

> Ping!
>
> Jonas, any comments or suggestions?

We discussed at https://github.com/magit/transient/pull/273.

Summary: It would probably be possible to avoid interning these symbols.
I did some experimentation to come to that provisional assessment.  But
Transient has been using interned from the beginning, and as a result,
many parts of Transient assume that interned symbols are being used.
Using symbols made this aspect of Transient much simpler.  This allowed
me to focus on gradually improve other aspects, which are more
complicated by necessity.  If I had not used symbols, this would have
slowed down development.  Switching to not using symbols for some
commands, would mean many things would have to be adjusted to handle
both cases.  This would undoubtedly lead to bugs and slow down
development.  I would rather work on the parts of Transient that IMO
need improvement.

IMO using commands that are fbound to interned symbols has one and
only one drawback: they are offered as completion candidates by
execute-extended-command.

I have addressed this by setting read-extended-command-predicate to a
function that hides "transient" commands that should be hidden and
nothing else. The option is only set if, and only if, it is not already
set to something other than nil.  If the user customizes the option and
pick one of the predicates offered by Emacs, then that predicate also
hides Transient's "anonymous" commands. [There was a bug, I just fixed
that.]

IMO the result of this approach is that the only drawback of interning
the symbols is gone; without me having to trade in that one drawback for
many avoidable complications, as I would have to if I followed the
request to not "needlessly" intern the "anonymous" symbols.

An additional benefit of using read-extended-command-predicate is that
it allows all of Transient's commands that should be hidden, to be
hidden.  Even if I did stop using interned symbols for the "anonymous"
commands, which are inline when a transient menu is defined, there would
still be other commands that are defined non-anonymously, but never the
less should be hidden.  (E.g., commands that toggle arguments and are
defined using transient-define-infix, because they are useful in more
than one menu, and are then referenced by name in the definitions of
these menus.)

Therefore, the addition to read-extended-command-predicate is necessary,
whether I avoid interning the "anonymous" commands or not.  And as a
consequence of that, there would be zero benefit to avoiding to intern
the "anonymous" commands; but many drawbacks.

I have therefore decided to stick to using interned symbols.

Even if you don't agree with me by now, there is one big drawback that
moving away from interned symbols would have, which I believe will win
you over.  Each command is associated with an object and the symbol is
used to link the two.  As a consequence all "anonymous" commands that
set an argument can be mere aliases to the same command, which only has
to live in memory once.  If the symbol can no longer be used to link the
command and object, then the anonymous commands would have to be defined
as oclosures.  These closures would be nearly identical, but each one of
them would permanently use up memory.

-- Jonas






  reply	other threads:[~2024-01-27 18:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-18 11:50 bug#68568: transient.el interns potentially enormous symbols as commands João Távora
2024-01-21  4:27 ` Psionic K
2024-01-21  7:15   ` João Távora
2024-01-21  7:29     ` Psionic K
2024-01-21  8:10       ` João Távora
2024-01-21  8:26         ` Psionic K
2024-01-21  8:33           ` João Távora
2024-01-27  9:15 ` Eli Zaretskii
2024-01-27 18:18   ` Jonas Bernoulli via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2024-02-01 10:24     ` 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

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

  git send-email \
    --in-reply-to=87mssqr66p.fsf@bernoul.li \
    --to=bug-gnu-emacs@gnu.org \
    --cc=68568@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=joaotavora@gmail.com \
    --cc=jonas@bernoul.li \
    /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 external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.