-
Notifications
You must be signed in to change notification settings - Fork 33.4k
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
Conversation
src/vs/workbench/contrib/terminalContrib/suggest/browser/terminal.suggest.contribution.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestAddon.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/suggest/common/terminal.suggest.ts
Outdated
Show resolved
Hide resolved
@Tyriar i've added a reset command and will make an issue to remove that after testing |
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); | ||
} | ||
}); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will be very helpful for testing
There was a problem hiding this comment.
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.
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.To do: