bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Jim Porter]

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

This is spun off from bug#52237[1]. There's a minor issue with 
`context-menu-mode' where some of the separators are named 
`FOO-separator' and others are named `separator-FOO'. Since these 
separators are partially useful for context menu functions to find the 
right place to insert their items, this inconsistency could be pretty 
confusing for authors of those functions (e.g. in third-party packages).

This patch renames the separators to `FOO-separator' following the logic 
in the above-linked comment (in short, `FOO' is a noun adjunct modifying 
the actual object: a separator). To verify that I changed all the 
appropriate places and didn't miss anything, I ran the following command 
and manually looked over all the results to eliminate false positives:

   ag '(?<![\w-])separator-+[A-Za-z][\w-]*' ../lisp -G '.*\.el$'

I've tested the patch on both Emacs 28 and 29 and don't see any issues 
in either.

Hopefully this is a small enough change to make it into Emacs 28, since 
otherwise I think we'd just have to live with the names as they are 
forever. It wouldn't be very nice to change these names after third 
parties had come to rely on them in Emacs 28, after all.

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

[-- Attachment #2: 0001-Rename-context-menu-separators-to-FOO-separator-for-.patch --]
[-- Type: text/plain, Size: 4013 bytes --]

From 30ff30e766a226d3eaec647525a46906f3e1aa9f Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 4 Dec 2021 13:16:56 -0800
Subject: [PATCH] Rename context menu separators to 'FOO-separator' for
 consistency

* lisp/mouse.el (context-menu-toolbar, context-menu-global)
(context-menu-local, context-menu-minor, context-menu-buffers)
(context-menu-vc, context-menu-undo, context-menu-region): Rename
separators to 'FOO-separator'.
---
 lisp/mouse.el | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lisp/mouse.el b/lisp/mouse.el
index b5ca80a446..ca58dff47f 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -357,7 +357,7 @@ context-menu-middle-separator
 (defun context-menu-toolbar (menu _click)
   "Populate MENU with submenus from the tool bar."
   (run-hooks 'activate-menubar-hook 'menu-bar-update-hook)
-  (define-key-after menu [separator-toolbar] menu-bar-separator)
+  (define-key-after menu [toolbar-separator] menu-bar-separator)
   (map-keymap (lambda (key binding)
                 (when (consp binding)
                   (define-key-after menu (vector key)
@@ -368,7 +368,7 @@ context-menu-toolbar
 (defun context-menu-global (menu _click)
   "Populate MENU with submenus from the global menu."
   (run-hooks 'activate-menubar-hook 'menu-bar-update-hook)
-  (define-key-after menu [separator-global] menu-bar-separator)
+  (define-key-after menu [global-separator] menu-bar-separator)
   (map-keymap (lambda (key binding)
                 (when (consp binding)
                   (define-key-after menu (vector key)
@@ -379,7 +379,7 @@ context-menu-global
 (defun context-menu-local (menu _click)
   "Populate MENU with submenus provided by major mode."
   (run-hooks 'activate-menubar-hook 'menu-bar-update-hook)
-  (define-key-after menu [separator-local] menu-bar-separator)
+  (define-key-after menu [local-separator] menu-bar-separator)
   (let ((keymap (local-key-binding [menu-bar])))
     (when keymap
       (map-keymap (lambda (key binding)
@@ -392,7 +392,7 @@ context-menu-local
 (defun context-menu-minor (menu _click)
   "Populate MENU with submenus provided by minor modes."
   (run-hooks 'activate-menubar-hook 'menu-bar-update-hook)
-  (define-key-after menu [separator-minor] menu-bar-separator)
+  (define-key-after menu [minor-separator] menu-bar-separator)
   (dolist (mode (reverse (minor-mode-key-binding [menu-bar])))
     (when (and (consp mode) (symbol-value (car mode)))
       (map-keymap (lambda (key binding)
@@ -405,7 +405,7 @@ context-menu-minor
 (defun context-menu-buffers (menu _click)
   "Populate MENU with the buffer submenus to buffer switching."
   (run-hooks 'activate-menubar-hook 'menu-bar-update-hook)
-  (define-key-after menu [separator-buffers] menu-bar-separator)
+  (define-key-after menu [buffers-separator] menu-bar-separator)
   (map-keymap (lambda (key binding)
                 (when (consp binding)
                   (define-key-after menu (vector key)
@@ -415,13 +415,13 @@ context-menu-buffers
 
 (defun context-menu-vc (menu _click)
   "Populate MENU with Version Control commands."
-  (define-key-after menu [separator-vc] menu-bar-separator)
+  (define-key-after menu [vc-separator] menu-bar-separator)
   (define-key-after menu [vc-menu] vc-menu-entry)
   menu)
 
 (defun context-menu-undo (menu _click)
   "Populate MENU with undo commands."
-  (define-key-after menu [separator-undo] menu-bar-separator)
+  (define-key-after menu [undo-separator] menu-bar-separator)
   (when (and (not buffer-read-only)
              (not (eq t buffer-undo-list))
              (if (eq last-command 'undo)
@@ -439,7 +439,7 @@ context-menu-undo
 
 (defun context-menu-region (menu click)
   "Populate MENU with region commands."
-  (define-key-after menu [separator-region] menu-bar-separator)
+  (define-key-after menu [region-separator] menu-bar-separator)
   (when (and mark-active (not buffer-read-only))
     (define-key-after menu [cut]
       '(menu-item "Cut" kill-region
-- 
2.25.1

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Eli Zaretskii]

> From: Jim Porter <jporterbugs@gmail.com>
> Date: Sat, 4 Dec 2021 13:57:31 -0800
> 
> This is spun off from bug#52237[1]. There's a minor issue with 
> `context-menu-mode' where some of the separators are named 
> `FOO-separator' and others are named `separator-FOO'. Since these 
> separators are partially useful for context menu functions to find the 
> right place to insert their items, this inconsistency could be pretty 
> confusing for authors of those functions (e.g. in third-party packages).

If you want to introduce a convention that others should follow, this
convention should be documented in the ELisp manual.

> Hopefully this is a small enough change to make it into Emacs 28, since 
> otherwise I think we'd just have to live with the names as they are 
> forever.

The first pretest of Emacs 28 is already out, so this problem is
already with us, isn't it?

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Jim Porter]

On 12/4/2021 10:45 PM, Eli Zaretskii wrote:
>> From: Jim Porter <jporterbugs@gmail.com>
>> Date: Sat, 4 Dec 2021 13:57:31 -0800
>>
>> This is spun off from bug#52237[1]. There's a minor issue with
>> `context-menu-mode' where some of the separators are named
>> `FOO-separator' and others are named `separator-FOO'.
[snip]
> 
> If you want to introduce a convention that others should follow, this
> convention should be documented in the ELisp manual.
> 
>> Hopefully this is a small enough change to make it into Emacs 28, since
>> otherwise I think we'd just have to live with the names as they are
>> forever.
> 
> The first pretest of Emacs 28 is already out, so this problem is
> already with us, isn't it?

If it's too late to change this, that's ok. I just noticed it while 
working on the patch for bug#52237 and figured it'd be good to file a 
bug about it while there's still a non-zero chance it could be changed. 
It's a pretty minor issue, so I don't really mind if this gets closed as 
wontfix.

If this change *is* ok to merge, I'll happily update the ELisp manual 
with a brief summary of the convention before it gets merged.

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Juri Linkov]

>> Hopefully this is a small enough change to make it into Emacs 28, since
>> otherwise I think we'd just have to live with the names as they are
>> forever.
>
> The first pretest of Emacs 28 is already out, so this problem is
> already with us, isn't it?

The problem will appear after the packages will start to build
own context menus after the release.

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Eli Zaretskii]

> Cc: 52286@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Sat, 4 Dec 2021 23:44:07 -0800
> 
> > The first pretest of Emacs 28 is already out, so this problem is
> > already with us, isn't it?
> 
> If it's too late to change this, that's ok. I just noticed it while 
> working on the patch for bug#52237 and figured it'd be good to file a 
> bug about it while there's still a non-zero chance it could be changed. 
> It's a pretty minor issue, so I don't really mind if this gets closed as 
> wontfix.
> 
> If this change *is* ok to merge, I'll happily update the ELisp manual 
> with a brief summary of the convention before it gets merged.

I'd like to hear opinions of others.  Meanwhile, having the proposed
convention spelled out in the manual would be good to make sure we are
all on the same page regarding the issue.

As for the issue itself: cannot we detect the separators in some more
robust way, one that doesn't depend on how they are named?  And if
that's not feasible, perhaps allow anything that has "separator" in
its name to be considered a separator?

Thanks.

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Juri Linkov]

> This is spun off from bug#52237[1]. There's a minor issue with
> `context-menu-mode' where some of the separators are named `FOO-separator'
> and others are named `separator-FOO'. Since these separators are partially
> useful for context menu functions to find the right place to insert their
> items, this inconsistency could be pretty confusing for authors of those
> functions (e.g. in third-party packages).

While I agree that it would be nice to release Emacs 28 with consistent
menu separator names, the following list explains the current inconsistency
where part of separator names were based on one naming convention,
and part on another:

  lisp/cmuscheme.el
    (define-key map [separator-eval] '("--"))
  lisp/help-mode.el
    (define-key-after map [separator-1] menu-bar-separator)
  lisp/info.el
    (define-key-after map [separator-1] menu-bar-separator)
    (define-key-after map [separator-2] menu-bar-separator)
    (define-key-after map [separator-3] menu-bar-separator)
  lisp/international/mule-cmds.el
    (bindings--define-key map [separator-3] menu-bar-separator)
    (bindings--define-key map [separator-2] menu-bar-separator)
    (bindings--define-key map [separator-1] menu-bar-separator)
    (bindings--define-key map [separator-coding-system] menu-bar-separator)
    (bindings--define-key map [separator-input-method] menu-bar-separator)
    (bindings--define-key map [separator-mule] menu-bar-separator)
  lisp/menu-bar.el
    (bindings--define-key menu [separator-exit]
    (bindings--define-key menu [separator-print]
    (bindings--define-key menu [separator-tab]
    (bindings--define-key menu [separator-frame]
    (bindings--define-key menu [separator-window]
    (bindings--define-key menu [separator-save]
    (bindings--define-key menu [separator-tag-isearch]
    (bindings--define-key menu [separator-tag-search] menu-bar-separator)
    (bindings--define-key menu [separator-repeat-search]
    (bindings--define-key menu [separator-replace-tags]
    (bindings--define-key menu [separator-tag-file]
    (bindings--define-key menu [separator-xref]
    (bindings--define-key menu [separator-bookmark]
    (bindings--define-key menu [separator-search]
    (bindings--define-key menu [separator-undo] menu-bar-separator))
    (bindings--define-key menu [separator-1]
    (bindings--define-key menu [separator-2]
    (bindings--define-key menu [separator-3]
    (bindings--define-key menu [separator-keys]
    (bindings--define-key menu [separator-file]
    (bindings--define-key menu [separator-games]
    (bindings--define-key menu [separator-encryption-decryption]
    (bindings--define-key menu [separator-net]
    (bindings--define-key menu [separator-vc]
    (bindings--define-key menu [separator-compare]
    (bindings--define-key menu [separator-spell]
    (bindings--define-key menu [separator-prog]
    (bindings--define-key menu [separator-desc-mule]
  lisp/net/eudc.el
    (define-key map [separator-eudc-email] menu-bar-separator)
    (define-key map [separator-eudc-query] menu-bar-separator)
  lisp/progmodes/compile.el
    (define-key map [separator-2] nil)
    (define-key-after map [separator-compile] menu-bar-separator)
  lisp/tool-bar.el
    (define-key-after (default-value 'tool-bar-map) [separator-1] menu-bar-separator)
    (define-key-after (default-value 'tool-bar-map) [separator-2] menu-bar-separator)
    (define-key-after (default-value 'tool-bar-map) [separator-3] menu-bar-separator)
  lisp/vc/ediff-hook.el
    (define-key menu-bar-ediff-menu [separator-ediff-misc] menu-bar-separator)
    (define-key menu-bar-ediff-menu [separator-ediff-windows] menu-bar-separator)
    (define-key menu-bar-ediff-menu [separator-ediff-regions] menu-bar-separator)
    (define-key menu-bar-ediff-menu [separator-ediff-directories] menu-bar-separator)
    (define-key menu-bar-ediff-menu [separator-ediff-files] menu-bar-separator)
    (define-key menu-bar-ediff-merge-menu [separator-ediff-merge] menu-bar-separator)
    menu-bar-ediff-merge-menu [separator-ediff-merge-dirs] menu-bar-separator)
  lisp/vc/vc-dir.el
    (define-key-after map [separator-1] menu-bar-separator)
    (define-key-after map [separator-2] menu-bar-separator)
    (define-key-after map [separator-3] menu-bar-separator)

And another naming convention:

  lisp/cedet/cedet.el
    (define-key map [semantic-options-separator] #'undefined)
    (define-key map [cedet-menu-separator] #'undefined)
  lisp/cedet/ede.el
    (define-key cedet-menu-map [cedet-menu-separator] '("--")))
    (define-key cedet-menu-map [cedet-menu-separator] nil)
  lisp/cedet/semantic.el
    (define-key edit-menu [semantic-completion-separator]
    (define-key edit-menu [semantic-edit-separator]
    (define-key navigate-menu [semantic-narrow-to-defun-separator]
    (define-key navigate-menu [semantic-navigation-separator]
    (define-key cedet-menu-map [semantic-options-separator]
    (define-key cedet-menu-map [cedet-menu-separator] '("--")))
    (define-key cedet-menu-map [cedet-menu-separator] nil)
    (define-key cedet-menu-map [semantic-options-separator] nil)
  lisp/international/iso-cvt.el
    (define-key menu [load-as-separator] '("--"))
    (define-key menu [translate-separator] '("--"))
  lisp/menu-bar.el
    (bindings--define-key menu [scrollbar-separator]
    (bindings--define-key menu [linecolumn-separator]
    (bindings--define-key menu [datetime-separator]
    (bindings--define-key menu [custom-separator]
    (bindings--define-key menu [custom-separator]
    (bindings--define-key menu [showhide-separator]
    (bindings--define-key menu [mule-separator]
    (bindings--define-key menu [debugger-separator]
    (bindings--define-key menu [cursor-separator]
    (bindings--define-key menu [edit-options-separator]
    (bindings--define-key menu [highlight-separator]
  lisp/net/newst-plainview.el
    (define-key map [newsticker-separator-1]
    (define-key map [newsticker-separator-2]
    (define-key map [newsticker-separator-3]
    (define-key map [newsticker-separator-4]
  lisp/so-long.el
    (define-key-after map [so-long-actions-separator]
    (define-key-after map [so-long-help-separator]

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Jim Porter]

On 12/5/2021 2:39 AM, Eli Zaretskii wrote:
>> Cc: 52286@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>> Date: Sat, 4 Dec 2021 23:44:07 -0800
>>
>>> The first pretest of Emacs 28 is already out, so this problem is
>>> already with us, isn't it?
>>
>> If it's too late to change this, that's ok. I just noticed it while
>> working on the patch for bug#52237 and figured it'd be good to file a
>> bug about it while there's still a non-zero chance it could be changed.
>> It's a pretty minor issue, so I don't really mind if this gets closed as
>> wontfix.
>>
>> If this change *is* ok to merge, I'll happily update the ELisp manual
>> with a brief summary of the convention before it gets merged.
> 
> I'd like to hear opinions of others.  Meanwhile, having the proposed
> convention spelled out in the manual would be good to make sure we are
> all on the same page regarding the issue.

Ok, I'll start working on some documentation. Do you have any suggestion 
on where it should go, or how general I should make it? (I could 
document the convention specifically for separators, or I could try to 
write up a more general guideline that could apply to any symbol name.)

> As for the issue itself: cannot we detect the separators in some more
> robust way, one that doesn't depend on how they are named?  And if
> that's not feasible, perhaps allow anything that has "separator" in
> its name to be considered a separator?

We can detect whether an item is a separator pretty easily without this 
change. However, context menu functions use specific separators to 
determine where exactly it should put new menu items. For example, 
`elisp-context-menu' adds menu items after `middle-separator':

     (define-key-after menu [elisp-separator] menu-bar-separator
       'middle-separator)

(Later in the function, it adds the actual menu items after the 
newly-added `elisp-separator'.)

The only issue (though it's a small one) is that some of the separator 
names added to the context menu by default are named `FOO-separator' and 
others are named `separator-FOO'. I thought it might be hard to remember 
which was which without double-checking, so my patch converts all of the 
names to use the `FOO-separator' format.

Of course, as Juri Linkov's message points out, many already-existing 
separators use both naming schemes, and it's too late to change those. 
Since the context menu is new, I thought it would be worth converting to 
a consistent naming scheme if it's not too late.

Still, this is a *very* small problem. I'm not sure third party packages 
would want to insert context menu items after, say, `separator-undo'; a 
user might customize `context-menu-functions' so that their menu doesn't 
even *have* a separator of that name. In practice, it's possible almost 
no one will even notice the different naming conventions.

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Eli Zaretskii]

> Cc: 52286@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Sun, 5 Dec 2021 18:09:22 -0800
> 
> >> If this change *is* ok to merge, I'll happily update the ELisp manual
> >> with a brief summary of the convention before it gets merged.
> > 
> > I'd like to hear opinions of others.  Meanwhile, having the proposed
> > convention spelled out in the manual would be good to make sure we are
> > all on the same page regarding the issue.
> 
> Ok, I'll start working on some documentation. Do you have any suggestion 
> on where it should go, or how general I should make it? (I could 
> document the convention specifically for separators, or I could try to 
> write up a more general guideline that could apply to any symbol name.)

I think the most natural place is where we describe how to construct
menus.

> > As for the issue itself: cannot we detect the separators in some more
> > robust way, one that doesn't depend on how they are named?  And if
> > that's not feasible, perhaps allow anything that has "separator" in
> > its name to be considered a separator?
> 
> We can detect whether an item is a separator pretty easily without this 
> change. However, context menu functions use specific separators to 
> determine where exactly it should put new menu items. For example, 
> `elisp-context-menu' adds menu items after `middle-separator':
> 
>      (define-key-after menu [elisp-separator] menu-bar-separator
>        'middle-separator)
> 
> (Later in the function, it adds the actual menu items after the 
> newly-added `elisp-separator'.)

If this is a private convention of elisp-mode, we don't have to codify
it.

> Still, this is a *very* small problem. I'm not sure third party packages 
> would want to insert context menu items after, say, `separator-undo'; a 
> user might customize `context-menu-functions' so that their menu doesn't 
> even *have* a separator of that name. In practice, it's possible almost 
> no one will even notice the different naming conventions.

So maybe instead of conventions we should have recommendations, with
explanation why following that could be useful in some situation.
Then let Lisp programmers decide what they want.

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Jim Porter]

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

On 12/6/2021 4:55 AM, Eli Zaretskii wrote:
> I think the most natural place is where we describe how to construct
> menus.

Thanks.

>> We can detect whether an item is a separator pretty easily without this
>> change. However, context menu functions use specific separators to
>> determine where exactly it should put new menu items. For example,
>> `elisp-context-menu' adds menu items after `middle-separator':
>>
>>       (define-key-after menu [elisp-separator] menu-bar-separator
>>         'middle-separator)
>>
>> (Later in the function, it adds the actual menu items after the
>> newly-added `elisp-separator'.)
> 
> If this is a private convention of elisp-mode, we don't have to codify
> it.

It's not just for `elisp-mode'; any mode (or other third-party code) may 
want to insert context menu items in a certain spot. `middle-separator' 
follows the naming convention I recommend, but a hypothetical mode might 
want to insert a new item just after the separator for the Undo section. 
However, that's currently named `separator-undo' on master, so it can be 
confusing to remember the difference in naming between these two cases:

   (define-key-after menu [my-separator] menu-bar-separator
     'middle-separator)

   (define-key-after menu [my-separator] menu-bar-separator
     'separator-undo)

> So maybe instead of conventions we should have recommendations, with
> explanation why following that could be useful in some situation.
> Then let Lisp programmers decide what they want.

Ok, I've tried to provide a brief explanation of the recommendation and 
the reasoning for it without *over*-explaining it. I also added a small 
explanation about how to use `menu-bar-separator', since I didn't see 
documentation on that in the manual, and it helped segue into an 
explanation about the naming recommendation.

[-- Attachment #2: 0001-Rename-context-menu-separators-to-FOO-separator-for-.patch --]
[-- Type: text/plain, Size: 5135 bytes --]

From 7b646daeb0a8c9838d92eb9c093ac203bd9e1f03 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 11 Dec 2021 12:44:12 -0800
Subject: [PATCH] Rename context menu separators to 'FOO-separator' for
 consistency

* lisp/mouse.el (context-menu-toolbar, context-menu-global)
(context-menu-local, context-menu-minor, context-menu-buffers)
(context-menu-vc, context-menu-undo, context-menu-region): Rename
separators to 'FOO-separator'.
* doc/lispref/keymaps.texi (Menu Separators): Add documentation
explaining 'menu-bar-separator' and naming convention of separators.
---
 doc/lispref/keymaps.texi | 15 ++++++++++++++-
 lisp/mouse.el            | 16 ++++++++--------
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/doc/lispref/keymaps.texi b/doc/lispref/keymaps.texi
index edf1d6e83f..4fb66cecde 100644
--- a/doc/lispref/keymaps.texi
+++ b/doc/lispref/keymaps.texi
@@ -2460,7 +2460,20 @@ Menu Separators
 
   A menu separator is a kind of menu item that doesn't display any
 text---instead, it divides the menu into subparts with a horizontal line.
-A separator looks like this in the menu keymap:
+To create a basic separator, you can use @code{menu-bar-separator} when
+defining the item:
+
+@example
+(define-key map [@var{foo-separator}] menu-bar-separator)
+@end example
+
+@noindent
+When possible, it's recommended to name the key for the separator with
+the form @var{foo-separator} so that it's clear the underlying object
+is a separator.  For example, @code{region-separator} is preferred
+over @code{separator-region}.
+
+  A separator looks like this in the menu keymap:
 
 @example
 (menu-item @var{separator-type})
diff --git a/lisp/mouse.el b/lisp/mouse.el
index 11fdd3f639..96205fa346 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -358,7 +358,7 @@ context-menu-middle-separator
 (defun context-menu-toolbar (menu _click)
   "Populate MENU with submenus from the tool bar."
   (run-hooks 'activate-menubar-hook 'menu-bar-update-hook)
-  (define-key-after menu [separator-toolbar] menu-bar-separator)
+  (define-key-after menu [toolbar-separator] menu-bar-separator)
   (map-keymap (lambda (key binding)
                 (when (consp binding)
                   (define-key-after menu (vector key)
@@ -369,7 +369,7 @@ context-menu-toolbar
 (defun context-menu-global (menu _click)
   "Populate MENU with submenus from the global menu."
   (run-hooks 'activate-menubar-hook 'menu-bar-update-hook)
-  (define-key-after menu [separator-global] menu-bar-separator)
+  (define-key-after menu [global-separator] menu-bar-separator)
   (map-keymap (lambda (key binding)
                 (when (consp binding)
                   (define-key-after menu (vector key)
@@ -380,7 +380,7 @@ context-menu-global
 (defun context-menu-local (menu _click)
   "Populate MENU with submenus provided by major mode."
   (run-hooks 'activate-menubar-hook 'menu-bar-update-hook)
-  (define-key-after menu [separator-local] menu-bar-separator)
+  (define-key-after menu [local-separator] menu-bar-separator)
   (let ((keymap (local-key-binding [menu-bar])))
     (when keymap
       (map-keymap (lambda (key binding)
@@ -393,7 +393,7 @@ context-menu-local
 (defun context-menu-minor (menu _click)
   "Populate MENU with submenus provided by minor modes."
   (run-hooks 'activate-menubar-hook 'menu-bar-update-hook)
-  (define-key-after menu [separator-minor] menu-bar-separator)
+  (define-key-after menu [minor-separator] menu-bar-separator)
   (dolist (mode (reverse (minor-mode-key-binding [menu-bar])))
     (when (and (consp mode) (symbol-value (car mode)))
       (map-keymap (lambda (key binding)
@@ -406,7 +406,7 @@ context-menu-minor
 (defun context-menu-buffers (menu _click)
   "Populate MENU with the buffer submenus to buffer switching."
   (run-hooks 'activate-menubar-hook 'menu-bar-update-hook)
-  (define-key-after menu [separator-buffers] menu-bar-separator)
+  (define-key-after menu [buffers-separator] menu-bar-separator)
   (map-keymap (lambda (key binding)
                 (when (consp binding)
                   (define-key-after menu (vector key)
@@ -416,13 +416,13 @@ context-menu-buffers
 
 (defun context-menu-vc (menu _click)
   "Populate MENU with Version Control commands."
-  (define-key-after menu [separator-vc] menu-bar-separator)
+  (define-key-after menu [vc-separator] menu-bar-separator)
   (define-key-after menu [vc-menu] vc-menu-entry)
   menu)
 
 (defun context-menu-undo (menu _click)
   "Populate MENU with undo commands."
-  (define-key-after menu [separator-undo] menu-bar-separator)
+  (define-key-after menu [undo-separator] menu-bar-separator)
   (when (and (not buffer-read-only)
              (not (eq t buffer-undo-list))
              (if (eq last-command 'undo)
@@ -440,7 +440,7 @@ context-menu-undo
 
 (defun context-menu-region (menu click)
   "Populate MENU with region commands."
-  (define-key-after menu [separator-region] menu-bar-separator)
+  (define-key-after menu [region-separator] menu-bar-separator)
   (when (and mark-active (not buffer-read-only))
     (define-key-after menu [cut]
       '(menu-item "Cut" kill-region
-- 
2.25.1

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Juri Linkov]

> It's not just for `elisp-mode'; any mode (or other third-party code) may
> want to insert context menu items in a certain spot. `middle-separator'
> follows the naming convention I recommend, but a hypothetical mode might
> want to insert a new item just after the separator for the Undo
> section. However, that's currently named `separator-undo' on master, so it
> can be confusing to remember the difference in naming between these two
> cases:
>
>   (define-key-after menu [my-separator] menu-bar-separator
>     'middle-separator)
>
>   (define-key-after menu [my-separator] menu-bar-separator
>     'separator-undo)

If it's too late to push this to the release branch,
then this definitely can't be done after the release.
So probably this bug report should be closed?

Meanwhile, I noticed another inconsistency
where context menus for some modes are named
with the -mode suffix, and some without it.

With `-mode':

  lisp/help-mode.el
  (defun help-mode-context-menu (menu click)

  lisp/textmodes/text-mode.el
  (defun text-mode-context-menu (menu click)

Without `-mode':

  lisp/progmodes/prog-mode.el
  (defun prog-context-menu (menu click)

  lisp/progmodes/elisp-mode.el
  (defun elisp-context-menu (menu click)

Maybe it's too late to fix this inconsistency too?

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  52286@debbugs.gnu.org
> Date: Sat, 15 Jan 2022 20:57:39 +0200
> 
> With `-mode':
> 
>   lisp/help-mode.el
>   (defun help-mode-context-menu (menu click)
> 
>   lisp/textmodes/text-mode.el
>   (defun text-mode-context-menu (menu click)
> 
> Without `-mode':
> 
>   lisp/progmodes/prog-mode.el
>   (defun prog-context-menu (menu click)
> 
>   lisp/progmodes/elisp-mode.el
>   (defun elisp-context-menu (menu click)
> 
> Maybe it's too late to fix this inconsistency too?

I don't even understand  why we should care about this nit.

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Juri Linkov]

>> With `-mode':
>> 
>>   lisp/help-mode.el
>>   (defun help-mode-context-menu (menu click)
>> 
>>   lisp/textmodes/text-mode.el
>>   (defun text-mode-context-menu (menu click)
>> 
>> Without `-mode':
>> 
>>   lisp/progmodes/prog-mode.el
>>   (defun prog-context-menu (menu click)
>> 
>>   lisp/progmodes/elisp-mode.el
>>   (defun elisp-context-menu (menu click)
>> 
>> Maybe it's too late to fix this inconsistency too?
>
> I don't even understand  why we should care about this nit.

For purely aesthetic purposes :-)

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: jporterbugs@gmail.com,  52286@debbugs.gnu.org
> Date: Sat, 15 Jan 2022 21:19:04 +0200
> 
> >> With `-mode':
> >> 
> >>   lisp/help-mode.el
> >>   (defun help-mode-context-menu (menu click)
> >> 
> >>   lisp/textmodes/text-mode.el
> >>   (defun text-mode-context-menu (menu click)
> >> 
> >> Without `-mode':
> >> 
> >>   lisp/progmodes/prog-mode.el
> >>   (defun prog-context-menu (menu click)
> >> 
> >>   lisp/progmodes/elisp-mode.el
> >>   (defun elisp-context-menu (menu click)
> >> 
> >> Maybe it's too late to fix this inconsistency too?
> >
> > I don't even understand  why we should care about this nit.
> 
> For purely aesthetic purposes :-)

I suggest to forget about this.  We have enough real problems on our
hands.

bug#52286: 28.0.90; [PATCH] Be consistent in naming of separators in context menu

[Juri Linkov]

tags 52286 wontfix
close 52286 29.0.50
quit

>> >> Maybe it's too late to fix this inconsistency too?
>> >
>> > I don't even understand  why we should care about this nit.
>> 
>> For purely aesthetic purposes :-)
>
> I suggest to forget about this.  We have enough real problems on our
> hands.

Ok, so closing.