-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Add .slnx
to XML extensions
#7084
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
cc @visose fyi, i've used your slnx file as a sample for syntax highlighting support. https://github.com/visose/Robots/blob/aa2be4ba40947b950b79d68fae3e22f32a0184c2/Robots.slnx |
fyi @rainersigwald @davkean (dotnet/msbuild#1730 (comment) it was simpler than i thought 😉) |
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.
Ah yeah, getting the default XML highlighting is a great low-cost way to do this!
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.
Your sample should be in the samples/XML
directory.
@lildude ok, but can you tell me why? there is an existing .sln, i'm adding .slnx next to it https://github.com/github-linguist/linguist/tree/main/samples/Microsoft%20Visual%20Studio%20Solution |
Sure... it's what you want 😉 As you're adding the extension to the XML language entry, you're saying these files are XML and should be highlighted as XML and as such, the sample needs to go into the XML language directory. This is why the tests were failing. |
Ah, I didn't realize that the samples are structured per language, and "Microsoft Visual Studio Solution" is treated as a language. The new format, however, is based on the XML language. :) |
You need to add this to the list of no-root-tag entries in the test too: linguist/test/test_strategies.rb Lines 176 to 194 in f164d13
This will exclude the sample from the XML strategy testing which is what is failing here (we're not using the strategy to detect the language). |
All good now. I'll review usage again when I start preparing the next release.
@lildude the search indicator is only going to improve significantly until dotnet/sdk#40913 is resolved (support for slnx in dotnet sdk which is scheduled for .NET 9 release 2 which usually happen at the end of Feb). Rest assured, it is happening; the whole community was waiting for new solution format for a very long time dotnet/msbuild#1730 and VS has just added its support (as a preview feature). |
Hello @lildude just wondering if you and your team had had the chance of taking a look into this. .NET 9.0.200 has been out now and the use of .slnx files has increased in popularity |
No. I only review pending popularity PRs when I make a new release which happens approximately every 3-4 months. |
@edvilme i hope visual studio will take this out of preview so more people can join. today everyone in a team has to enable preview feature to even get slnx open the solution in vs which i think is holding back the masses |
numbers are steadily growing. what's the popularity criteria of adding an extension to existing language, with no conflict? is it a fixed value? |
VS 2022 17.14 has taken it out of preview. |
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.
LGTM. Thanks.
Important
The changes in this PR will not appear on GitHub until the next release has been made and deployed. See here for more details.
Description
Checklist:
Fix #7083