-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Update document highlighting to follow new pattern for embedded language extensibility. #61817
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
Conversation
| internal sealed class CSharpJsonEmbeddedLanguageBraceMatcher : | ||
| AbstractJsonEmbeddedLanguageBraceMatcher | ||
| PredefinedEmbeddedLanguageNames.Json, LanguageNames.CSharp, supportsUnannotatedAPIs: true, "Json"), Shared] | ||
| internal sealed class CSharpJsonBraceMatcher : AbstractJsonBraceMatcher |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of having names for each embeded-language/feature-name pair, we just have the names of hte embedded languages once.
| [ImportingConstructor] | ||
| [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] | ||
| public CSharpJsonEmbeddedLanguageBraceMatcher() | ||
| public CSharpJsonBraceMatcher() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to a simpler and more consistent naming pattern. Instead of repeating 'EmbeddedLanguage' in teh name, we just state the name of the embedded language. Regex/Json are already clearly embedded langauges, so this cut down on verbosity everywhere.
| : base(name, language, identifiers) | ||
| { | ||
| SupportsUnannotatedAPIs = supportsUnannotatedAPIs; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at this point, this pattern is repeated 3 times. strongly thinking of just converting it to two classes which take in the typeof(IEmbeddedLanguageDocumentHighlightsService) etc. data as a parameter at teh use site.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #61853 to simplify this.
| src\Analyzers\VisualBasic\CodeFixes\VisualBasicCodeFixes.projitems*{0141285d-8f6c-42c7-baf3-3c0ccd61c716}*SharedItemsImports = 5 | ||
| src\Workspaces\SharedUtilitiesAndExtensions\Workspace\VisualBasic\VisualBasicWorkspaceExtensions.projitems*{0141285d-8f6c-42c7-baf3-3c0ccd61c716}*SharedItemsImports = 5 | ||
| src\Compilers\CSharp\CommandLine\CscCommandLine.projitems*{0161e25c-918a-4dc8-9648-30fdcc8e31e9}*SharedItemsImports = 5 | ||
| src\Compilers\CSharp\csc\CscCommandLine.projitems*{0161e25c-918a-4dc8-9648-30fdcc8e31e9}*SharedItemsImports = 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaredpar not sure what's going on here. every time i undo this, vs changes it back.
This is part 1 of 2. The second part will provide the hooks to asp.net to use this.