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 2 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 48m 12s ⏱️ - 6m 13s
 7 724 tests ±0   7 495 ✅ ±0  228 💤 ±0  1 ❌ ±0 
24 333 runs  ±0  23 585 ✅ ±0  747 💤 ±0  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit 000f4db. ± Comparison against base commit 83c128c.

♻️ 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
Contributor

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
Contributor

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!

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.

2 participants