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

Autocomplete: Don't trigger on closing characters #1218

Merged
merged 3 commits into from
Sep 28, 2023

Conversation

philipp-spiess
Copy link
Contributor

Test plan

Test added

@philipp-spiess philipp-spiess requested a review from a team September 28, 2023 00:09
@philipp-spiess philipp-spiess self-assigned this Sep 28, 2023
@philipp-spiess philipp-spiess changed the title Autocomplete: Don't trigger on closing tags Autocomplete: Don't trigger on closing characters Sep 28, 2023
@@ -183,6 +183,11 @@ async function doGetInlineCompletions(params: InlineCompletionsParams): Promise<
return null
}

// Do not trigger when the last character is a closing symbol
if (triggerKind !== TriggerKind.Manual && /[)\]}]$/.test(docContext.prefix.trim())) {
Copy link
Member

Choose a reason for hiding this comment

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

What if I'm in the middle of a function body and expect a single-line completion?

function whatever() {
  const data = {
    value: true
  }

  
}

There are multiple similar cases where I would like to see completions being generated. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair, let's not look at the prefix but the sameLinePrefix instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think looking at the current line only is a closer approximation to the cursor: "}" metric we were seeing anyways. I think right now this triggers pretty often because inserting a completion will trigger a new completion request

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Blissful

@philipp-spiess philipp-spiess merged commit f07680e into main Sep 28, 2023
11 checks passed
@philipp-spiess philipp-spiess deleted the ps/ac-dont-trigger-on-closing-tags branch September 28, 2023 01:44
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.

None yet

3 participants