From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: [WIP PATCH] Adding keys to keymaps in alphabetical order (for use with `mode-line-mode-menu') Date: Wed, 23 Jun 2021 09:58:19 -0400 Message-ID: References: <87tulu11f1.fsf@gnus.org> <87bl7ztn4p.fsf@gnus.org> <44969496-4f6c-e299-9fee-7bad01c17815@gmail.com> <87a6ngn31s.fsf@gnus.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="33653"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: Jim Porter , emacs-devel@gnu.org To: Lars Ingebrigtsen Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Jun 23 15:59:01 2021 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lw3PB-0008aD-L3 for ged-emacs-devel@m.gmane-mx.org; Wed, 23 Jun 2021 15:59:01 +0200 Original-Received: from localhost ([::1]:36542 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lw3PA-0001rx-Jj for ged-emacs-devel@m.gmane-mx.org; Wed, 23 Jun 2021 09:59:00 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:41094) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lw3Od-0001Bh-4u for emacs-devel@gnu.org; Wed, 23 Jun 2021 09:58:27 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:60250) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lw3OZ-00027z-Uj for emacs-devel@gnu.org; Wed, 23 Jun 2021 09:58:25 -0400 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id B6B5844097F; Wed, 23 Jun 2021 09:58:21 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 3F09E4406B8; Wed, 23 Jun 2021 09:58:20 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1624456700; bh=Owp7lEnReZrIqdzZes9lD0HPPsUKySNqFFWopdLOtTc=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=EZYPuYHE7N6DQZeSRbUkrGfiKpgGV1Lrwy0tF5AmoQscE4yh8PRjoa2Fx/gQT5D+r R7fhsGKgjKW2H/Iklng1QleYMixf3xkP+PxZPBYPtZD3ZygCKJjDaMeP8cu9PMOOMO yk3yvCQNxz4TwfC2+jFuYsRb+DKRamoWrEsKrneSmBm7a0pHDrzLQZuJLplsG4WLWy UwM0qosf5nX8J+jyEMKCUXFKgM63oIU4Gd1nvpTk4x845BWKGVprP41ItkuwpaYtTm S7lHCSDQGMwurejpSKB+4nSiOjsnX7EeOwFj+HK3QbCCKEo+A22uEfSGDtDDl5Rl4N wl8LT+69JuN9w== Original-Received: from alfajor (69-196-163-239.dsl.teksavvy.com [69.196.163.239]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 0AE6B120377; Wed, 23 Jun 2021 09:58:19 -0400 (EDT) In-Reply-To: <87a6ngn31s.fsf@gnus.org> (Lars Ingebrigtsen's message of "Wed, 23 Jun 2021 15:06:23 +0200") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca X-Spam_score_int: -42 X-Spam_score: -4.3 X-Spam_bar: ---- X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:270988 Archived-At: >> Performance-wise, the cost is negligible for the default value of >> `mode-line-mode-menu': ~10ms for the first time and 0.02ms for >> subsequent calls. > Thanks; looks good, so I've applied it to Emacs 28 (with some trivial > changes). It might want to clarify that the sorting is based on the menu item names (rather than, say, the name of the key sequence). Maybe it should have "menu" somewhere in its name ;-) >> + (sort (cdr keymap) >> + (lambda (a b) >> + (string< (bindings--menu-item-string (cdr-safe a)) >> + (bindings--menu-item-string (cdr-safe b))))))) >> + (setcdr keymap menu-items) > > I'm slightly worried about this, though -- will altering the keymap > structure have any adverse side effects? I don't think so, and testing > a bit doesn't seem to reveal anything obvious, but we should be on the > lookup. Altering the keymap by side-effect is not dangerous in and of itself (`define-key` does it as well). But the above code will mess things up when applied to a keymap that has a parent keymap. It will also fail to do its job on menu keymaps which use a vector rather or on composed keymaps (which can be created "implicitly", e.g. when looking up `menu-bar` in a keymap which has a `menu-bar` binding and which additionally inherits from a keymap that also has a `menu-bar` binding). To solve those issues I think one would have to `map-keymap` and return a brand new keymap rather than modify it in-situ, AFAICT. Note that in my example above of a composed keymap created implicitly by `access_keymap`, the menu.c code currently deals very poorly with such situations (the two composed keymaps get just "concatenated"; worse, even though entries from the first keymap can hide/override elements from the second, menu.c will show them all, so you can end up with menu entries which can't actually be used). The merging of the composed keymaps would best be done according to directions embedded directly in the keymaps themselves so a child keymap can insert new menu entries in the middle of a parent menu. Stefan