Skip to content
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

XML classifier #40

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

KirillOsenkov
Copy link
Contributor

Add a parser-based XML classifier and tests.

We need to think about maybe having it off by default because if this is hosted in VS, VS already has its own XML classifier, so not sure whether they will conflict.

VSMac uses this classifier already.

@mhutch
Copy link
Owner

mhutch commented Apr 21, 2021

This is probably fine on VSWin - the XML classifier isn't active in my MSBuild editor, for example - I explicitly opted into the TextMate tagger to get highlighting.

I would like to figure out a way to make this opt-in/extensible for derived languages. I have no idea how it would interact with TextMate tagger that the MSBuild editor uses, for example, so at the very least I'd want to be able to disable it. Ideally I'd want to be able to extend it to add MSBuild expression highlighting.

Add a parser-based XML classifier and tests.

We need to think about maybe having it off by default because if this is hosted in VS, VS already has its own XML classifier, so not sure whether they will conflict.

VSMac uses this classifier already.
An extension has a way to disable the XML classifier entirely or provide custom classifications to replace default classification spans.
@KirillOsenkov
Copy link
Contributor Author

I added IXmlClassifierExtension, let me know what you think.

I imagine MSBuildEditor would provide one.

@KirillOsenkov
Copy link
Contributor Author

@mhutch do you have an MSBuild-specific classifier anywhere so we could test the extension point? I could probably write something simple that classifies contents of some attribute values as a simple test scenario.

@mhutch
Copy link
Owner

mhutch commented Nov 29, 2021

I don't have an MSBuild classifier, I just use the TextMate format - see https://github.com/mhutch/MonoDevelop.MSBuildEditor/blob/main/MonoDevelop.MSBuildEditor/Syntax/msbuild.json

I think the best approach here is probably to not bind to [ContentType (XmlContentTypeNames.XmlCore)] but instead bind to the more derived content types and make the classifier non-sealed so other XML-derived languages can subclass it when implementing a classifier?

@KirillOsenkov
Copy link
Contributor Author

OK, I unsealed the XmlClassifier class and made some members virtual.

Changed the classifier provider to export for Xml, Xslt and Xsd instead of XmlCore.

If you want, we could remove the XmlClassifierProvider altogether and have the host export it to give full control to the host.

Another problem is that XmlClassificationTypes is an enum, so not extensible. We could change to string constants I guess? Or ints?

Feel free to take over this PR and push directly to it if you like.

@mhutch mhutch self-assigned this Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants