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

Allow text search providers to give messages with links that trigger researching. #123213

Merged
merged 6 commits into from May 13, 2021

Conversation

JacksonKearl
Copy link
Contributor

@JacksonKearl JacksonKearl commented May 6, 2021

cc @joaomoreno this modifies the Link class to allow setting a custom opener, to enable search to interpret the output of commands in provider messages in a way specific to the particular search UI.

@JacksonKearl JacksonKearl self-assigned this May 6, 2021
@JacksonKearl JacksonKearl added this to the May 2021 milestone May 6, 2021
@JacksonKearl JacksonKearl added the search Search widget and operation issues label May 6, 2021
src/vs/vscode.proposed.d.ts Outdated Show resolved Hide resolved
Comment on lines 69 to 72
setHandler(handler: (href: string) => void) {
this.handler = handler;
}

Copy link
Member

Choose a reason for hiding this comment

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

Let's just add an options? bag to the ctor of Link instead of a mutable setHandler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I initially didn't just because the injected openerService means everywhere this is constructed would need to explicitly pass the empty bag or undefined.

src/vs/platform/opener/browser/link.ts Outdated Show resolved Hide resolved
@@ -579,7 +579,7 @@ export abstract class ViewPane extends Pane implements IView {
if (typeof node === 'string') {
append(p, document.createTextNode(node));
} else {
const link = this.instantiationService.createInstance(Link, node);
const link = this.instantiationService.createInstance(Link, node, {});
Copy link
Member

Choose a reason for hiding this comment

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

The last parameter is optional, does the instantiation service make you pass it in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TS doesn't allow the required service injection parameters follow an optional parameter.

if (parsed.scheme === Schemas.command) {
const result = await this.commandService.executeCommand(parsed.path);
if ((result as any)?.triggerSearch) {
this.triggerQueryChange();
Copy link
Member

Choose a reason for hiding this comment

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

What command are you going to use this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

remoteHub.enableIndexing

@JacksonKearl JacksonKearl force-pushed the jackson/allow-messages-to-trigger-search branch from b38064f to cb616bc Compare May 11, 2021 04:58
@JacksonKearl JacksonKearl enabled auto-merge (squash) May 13, 2021 04:43
@JacksonKearl JacksonKearl merged commit ea39e2b into main May 13, 2021
@JacksonKearl JacksonKearl deleted the jackson/allow-messages-to-trigger-search branch May 13, 2021 04:43
@github-actions github-actions bot locked and limited conversation to collaborators Jun 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
search Search widget and operation issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants