Skip to content

For getCompletionsAtPosition, require a flag to provide completions with code actions#19687

Merged
7 commits merged into
masterfrom
completions_flag
Nov 3, 2017
Merged

For getCompletionsAtPosition, require a flag to provide completions with code actions#19687
7 commits merged into
masterfrom
completions_flag

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Nov 2, 2017

No description provided.

@ghost ghost requested review from amcasey, mhegazy and mjbvz November 2, 2017 19:44
Comment thread src/server/protocol.ts Outdated
* Optional prefix to apply to possible completions.
*/
prefix?: string;
includeCompletionsWithActions: boolean;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i would rather we be more specific about what the feature does today, and make it includeNonImportedDeclarations or something along these lines.

Also please add a comment explaining what it does

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

How about includeExternalModuleExports?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Nov 2, 2017

since this is a breaking change, we should rev the API version.

@mhegazy mhegazy added Breaking Change Would introduce errors in existing code Domain: API Relates to the public API for TypeScript labels Nov 2, 2017
@mhegazy mhegazy added this to the TypeScript 2.6.2 milestone Nov 2, 2017
@ghost
Copy link
Copy Markdown
Author

ghost commented Nov 2, 2017

@mhegazy Good to go?

@mjbvz
Copy link
Copy Markdown
Contributor

mjbvz commented Nov 2, 2017

Will add this flag to vs code for 1.18

mjbvz added a commit to microsoft/vscode that referenced this pull request Nov 2, 2017
This new flag will make auto import completions opt-in for TS 2.6.2. Adding it for furture proofing

microsoft/TypeScript#19687
Comment thread src/server/protocol.ts
* Optional prefix to apply to possible completions.
*/
prefix?: string;
includeExternalModuleExports: boolean;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please add a comment

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Nov 2, 2017

@amcasey can you take this one for a spin as well.

@ghost
Copy link
Copy Markdown
Author

ghost commented Nov 3, 2017

Waiting for approval from @amcasey

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Nov 3, 2017

Please port to release-2.6 as well.

@ghost ghost merged commit bb7fb7d into master Nov 3, 2017
@ghost ghost deleted the completions_flag branch November 3, 2017 22:55
ghost pushed a commit that referenced this pull request Nov 3, 2017
…ith code actions (#19687)

* For getCompletionsAtPosition, require a flag to provide completions with code actions

* Change name

* Increase API version

* Update API baselines

* Add comment

* Update API baseline
ghost pushed a commit that referenced this pull request Nov 3, 2017
…ith code actions (#19687) (#19731)

* For getCompletionsAtPosition, require a flag to provide completions with code actions

* Change name

* Increase API version

* Update API baselines

* Add comment

* Update API baseline
Comment thread src/services/types.ts
}

export interface GetCompletionsAtPositionOptions {
includeExternalModuleExports: boolean;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This name suggests we're going to have a separate opt-in for each kind of completion that might return an action. Is that the plan?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I could not think of another feature that we would add, and thus did not have a frame of reference. We also got feedback form the VSCode team that they would like to be able to disable the feature altogether, and incidentally from customer reports later on.

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Breaking Change Would introduce errors in existing code Domain: API Relates to the public API for TypeScript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants