From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Jim Porter 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 09:25:14 -0800 Message-ID: <8c440269-262a-ba78-6630-d3f90c2fcc19@gmail.com> 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> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="31527"; mail-complaints-to="usenet@ciao.gmane.io" Cc: "52293@debbugs.gnu.org" <52293@debbugs.gnu.org> To: Juri Linkov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Dec 13 18:26:26 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 1mwp5l-0007wd-RA for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 13 Dec 2021 18:26:25 +0100 Original-Received: from localhost ([::1]:34028 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mwp5j-0002dy-Uq for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 13 Dec 2021 12:26:23 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:35586) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mwp5O-0002dj-RI for bug-gnu-emacs@gnu.org; Mon, 13 Dec 2021 12:26:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:45152) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mwp5O-0003oa-JB for bug-gnu-emacs@gnu.org; Mon, 13 Dec 2021 12:26:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mwp5O-00089P-6L for bug-gnu-emacs@gnu.org; Mon, 13 Dec 2021 12:26:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Jim Porter Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 13 Dec 2021 17:26:02 +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.163941632431288 (code B ref 52293); Mon, 13 Dec 2021 17:26:02 +0000 Original-Received: (at 52293) by debbugs.gnu.org; 13 Dec 2021 17:25:24 +0000 Original-Received: from localhost ([127.0.0.1]:56698 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mwp4m-00088a-DL for submit@debbugs.gnu.org; Mon, 13 Dec 2021 12:25:24 -0500 Original-Received: from mail-pf1-f182.google.com ([209.85.210.182]:38775) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mwp4i-00088K-0u for 52293@debbugs.gnu.org; Mon, 13 Dec 2021 12:25:23 -0500 Original-Received: by mail-pf1-f182.google.com with SMTP id g18so15528374pfk.5 for <52293@debbugs.gnu.org>; Mon, 13 Dec 2021 09:25:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=subject:to:cc:references:from:message-id:date:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=B4xWjtVrzDfAOlLwMxsqEmES+a/ZSsealZVhyfeqfFk=; b=Jf29/G3tyAc6C2v2TgtBAqVtG6T6fMRCk0ktm0MmrX5ANLWmFG8/3tk1BIdPzX4ZCO BRmM+PdJwJpGROs13lbyVZ1qWvQdtWF7SxDRepDuJ9EXOpgxD8nqDnpR7YSYpY68hxBD Uvtmo7f7Rx7E/bEuqeCWn6iqZ+Sgzi8k4lDOmFD8HvJQ7jYI7BithE+nofupG7JdqOaA whgm56oy+5m+MKsa8PqAib4jjdZthq0R6NgHF1fTQ1MfyNfhd2L0sLYcQsty7ZmBV6Hy nhgaTJZsSloQwkZYBLtTZSwDcRs7Yhs3aexzC5ZtprH2IqkZvzRstO+y2T3osnd8rzBp aMyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=B4xWjtVrzDfAOlLwMxsqEmES+a/ZSsealZVhyfeqfFk=; b=z6aonjDj+PxgFvdiX9i71Cp4XoCdZSgyMYLfZeWEBfHwYYYvPYa+w59dHmRMUT5PWx YuYfSLLnCw2Uj+pKEVMwRD2N1IBNfP3ta8OlhxOaRhz1qnOCAvlubpqTBJ3Iz/Sljyu8 /mmjztVJoOeGHYUzp9PHNa67ZLF4nDNxO6LeVyx1RzRIUN4r4N9MyBvgVGeh4aqd0GxI dmm3PkE6EtOB6Rn1IKcB25I0CDe4CykYGQA2rSe2ap+znVm6ROCoxfUK8lYkrBeJW0Ye v99R0I8mviQOf8X803DFYhPmFKu8id9xUSNv3t91SPzCnO6rLkfgufCfYUaKgmEW/6/k Rrpg== X-Gm-Message-State: AOAM532FWjyJT5+lTUbxbyGdVBxZGz0ptxOcUfFNgeizDUkqkH6Ctifa n8PcBu7dc7sB+5nvx/HY/hbdxNDF948= X-Google-Smtp-Source: ABdhPJz4KgCcO2FY8f0OEiXFKYdPq3KOmQbLUjBxOzEHcVKZA5NCdPitwAesfpqlTpLY9Da+R8q+hg== X-Received: by 2002:a63:e04f:: with SMTP id n15mr53284015pgj.31.1639416314042; Mon, 13 Dec 2021 09:25:14 -0800 (PST) Original-Received: from [192.168.1.2] (cpe-76-168-148-233.socal.res.rr.com. [76.168.148.233]) by smtp.googlemail.com with ESMTPSA id b16sm11077898pgi.36.2021.12.13.09.25.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 13 Dec 2021 09:25:13 -0800 (PST) In-Reply-To: <86czm0eue5.fsf@mail.linkov.net> Content-Language: en-US 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:222332 Archived-At: On 12/13/2021 12:47 AM, Juri Linkov wrote: > 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. > 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. 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. 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'. 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.