all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Stefan Kangas <stefan@marxist.se>
To: emacs-devel@gnu.org
Cc: "Clément Pit-Claudel" <cpitclaudel@gmail.com>,
	"Stefan Monnier" <monnier@iro.umontreal.ca>
Subject: scratch/substitute-command-keys: C conversion of s-c-k
Date: Thu, 20 Aug 2020 17:27:19 -0700	[thread overview]
Message-ID: <CADwFkmm+J0zy8VOBnjbVRsrZMmLAF4cj1paAq71cuAeCNFomNQ@mail.gmail.com> (raw)
In-Reply-To: <CADwFkmmhezor_jCwff8YH2wVvnyb7Rf=7YnLeeN_aRehkw8beA@mail.gmail.com>

Stefan Kangas <stefan@marxist.se> writes:

> Please find attached the first in a planned series of patches to replace
> `substitute-command-keys' with a Lisp version.
>
> - The goal is to produce byte-for-byte identical output to the old C
>   version.
>
> - This is a more or less direct translation of the C code, and so is not
>   very elegant Lisp.  It turned out to be easier to just "lift" the code
>   from C before working on any improvements.  (The code is not super
>   involved overall, but some details are a bit finicky.)

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Please find attached the first in a planned series of patches to replace
>> `substitute-command-keys' with a Lisp version.
>
> The strategy sounds good (I followed basically the same principles in
> various other rewrites from C to Elisp ;-).
>
> I don't know what Eli wants to do, but I think the better option is to
> push this to a branch where you can have this change followed by a few
> others, and when the patches's sum is clean we can merge it to `master`.

[The above discussion apparently never included emacs-devel.]

I now have a working translation of 'substitute-command-keys' from C to
Lisp on the scratch/substitute-command-keys branch.  For more background
see Bug#8951 and other bugs regarding s-c-k.

I have added some defsubrs that could be removed by writing new Lisp
equivalents.  It would be good if I could get advice on which is better
here:

   1) Just leave the defsubrs as is.
   2) Replace them with defun's in help.el and then either:
       a) Accept duplicate code on the C and Lisp level.
       b) Call the Lisp functions from C code, and remove the C
          functions altogether.

I also made some measurements to confirm that we get insubstantial
performance benefits by keepin 'substitute-command-keys' in C:

    (progn
      (with-temp-buffer (org-mode))
      (let* ((times 1000)
             (new (car (benchmark-run times
                         (substitute-command-keys "\\{org-mode-map}"))))
             (old (car (benchmark-run times
                         (substitute-command-keys-old "\\{org-mode-map}")))))
        (/ (- new old) times)))

    => 0.0010306464810000003

Looking around keymap.c got me thinking that maybe there is more stuff
that could be usefully lifted to Lisp there.  Obviously this would only
apply to functions that are called infrequently.  (Perhaps there is even
a case to be made for a new keymap.el where we could put this logically.)

Or maybe this is exactly the point where I stop, remove the C
implementation of 'substitute-command-keys', do some last cleanups, and
get all this merged.

Comments on all this are more than welcome.  Thanks in advance!

Best regards,
Stefan Kangas



  parent reply	other threads:[~2020-08-21  0:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-01 14:34 Ideas to improve the output of C-h m? Clément Pit-Claudel
2020-05-01 15:55 ` Stefan Monnier
2020-05-01 16:16   ` Clément Pit-Claudel
2020-05-01 17:44     ` Stefan Monnier
2020-05-01 23:02     ` Stefan Kangas
2020-05-02  3:24       ` Clément Pit-Claudel
2020-05-06 13:34         ` Stefan Kangas
     [not found]         ` <jwv5zd9wyj6.fsf-monnier+emacs@gnu.org>
     [not found]           ` <CADwFkmmhezor_jCwff8YH2wVvnyb7Rf=7YnLeeN_aRehkw8beA@mail.gmail.com>
2020-08-21  0:27             ` Stefan Kangas [this message]
2020-05-03  3:39       ` Richard Stallman
2020-05-02  4:05 ` Jean-Christophe Helary

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=CADwFkmm+J0zy8VOBnjbVRsrZMmLAF4cj1paAq71cuAeCNFomNQ@mail.gmail.com \
    --to=stefan@marxist.se \
    --cc=cpitclaudel@gmail.com \
    --cc=emacs-devel@gnu.org \
    --cc=monnier@iro.umontreal.ca \
    /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.