Skip to content

improve discoverability of terminal suggest configuration #252543

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

Merged
merged 10 commits into from
Jun 30, 2025

Conversation

meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Jun 26, 2025

fix #252321

The Learn More action is highlighted for the first ten shows or for the first time the widgt shows for 10 seconds per machine.

Screenshot 2025-06-27 at 5 08 01 PM

To do:

  • do not merge until we have a live link

@meganrogge meganrogge requested a review from Tyriar June 26, 2025 19:54
@meganrogge meganrogge self-assigned this Jun 26, 2025
@meganrogge meganrogge added this to the July 2025 milestone Jun 26, 2025
@meganrogge meganrogge enabled auto-merge (squash) June 27, 2025 21:08
Tyriar
Tyriar previously requested changes Jun 27, 2025
@meganrogge meganrogge modified the milestones: July 2025, June 2025 Jun 30, 2025
@meganrogge
Copy link
Contributor Author

@Tyriar i've added a reset command and will make an issue to remove that after testing

@meganrogge meganrogge requested a review from Tyriar June 30, 2025 16:08
Tyriar
Tyriar previously requested changes Jun 30, 2025
Comment on lines 312 to 327
registerTerminalAction({
id: TerminalSuggestCommandId.ResetDiscoverability,
title: localize2('workbench.action.terminal.resetDiscoverability', 'Reset Discoverability'),
f1: true,
precondition: ContextKeyExpr.and(
ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
TerminalContextKeys.focus,
TerminalContextKeys.isOpen,
ContextKeyExpr.equals(`config.${TerminalSuggestSettingId.Enabled}`, true)
),
run: (c, accessor) => {
const storageService = accessor.get(IStorageService);
storageService.store(TERMINAL_SUGGEST_DISCOVERABILITY_KEY, false, StorageScope.APPLICATION, StorageTarget.MACHINE);
storageService.store(TERMINAL_SUGGEST_DISCOVERABILITY_COUNT_KEY, 0, StorageScope.APPLICATION, StorageTarget.MACHINE);
}
});
Copy link
Member

Choose a reason for hiding this comment

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

We can probably do without this? A command named Terminal: Reset Discoverability just raises questions imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#253032

will be very helpful for testing

Copy link
Member

Choose a reason for hiding this comment

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

That's fine if we remove. You could also instruct the tester to use --user-data-dir ./tmp to reset it, though that wouldn't work if you're testing syncing.

@meganrogge meganrogge requested a review from Tyriar June 30, 2025 16:15
@meganrogge meganrogge merged commit 6943e61 into main Jun 30, 2025
8 checks passed
@meganrogge meganrogge deleted the merogge/discoverability branch June 30, 2025 17:06
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.

Improve terminal suggest configuration discoverability
2 participants