Skip to content

Wrong key binding displayed in MenuItem #2968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
2 tasks done
ghentschke opened this issue May 9, 2025 · 3 comments · May be fixed by #2969
Open
2 tasks done

Wrong key binding displayed in MenuItem #2968

ghentschke opened this issue May 9, 2025 · 3 comments · May be fixed by #2969
Labels
bug Something isn't working

Comments

@ghentschke
Copy link
Contributor

The org.eclipse.ui.bindings extension point allows to define a key sequence for a given commandId in a schemeId:

<key sequence="M2+M3+R"
      commandId="org.eclipse.ui.edit.rename"
      contextId="org.eclipse.ui.textEditorScope"
      schemeId="org.eclipse.ui.defaultAcceleratorConfiguration" />

Lets assume that the used commandId is a platform id (for example: org.eclipse.ui.edit.rename)
This commandId has been added to a context menu via this location URI: popup:#TextEditorContext:

<extension point="org.eclipse.ui.menus">
  <menuContribution
        allPopups="true"
        locationURI="popup:#TextEditorContext?after=additions">
     <menu id="org.eclipse.lsp4e.menu.refactorings"
           label="%refactorings.menu.label">
        <command commandId="org.eclipse.ui.edit.rename" style="push" />
        <separator name="org.eclipse.lsp4e.refactoringseparator" />
     </menu>

The displayed key binding in the context menu does not reflect the correct key binding (see this lsp4e issue #1259), because the contextId is not considered when the MenuItem gets updated here .
If there are more than one binding for a ParameterizedCommand, the first one gets selected after they have been sorted here.

As "F2" is shorter than "M2+M3+R", "F2" is sorted before "M2+M3+R" and gets displayed in the menu entry. This behavior leads to confused users as they expect another key binding in their editor.

Community

  • I understand reporting an issue to this OSS project does not mandate anyone to fix it. Other contributors may consider the issue, or not, at their own convenience. The most efficient way to get it fixed is that I fix it myself and contribute it back as a good quality patch to the project.
@iloveeclipse
Copy link
Member

@ghentschke : do you plan to provide a fix?

@ghentschke
Copy link
Contributor Author

I would like to. I just look for a way to determine the context id when the MenuItem gets updated. It looks to me that the org.eclipse.e4.ui.bindings.internal.BindingTableManager holds no (direct) information about the context.

ghentschke added a commit to ghentschke/eclipse.platform.text-fork that referenced this issue May 11, 2025
Consider the Binding context when there is more than one Binding
for a ParametrizedCommand. The deeper/specialized context will be
preferred.

fixes eclipse-platform#2968
@ghentschke
Copy link
Contributor Author

@iloveeclipse can you please take a look on my PR #2969

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants