bug#52293: [External] : bug#52293: 29.0.50; [PATCH v3] Prevent further cases of duplicated separators in context menus

[Jim Porter <jporterbugs@gmail.com>]

On 12/12/2021 5:16 PM, Drew Adams wrote:
>>> 3. I think there's zero need for any naming
>>>      convention for menu-item names, whether
>>>      separators or other.
>>
>> The goal was to make it easier to remember the names of the separators
>> if you wanted to add a new item after a particular separator that had
>> already been added. It's easier to remember if they're all
>> `foo-separator' instead of a mix of styles.
> 
> Why does anyone need to _remember_ such names?
> What's the use case for remembering?

A third-party package author may want to insert context menu items in a 
particular spot in the menu (i.e. use `define-key-after'). Because the 
naming convention of separators is inconsistent, the author can more 
easily slip up and use the wrong name.

As you might be able to tell, this is a very small issue. But the fix 
was easy, so I posted a patch for it. I really don't have a problem with 
closing that bug as wontfix if it's controversial.

> Deduplication?  We're not talking about removing
> exact duplicates are we?  I thought this was about
> removing consecutive separators, leaving only one.
> This involves no duplicate menu items:
> 
>    (define-key menu [separator-1] '("--"))
>    (define-key menu [separator-2] '("--"))

That's correct. The above menu items would be de-duplicated since 
they're both separators. However, that logic only applies to "simple" 
separators, so the de-duplication code doesn't apply to this:

   (define-key menu [separator-1] '(menu-item "--"))
   (define-key menu [separator-2] '(menu-item "--"))

(It's possible that's just an accidental omission in the original code 
though.)

> More importantly, I didn't know this was about
> removing any ordinary `define-key' bindings.
> I really hope it's not.  I thought this was only
> for Juri's new context menus.

This is *only* for context menus. The de-duplication code is in 
`context-menu-map', which is a function that gets called when opening 
the context menu. It calls all the elements of `context-menu-functions' 
in order, and then does the de-duplication step. No other menus will be 
affected by this.

In any case, I think I prefer the solution I proposed in my followup: 
instead of removing consecutive separators so things look right, we can 
*add* separators between different "sections" of the context menu. 
Sections can be marked by the `:section' keyword and a name for the 
section (note: this only applies to the context menu, not other menus). 
That should completely prevent any unwanted manipulation of menus, since 
the programmer needs to opt into this behavior explicitly.

In my limited tests, that method is also less brittle than the current 
method (this bug is an example of the brittleness). See my other 
message[1] for more details.

[1] https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-12/msg01079.html