Skip to content

fix selections not included in implicit#276604

Merged
justschen merged 1 commit intorelease/1.106from
justin/implicit-fix
Nov 10, 2025
Merged

fix selections not included in implicit#276604
justschen merged 1 commit intorelease/1.106from
justin/implicit-fix

Conversation

@justschen
Copy link
Collaborator

@justschen justschen commented Nov 10, 2025

release branch fix for #276599

(which just reverts #275432)

@justschen justschen marked this pull request as ready for review November 10, 2025 21:52
@vs-code-engineering
Copy link

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@bpasero

Matched files:

  • src/vs/workbench/contrib/chat/browser/chatInputPart.ts

@justschen justschen enabled auto-merge (squash) November 10, 2025 21:54
const contextArr = this.getAttachedContext(sessionResource);

if (this.implicitContext?.enabled && this.implicitContext?.value && this.configurationService.getValue<boolean>('chat.implicitContext.suggestedContext')) {
if ((this.implicitContext?.enabled && this.implicitContext?.value) || (this.implicitContext && !URI.isUri(this.implicitContext.value) && this.configurationService.getValue<boolean>('chat.implicitContext.suggestedContext'))) {
Copy link
Member

Choose a reason for hiding this comment

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

Is !isUri correct here, because it will then always include StringChatContextValue?

Copy link
Member

Choose a reason for hiding this comment

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

Side note, do we still need this setting?

Copy link
Collaborator Author

@justschen justschen Nov 10, 2025

Choose a reason for hiding this comment

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

i tried it out and i think it's fine, but yeah it's a revert from what it was before we added the new value type, so i'm not 100% sure

and yes, we probably do not need this setting anymore.

Copy link
Member

Choose a reason for hiding this comment

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

You tried it with a custom context provider?

@justschen justschen merged commit 507f50c into release/1.106 Nov 10, 2025
28 checks passed
@justschen justschen deleted the justin/implicit-fix branch November 10, 2025 22:22
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Dec 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants