Use cpptools API v5 by michelleangela · Pull Request #6823 · microsoft/vscode-cpptools · GitHub
Skip to content

Use cpptools API v5#6823

Merged
michelleangela merged 6 commits into
mainfrom
mimatias/use-api-5
Jan 21, 2021
Merged

Use cpptools API v5#6823
michelleangela merged 6 commits into
mainfrom
mimatias/use-api-5

Conversation

@michelleangela

Copy link
Copy Markdown
Contributor

No description provided.

@Colengms Colengms left a comment

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 had made some changes to the API, to these fields (to make them optional. microsoft/vscode-cpptools-api#28 ) :

SourceFileConfiguration 

	intelliSenseMode
	standard

WorkspaceBrowseConfiguration 

	standard

They now accept undefined. But, we were waiting until we ingested the new API to make the corresponding changes to cpptools. Could you update isSourceFileConfigurationItem() to use isOptionalString() instead of isString() when validating? Perhaps consider it an error if either of these are undefined when the compilerPath is also undefined. We need compilePath to be present to auto-detect these.

Similarly for WorkspaceBrowseConfiguration in sendCustomBrowseConfiguration(). That is already using isOptionalString for standard, but should probably flag an error if it's undefined and compilerPath is also undefined.

@sean-mcmanus

Copy link
Copy Markdown
Contributor

Comment thread Extension/src/LanguageServer/client.ts Outdated
@michelleangela

Copy link
Copy Markdown
Contributor Author

@michelleangela michelleangela merged commit f73a5d1 into main Jan 21, 2021
@michelleangela michelleangela deleted the mimatias/use-api-5 branch January 29, 2021 01:16
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 8, 2021
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