Merging the regex work into master.#29580
Conversation
Merging master into Features/embedded regex
…e in a string in a directive node.
Add Regex Language Service features to the IDE.
|
Tagging @dpoeschl @jinujoseph @jcouv |
|
test WIP |
dpoeschl
left a comment
There was a problem hiding this comment.
✔️ except handling the public api & legal things.
| Microsoft.CodeAnalysis.CodeStyle.NotificationOption.Severity.set -> void | ||
| abstract Microsoft.CodeAnalysis.Editing.SyntaxGenerator.AliasImportDeclaration(string aliasIdentifierName, Microsoft.CodeAnalysis.SyntaxNode name) -> Microsoft.CodeAnalysis.SyntaxNode | ||
| abstract Microsoft.CodeAnalysis.Editing.SyntaxGenerator.NameExpression(Microsoft.CodeAnalysis.INamespaceOrTypeSymbol namespaceOrTypeSymbol) -> Microsoft.CodeAnalysis.SyntaxNode | ||
| const Microsoft.CodeAnalysis.Classification.ClassificationTypeNames.RegexAlternation = "regex - alternation" -> string |
There was a problem hiding this comment.
This & licensing are my only concerns about going into master. @jinujoseph Who should we talk to about the public api change?
There was a problem hiding this comment.
Note: this public api change is just keeping in line with how we've done things around classification. Namely that we export our classification type names as public constants. Presumably this is so other extensions can know aobut these names and interact with them if they want (for example, they can prioritize their own classifications above/below these ones).
There was a problem hiding this comment.
Assuming that these classifications would be returned by normal calls to the Classifier.GetClassifiedSpansAsync(...) outside of VS, these need to be public so that editors can handle them appropriately. I fully support adding them to the public API surface.
|
Re: Public API -- We can always switch them back to internal to get this into master and then create a separate PR/discussion for the Public API approval if needed. EDIT: Nevermind, it got approved. |
|
As of this comment, the licensing change isn't merged yet (it's running tests) but I'm making a note here that all test runs have passed without that change in case we want to later justify merging in the face of unrelated integration failures. ✔ |
RegEx Licensing
|
Yaay! |
Followup to #25517 once all the licensing stuff is merged into features/embeddedRegex.