GNU bug report logs -
#52237
29.0.50; [PATCH] Doubled separators in context-menu-mode
Previous Next
Reported by: Jim Porter <jporterbugs <at> gmail.com>
Date: Thu, 2 Dec 2021 06:07:01 UTC
Severity: normal
Tags: moreinfo, patch
Found in version 29.0.50
Fixed in version 29.1
Done: Lars Ingebrigtsen <larsi <at> gnus.org>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 52237 in the body.
You can then email your comments to 52237 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Thu, 02 Dec 2021 06:07:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Jim Porter <jporterbugs <at> gmail.com>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Thu, 02 Dec 2021 06:07:01 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Sometimes, menu separators are doubled up in context-menu-mode. To see
this in action:
emacs -Q --eval '(context-menu-mode)'
;; Right-click somewhere in the scratch buffer, like the empty area.
You should see a double separator just before "Lisp-Interaction".
`context-menu-map' tries to eliminate doubled separators, but it doesn't
account for *tripled* separators. If we have a menu like so ("->" is the
current list item when iterating):
-> (separator-foo "--")
(separator-bar "--")
(separator-baz "--")
(regular-item "Item")
The duplicate remover sees that both the current item and the next are
separators, so removes it:
-> (separator-foo "--")
(separator-baz "--")
(regular-item "Item")
But then the very next operation is to move to the next list item:
(separator-foo "--")
-> (separator-baz "--")
(regular-item "Item")
Now, on the next iteration of the while loop, it won't detect the
duplicate because it's too far ahead. Attached is a patch to fix this;
it only advances to the next list item when it *didn't* just delete a
duplicate separator. That way, it can keep deleting subsequent dupes
until it sees a non-separator item.
I've only tested this on Emacs 29 so far, but it may occur on Emacs 28
as well. Also, I noticed that separators can appear at the beginning
and/or end of the context menu. Should they be removed too?
[0001-Ensure-there-are-no-duplicate-separators-when-creati.patch (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Thu, 02 Dec 2021 08:25:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 52237 <at> debbugs.gnu.org (full text, mbox):
Jim Porter <jporterbugs <at> gmail.com> writes:
> I've only tested this on Emacs 29 so far, but it may occur on Emacs 28
> as well.
Thanks; applied to Emacs 29.
> Also, I noticed that separators can appear at the beginning
> and/or end of the context menu. Should they be removed too?
Hm... I think that might make sense, but I'm not sure. Anybody got an
opinion?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Added tag(s) moreinfo.
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org
.
(Thu, 02 Dec 2021 08:25:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Thu, 02 Dec 2021 17:33:01 GMT)
Full text and
rfc822 format available.
Message #13 received at 52237 <at> debbugs.gnu.org (full text, mbox):
Why should any repetition of separators be removed
or ignored?
If someone codes that then they presumably want that.
Vanilla Emacs need not code such repetition.
Runtime "fixes" of such repetition should be a no-no.
If you don't want consecutive separators then don't
use them. End of story.
If Emacs automatically ignores or removes separators
then that behavior is too clever by half, IMHO.
(Apologies, if I've misunderstood what this feature,
"`context-menu-map' tries to eliminate doubled
separators", is really about. But if I've understood
correctly then I suggest this control is misguided.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Thu, 02 Dec 2021 17:59:02 GMT)
Full text and
rfc822 format available.
Message #16 received at 52237 <at> debbugs.gnu.org (full text, mbox):
>> I've only tested this on Emacs 29 so far, but it may occur on Emacs 28
>> as well.
>
> Thanks; applied to Emacs 29.
Why not to Emacs 28?
>> Also, I noticed that separators can appear at the beginning
>> and/or end of the context menu. Should they be removed too?
>
> Hm... I think that might make sense, but I'm not sure. Anybody got an
> opinion?
Removing leading/trailing separators would be nice too.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Thu, 02 Dec 2021 18:10:02 GMT)
Full text and
rfc822 format available.
Message #19 received at 52237 <at> debbugs.gnu.org (full text, mbox):
On 12/2/2021 9:31 AM, Drew Adams wrote:
> Why should any repetition of separators be removed
> or ignored?
>
> If someone codes that then they presumably want that.
> Vanilla Emacs need not code such repetition.
As far as I understand (which isn't very far; I've only just started
tinkering with context-menu-mode), the general idea is that the context
menu is generated dynamically by a list of functions stored in
`context-menu-functions'. Each of these can add items to the menu. Some
of these, like `context-menu-minor', first add a separator and then
iterate over a list of things (minor modes in this case) to add more
items. If that list is empty, you just get a separator, but then that
separator might get doubled up with the separator from the *next*
context menu function.
In some cases, these separators are used as anchors to determine where
to put the results of *later* context menu functions too. For example,
`context-menu-middle-separator' is one of the default entries in
`context-menu-functions', and as the name implies, it *only* adds a
separator. Some other context menu functions (e.g. `elisp-context-menu')
look for that separator to know where to put new menu items, so we want
that separator to be there during construction, even if it might result
in duplicated separators by the end (which `context-menu-map' would then
strip out before display). This logic could apply to separators
generated by other functions too, such as `context-menu-minor' described
above.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Thu, 02 Dec 2021 18:26:02 GMT)
Full text and
rfc822 format available.
Message #22 received at 52237 <at> debbugs.gnu.org (full text, mbox):
I see. In that case, maybe there should be
an enhancement request, to be able to produce
consecutive separators in a context menu.
(No, I'm not requesting that, myself.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Thu, 02 Dec 2021 18:48:01 GMT)
Full text and
rfc822 format available.
Message #25 received at 52237 <at> debbugs.gnu.org (full text, mbox):
On 12/1/2021 10:06 PM, Jim Porter wrote:
> Sometimes, menu separators are doubled up in context-menu-mode.
I found another odd case, but I'm not 100% sure the best way to fix it:
emacs -Q --eval '(context-menu-mode)'
C-h o identity RET
;; Right-click somewhere in the Help buffer
There's a doubled separator after "Next Topic". Looking at the code,
this is because `help-mode-context-menu' inserts new items using
`define-key', which has the effect of putting the new items *before* the
(hidden) menu title. The resulting keymap ends up looking like this:
(keymap
(Previous\ Topic ...)
(Next\ Topic ...)
(help-mode-separator "--")
#("Context Menu" 0 12 (hide t))
(separator-undo "--")
...)
Since there's a hidden item (the keymap title) between the
`help-mode-separator' and `separator-undo'[1], the de-duplication
doesn't handle that. Is there a standard/common way of defining a key
such that it goes immediately *after* the keymap's title? I guess we
could add `context-menu-top-separator' as an anchor (by analogy to
`context-menu-middle-separator'), but maybe there's a simpler way...
[1] As an aside, is there a standard naming convention to use here?
Should "separator" go at the beginning of the name or the end?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Fri, 03 Dec 2021 04:47:01 GMT)
Full text and
rfc822 format available.
Message #28 received at 52237 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 12/2/2021 9:44 AM, Juri Linkov wrote:
>>> I've only tested this on Emacs 29 so far, but it may occur on Emacs 28
>>> as well.
>>
>> Thanks; applied to Emacs 29.
>
> Why not to Emacs 28?
I got a chance to check this under Emacs 28, and the bug exists there
too, so it'd be nice to backport.
>>> Also, I noticed that separators can appear at the beginning
>>> and/or end of the context menu. Should they be removed too?
>>
>> Hm... I think that might make sense, but I'm not sure. Anybody got an
>> opinion?
>
> Removing leading/trailing separators would be nice too.
Ok, here's a patch to do it. There might be a better way to do this, but
it should work reliably.
Note: this patch doesn't fix the duplicated separators in the help-mode
context menu. I haven't had time to look into the right way to fix that
yet (there may be other mode-specific context menu functions that have
this bug too).
[0001-Remove-separators-at-the-beginning-and-end-of-the-co.patch (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Fri, 03 Dec 2021 16:09:02 GMT)
Full text and
rfc822 format available.
Message #31 received at 52237 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
>>> I've only tested this on Emacs 29 so far, but it may occur on Emacs 28
>>> as well.
>>
>> Thanks; applied to Emacs 29.
>
> Why not to Emacs 28?
We're only fixing regressions on the release branch now, so I don't
think this qualifies? (And even if it did, since it's a visual thing,
I'm not sure it's important enough anyway.)
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Fri, 03 Dec 2021 16:11:02 GMT)
Full text and
rfc822 format available.
Message #34 received at 52237 <at> debbugs.gnu.org (full text, mbox):
Jim Porter <jporterbugs <at> gmail.com> writes:
>> Removing leading/trailing separators would be nice too.
>
> Ok, here's a patch to do it. There might be a better way to do this,
> but it should work reliably.
Thanks; applied to Emacs 29.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
bug marked as fixed in version 29.1, send any further explanations to
52237 <at> debbugs.gnu.org and Jim Porter <jporterbugs <at> gmail.com>
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org
.
(Fri, 03 Dec 2021 16:11:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Sat, 04 Dec 2021 06:45:02 GMT)
Full text and
rfc822 format available.
Message #39 received at 52237 <at> debbugs.gnu.org (full text, mbox):
On 12/3/2021 8:08 AM, Lars Ingebrigtsen wrote:
> We're only fixing regressions on the release branch now, so I don't
> think this qualifies? (And even if it did, since it's a visual thing,
> I'm not sure it's important enough anyway.)
Just so I know, for stuff like this, is there a chance it could be
backported at some point for 28.2 (assuming that version gets published
in the future), or would it have to wait until 29.1?
If the former, I'll try to remember to remind folks post-28.1 about this
bug so it can be backported.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Sat, 04 Dec 2021 08:27:02 GMT)
Full text and
rfc822 format available.
Message #42 received at 52237 <at> debbugs.gnu.org (full text, mbox):
> Resent-From: Jim Porter <jporterbugs <at> gmail.com>
> Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
> Resent-CC: bug-gnu-emacs <at> gnu.org
> Resent-Sender: help-debbugs <at> gnu.org
> From: Jim Porter <jporterbugs <at> gmail.com>
> Date: Fri, 3 Dec 2021 22:44:43 -0800
> Cc: 52237 <at> debbugs.gnu.org
>
> On 12/3/2021 8:08 AM, Lars Ingebrigtsen wrote:
> > We're only fixing regressions on the release branch now, so I don't
> > think this qualifies? (And even if it did, since it's a visual thing,
> > I'm not sure it's important enough anyway.)
>
> Just so I know, for stuff like this, is there a chance it could be
> backported at some point for 28.2 (assuming that version gets published
> in the future), or would it have to wait until 29.1?
We could consider it for Emacs 28.2, yes, assuming that it wouldn't
cause any regressions or adverse side effects.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Sat, 04 Dec 2021 20:10:03 GMT)
Full text and
rfc822 format available.
Message #45 received at 52237 <at> debbugs.gnu.org (full text, mbox):
> I found another odd case, but I'm not 100% sure the best way to fix it:
>
> emacs -Q --eval '(context-menu-mode)'
> C-h o identity RET
> ;; Right-click somewhere in the Help buffer
>
> There's a doubled separator after "Next Topic". Looking at the code, this
> is because `help-mode-context-menu' inserts new items using `define-key',
> which has the effect of putting the new items *before* the (hidden) menu
> title. The resulting keymap ends up looking like this:
>
> (keymap
> (Previous\ Topic ...)
> (Next\ Topic ...)
> (help-mode-separator "--")
> #("Context Menu" 0 12 (hide t))
> (separator-undo "--")
> ...)
The core function that displays the menu can handle
the menu title in the middle of the menu, so it wasn't
a problem until the recent need to remove duplicate items.
> Since there's a hidden item (the keymap title) between the
> `help-mode-separator' and `separator-undo'[1], the de-duplication doesn't
> handle that. Is there a standard/common way of defining a key such that it
> goes immediately *after* the keymap's title? I guess we could add
> `context-menu-top-separator' as an anchor (by analogy to
> `context-menu-middle-separator'), but maybe there's a simpler way...
I see no simpler way, so perhaps we need to add `context-menu-top-separator'.
> [1] As an aside, is there a standard naming convention to use here? Should
> "separator" go at the beginning of the name or the end?
`context-menu-middle-separator' is a function, so it requires the
`context-menu-' prefix. It adds the menu item [middle-separator].
Since "middle" is an adjective, the word order can't be "separator-middle".
But "buffers-separator" doesn't look nicer than `separator-buffers'.
If you could propose a more consistent naming convention,
maybe it's not too late to rename these symbols on the release branch,
because after the release it will be impossible to rename them.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Sat, 04 Dec 2021 20:57:01 GMT)
Full text and
rfc822 format available.
Message #48 received at 52237 <at> debbugs.gnu.org (full text, mbox):
On 12/4/2021 11:50 AM, Juri Linkov wrote:
>> I found another odd case, but I'm not 100% sure the best way to fix it:
>>
>> emacs -Q --eval '(context-menu-mode)'
>> C-h o identity RET
>> ;; Right-click somewhere in the Help buffer
>>
>> There's a doubled separator after "Next Topic".
[snip]
> I see no simpler way, so perhaps we need to add `context-menu-top-separator'.
Sounds good. I'll work on a patch for this and file a new bug for it
once it's ready.
>> [1] As an aside, is there a standard naming convention to use here? Should
>> "separator" go at the beginning of the name or the end?
>
> `context-menu-middle-separator' is a function, so it requires the
> `context-menu-' prefix. It adds the menu item [middle-separator].
> Since "middle" is an adjective, the word order can't be "separator-middle".
> But "buffers-separator" doesn't look nicer than `separator-buffers'.
I think `FOO-separator' is the best choice here for the separators
themselves. `FOO' acts as a noun adjunct[1], modifying the underlying
object: a separator. For functions that just make a separator, they'd be
named `MODULE-[FOO-]-separator', with `FOO-' being optional if it would
be redundant with the module name. So then
`context-menu-middle-separator' is the right function name, and it adds
a separator named `middle-separator'.
`buffers-separator' is a bit of an odd phrasing since, as the Wikipedia
article mentions, "Noun adjuncts were traditionally mostly singular".
However, I think it's still a bit easier to read that correctly as "the
separator used to mark the buffers", whereas `separator-buffers' reads
more like "buffers used to separate things" to me.
Since this should just be a trivial renaming, hopefully we can merge
that into Emacs 28 so we're not locked into the current names.
[1] https://en.wikipedia.org/wiki/Noun_adjunct
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Sat, 04 Dec 2021 22:10:01 GMT)
Full text and
rfc822 format available.
Message #51 received at 52237 <at> debbugs.gnu.org (full text, mbox):
On 12/4/2021 12:56 PM, Jim Porter wrote:
> On 12/4/2021 11:50 AM, Juri Linkov wrote:
>> `context-menu-middle-separator' is a function, so it requires the
>> `context-menu-' prefix. It adds the menu item [middle-separator].
>> Since "middle" is an adjective, the word order can't be
>> "separator-middle".
>> But "buffers-separator" doesn't look nicer than `separator-buffers'.
>
> I think `FOO-separator' is the best choice here for the separators
> themselves. `FOO' acts as a noun adjunct[1], modifying the underlying
> object: a separator. For functions that just make a separator, they'd be
> named `MODULE-[FOO-]-separator', with `FOO-' being optional if it would
> be redundant with the module name. So then
> `context-menu-middle-separator' is the right function name, and it adds
> a separator named `middle-separator'.
>
> `buffers-separator' is a bit of an odd phrasing since, as the Wikipedia
> article mentions, "Noun adjuncts were traditionally mostly singular".
> However, I think it's still a bit easier to read that correctly as "the
> separator used to mark the buffers", whereas `separator-buffers' reads
> more like "buffers used to separate things" to me.
>
> Since this should just be a trivial renaming, hopefully we can merge
> that into Emacs 28 so we're not locked into the current names.
>
> [1] https://en.wikipedia.org/wiki/Noun_adjunct
Ok, filed bug#52286 for this:
https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-12/msg00327.html
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Sun, 05 Dec 2021 10:05:03 GMT)
Full text and
rfc822 format available.
Message #54 received at 52237 <at> debbugs.gnu.org (full text, mbox):
> Ok, here's a patch to do it. There might be a better way to do this, but it
> should work reliably.
>
> - (while (consp l)
> + (while (consp (cdr l))
This change broke flyspell-correct-word. When a context menu is invoked
on a misspelled word in flyspell-mode, context-menu-map uses the property
context-menu-function to get a command symbol 'flyspell-correct-word'
instead of a list of menu items.
bug#50851 fixed this error by checking for a list with `(consp l)' like above:
(consp 'flyspell-correct-word) => nil
but now 'cdr' fails on (consp (cdr 'flyspell-correct-word)).
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52237
; Package
emacs
.
(Sun, 05 Dec 2021 20:22:02 GMT)
Full text and
rfc822 format available.
Message #57 received at 52237 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
>> - (while (consp l)
>> + (while (consp (cdr l))
>
> This change broke flyspell-correct-word. When a context menu is invoked
> on a misspelled word in flyspell-mode, context-menu-map uses the property
> context-menu-function to get a command symbol 'flyspell-correct-word'
> instead of a list of menu items.
>
> bug#50851 fixed this error by checking for a list with `(consp l)' like above:
>
> (consp 'flyspell-correct-word) => nil
>
> but now 'cdr' fails on (consp (cdr 'flyspell-correct-word)).
OK; I've added another (and (consp there now.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Mon, 03 Jan 2022 12:24:07 GMT)
Full text and
rfc822 format available.
This bug report was last modified 2 years and 125 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.