-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Connect: Collect protocol origin #23898
Changes from 2 commits
05bff2a
f01a9e8
28fcf3d
c9d5006
6a0332b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,7 +35,12 @@ import { assertUnreachable, retryWithRelogin } from '../utils'; | |
|
||
export default function useQuickInput() { | ||
const appContext = useAppContext(); | ||
const { quickInputService, workspacesService, commandLauncher } = appContext; | ||
const { | ||
quickInputService, | ||
workspacesService, | ||
commandLauncher, | ||
usageService, | ||
} = appContext; | ||
workspacesService.useState(); | ||
const documentsService = | ||
workspacesService.getActiveWorkspaceDocumentService(); | ||
|
@@ -105,6 +110,14 @@ export default function useQuickInput() { | |
const params = routing.parseClusterUri( | ||
workspacesService.getActiveWorkspace()?.localClusterUri | ||
).params; | ||
// ugly hack but QuickInput will be removed in v13 | ||
if (inputValue.startsWith('tsh proxy db')) { | ||
usageService.captureProtocolUse( | ||
workspacesService.getRootClusterUri(), | ||
'db', | ||
'search_bar' | ||
); | ||
} | ||
Comment on lines
+113
to
+120
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is for a scenario where someone autocompletes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, initially I wanted to ignore it, but then I thought that I could catch it with a simple |
||
documentsService.openNewTerminal({ | ||
initCommand: inputValue, | ||
rootClusterId: routing.parseClusterUri( | ||
|
@@ -120,6 +133,7 @@ export default function useQuickInput() { | |
commandLauncher.executeCommand('tsh-ssh', { | ||
loginHost: command.loginHost, | ||
localClusterUri, | ||
origin: 'search_bar', | ||
}); | ||
break; | ||
} | ||
|
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.
What do you think about wrapping the
resource_table
in some sort of an object argument so that the call looks like this?While the ID argument is fine as a positional argument, I feel like
origin
being a positional argument will be hard to understand for someone not as familiar with the codebase as we are.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.
I like it,
'resource_table'
alone doesn't tell much, but when combined withorigin
it is much more readable.