Skip to content
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

Avoid "null" in "Show In" menu when there is no key binding #2472

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

orionll
Copy link
Contributor

@orionll orionll commented Oct 31, 2024

When the "Show In" command has no key binding (we disable it for our reasons in our product), there is a null label in the menu:

image

The proposed change fixes this issue.

Copy link
Contributor

github-actions bot commented Oct 31, 2024

Test Results

 1 821 files  ±0   1 821 suites  ±0   1h 55m 47s ⏱️ - 9m 43s
 7 726 tests ±0   7 498 ✅ +3  228 💤 ±0  0 ❌  - 3 
24 339 runs  ±0  23 592 ✅ +3  747 💤 ±0  0 ❌  - 3 

Results for commit bf85c6e. ± Comparison against base commit 6a3eb35.

♻️ This comment has been updated with latest results.

@@ -45,6 +45,6 @@ private String getShowInMenuLabel() {
? bindingService
.getBestActiveBindingFormattedFor(IWorkbenchCommandConstants.NAVIGATE_SHOW_IN_QUICK_MENU)
: ""; //$NON-NLS-1$
return WorkbenchNavigatorMessages.ShowInActionProvider_showInAction_label + '\t' + keyBinding;
return WorkbenchNavigatorMessages.ShowInActionProvider_showInAction_label + (keyBinding != null ? "\t" + keyBinding : ""); //$NON-NLS-1$ //$NON-NLS-2$
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a static analysis, this looks sound. I looked into other ActionProviders and couldn't see in a glance why they don't have the same problem and how they solve that problem. Is there any mechanism we might have missed here?

}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please keep the diff clean!

@eclipse-platform-bot
Copy link
Contributor

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

bundles/org.eclipse.ui.navigator.resources/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch
From c0b47c576e58cf4d5a0e383bd176f49b69aa8428 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Tue, 26 Nov 2024 20:28:05 +0000
Subject: [PATCH] Version bump(s) for 4.35 stream


diff --git a/bundles/org.eclipse.ui.navigator.resources/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.navigator.resources/META-INF/MANIFEST.MF
index 5e9f968e31..52eee57bc2 100644
--- a/bundles/org.eclipse.ui.navigator.resources/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.ui.navigator.resources/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Plugin.name
 Bundle-SymbolicName: org.eclipse.ui.navigator.resources; singleton:=true
-Bundle-Version: 3.9.500.qualifier
+Bundle-Version: 3.9.600.qualifier
 Bundle-Activator: org.eclipse.ui.internal.navigator.resources.plugin.WorkbenchNavigatorPlugin
 Bundle-Vendor: %Plugin.providerName
 Bundle-Localization: plugin
-- 
2.47.0

Further information are available in Common Build Issues - Missing version increments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants