From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Juri Linkov Newsgroups: gmane.emacs.bugs Subject: bug#52293: [External] : bug#52293: 29.0.50; [PATCH v3] Prevent further cases of duplicated separators in context menus Date: Mon, 13 Dec 2021 20:58:58 +0200 Organization: LINKOV.NET Message-ID: <86fsqwmig7.fsf@mail.linkov.net> References: <86tufn5jn9.fsf@mail.linkov.net> <86pmqa14fc.fsf@mail.linkov.net> <15eebcb1-b67a-8363-bc23-cf113913856f@gmail.com> <86h7bkhlo7.fsf@mail.linkov.net> <37047ada-fa33-88dd-5237-769611fec61b@gmail.com> <83wnkfw8ud.fsf@gnu.org> <86ilvysyq3.fsf@mail.linkov.net> <834k7ix4ij.fsf@gnu.org> <86pmq6taeb.fsf@mail.linkov.net> <83czm6unfs.fsf@gnu.org> <414be477-34d4-c777-f623-3e0e59c0dd08@gmail.com> <83o85mpaqb.fsf@gnu.org> <8e9b29cd-8700-1a60-1d91-b6995d8f433a@gmail.com> <86czm0eue5.fsf@mail.linkov.net> <8c440269-262a-ba78-6630-d3f90c2fcc19@gmail.com> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="21389"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (x86_64-pc-linux-gnu) Cc: "52293@debbugs.gnu.org" <52293@debbugs.gnu.org> To: Jim Porter Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Dec 13 20:21:29 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1mwqt6-0005L2-Co for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 13 Dec 2021 20:21:28 +0100 Original-Received: from localhost ([::1]:54168 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mwqt5-0007i3-Bo for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 13 Dec 2021 14:21:27 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:35202) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mwqsh-0007gI-Vk for bug-gnu-emacs@gnu.org; Mon, 13 Dec 2021 14:21:04 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:45320) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mwqsh-0005x2-Nm for bug-gnu-emacs@gnu.org; Mon, 13 Dec 2021 14:21:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mwqsh-0002Yg-Ie for bug-gnu-emacs@gnu.org; Mon, 13 Dec 2021 14:21:03 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Juri Linkov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 13 Dec 2021 19:21:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 52293 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 52293-submit@debbugs.gnu.org id=B52293.16394232419736 (code B ref 52293); Mon, 13 Dec 2021 19:21:03 +0000 Original-Received: (at 52293) by debbugs.gnu.org; 13 Dec 2021 19:20:41 +0000 Original-Received: from localhost ([127.0.0.1]:56854 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mwqsL-0002Wx-Bz for submit@debbugs.gnu.org; Mon, 13 Dec 2021 14:20:41 -0500 Original-Received: from relay9-d.mail.gandi.net ([217.70.183.199]:55631) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mwqsH-0002WW-GX for 52293@debbugs.gnu.org; Mon, 13 Dec 2021 14:20:38 -0500 Original-Received: (Authenticated sender: juri@linkov.net) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id A33BDFF807; Mon, 13 Dec 2021 19:20:30 +0000 (UTC) In-Reply-To: <8c440269-262a-ba78-6630-d3f90c2fcc19@gmail.com> (Jim Porter's message of "Mon, 13 Dec 2021 09:25:14 -0800") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:222341 Archived-At: >> Actually, this wasn't an omission. Now that I'm thinking more about this, >> maybe separators that are subject to possible removal could be marked by e.g. >> using text properties, thus opting into this behavior explicitly: >> (defconst context-menu-separator (list (propertize "--" 'remove t))) >> (define-key menu [separator-1] context-menu-separator) >> (define-key menu [separator-2] context-menu-separator) >> Then code that de-duplicates separators could check for this property. > > If we did that, how about using an extend-format separator, since it > already supports properties? We could just add a new property for the > de-duplicator to check: > > (define-key menu [separator-1] '(menu-item "--" nil :deduplicate t)) > > Maybe there's a relatively simple way to reuse `:visible' for this? > > One benefit to this being opt-in is that if people wanted this behavior for > other menus, it would be possible to add it without breaking any existing > code. I agree that using an extend-format separator is a good idea, especially if we'll provide a special variable that will contain it. But I don't understand why opt-in. I think opt-out would be better since most of the time the users want to have de-duplicated menus. >> Adding a new keyword to every menu item requires more work from authors >> of context menus, and actually makes menus more brittle - >> when an author forgets to add the new keyword `:section' to some menu item, >> then two unexpected separators will be inserted: before and after >> such an item. > > The way I've implemented this now shouldn't have this problem: if a menu > item doesn't have a `:section', it's treated as being in the same section > as the previous item (unless there were no sections before this item; in > that case, it's treated as being in the same section as the next > item). It's only actually *required* to specify the section for the first > item in the section. So the authors will have the requirement to be careful to mark the first menu item. When the menus are added conditionally, then authors will need to mark several menu items that can become the first menu item. > One of the main benefits here is that we don't have to be as careful with > the order of menu items. For example, my previous patch[1] adds > `top-separator' so that we can ensure the context menu title is always the > first item in the keymap in order to let us find consecutive > separators. With `:section', the `top-separator' patch can be thrown out, > and programmers can use `define-key' to add menu items to the top as they > normally would. The same way the `top-separator' can be avoided when post-processing code will search the menu title and move it to the beginning before starting to remove duplicates, or to use some more complex logic that takes into account the location of the menu title in the middle of the menu. > However, using `:section' makes it harder to insert new items at the > beginning of a previously-defined section. With separators, you can just > call `define-key-after' and pass in the separator's name, but it's pretty > tricky when using `:section'. One way to handle this would be to add > `define-key-before', but then the programmer still has to remember to add > `:section'. Please also consider additional costs for authors to learn about this subset of the standard menu functionality with more features. Currently authors have no problems with constructing the standard menus with separators, but with `:section' they need to learn that this feature is not available in all menus, but only in context menus, this would be too confusing. > In the end, there's a tradeoff with each implementation. When using > separators, programmers have to be careful to use `define-key-after' (and > if we add a property to opt into de-duplication, to use the property). When > using `:section', programmers have to remember to set the section, and we > probably want to add something like `define-key-before' to make things > easier[2]. I think the implementation of `context-menu-map' is slightly > easier to follow for `:section' too, but the difference isn't major (that > said, my current implementation is just a sketch and could use some > cleanup). > > [1] https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-12/msg00709.html > [2] This may be useful in general though. It's not *strictly* necessary, > but it'd be helpful in any case where you want to insert a menu item before > another, but you don't know the name of the item already preceding it in > the menu. Maybe better first to try fixing the problem using the separators? When this doesn't help, only then we could consider alternatives.