Skip to content

Break out embedded language classification entirely from normal classification#59896

Merged
CyrusNajmabadi merged 4 commits intodotnet:release/dev17.3from
CyrusNajmabadi:embeddedClassification2
Mar 4, 2022
Merged

Break out embedded language classification entirely from normal classification#59896
CyrusNajmabadi merged 4 commits intodotnet:release/dev17.3from
CyrusNajmabadi:embeddedClassification2

Conversation

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

@CyrusNajmabadi CyrusNajmabadi commented Mar 2, 2022

Continued part of the effort to expose an extension point for 3rd parties to classify strings containing embedded DSLs.

There will be a followup to this to allow embedded language classifiers to be found and used using MEF.

@CyrusNajmabadi CyrusNajmabadi requested review from a team as code owners March 2, 2022 23:19
@CyrusNajmabadi CyrusNajmabadi requested a review from a team March 2, 2022 23:19
@ghost ghost added the Area-IDE label Mar 2, 2022
@CyrusNajmabadi CyrusNajmabadi changed the base branch from main to release/dev17.3 March 2, 2022 23:19

public CancellationToken CancellationToken { get; }

internal EmbeddedLanguageClassifierContext(
Copy link
Copy Markdown
Member

@genlu genlu Mar 3, 2022

Choose a reason for hiding this comment

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

Why internal members? Is this going to be promoted to public APIs? If so, there's some missing doc comments on public surface

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It will be once we have confidence through a third party :-)

_syntaxTokenKinds.Add(syntaxKinds.StringLiteralToken);
_syntaxTokenKinds.Add(syntaxKinds.InterpolatedStringTextToken);

if (syntaxKinds.SingleLineRawStringLiteralToken != null)
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.

Does the Fallback classifier need similar checks too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It does already have checks built in.

@CyrusNajmabadi CyrusNajmabadi enabled auto-merge March 3, 2022 21:32
@CyrusNajmabadi CyrusNajmabadi merged commit f13576a into dotnet:release/dev17.3 Mar 4, 2022
@CyrusNajmabadi CyrusNajmabadi deleted the embeddedClassification2 branch May 3, 2022 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants