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

Include C# textmate grammar #19469

Closed
chrisdias opened this issue Jan 27, 2017 · 5 comments
Closed

Include C# textmate grammar #19469

chrisdias opened this issue Jan 27, 2017 · 5 comments
Assignees
Labels
feature-request Request for new features or functionality plan-item VS Code - planned item for upcoming verified Verification succeeded
Milestone

Comments

@chrisdias
Copy link
Member

Request from C#/OmniSharp team, grammar is now maintained in an OSS repo:

https://github.com/dotnet/csharp-tmLanguage

@aeschli
Copy link
Contributor

aeschli commented Jan 27, 2017

So we should add a c# extension again?

@chrisdias
Copy link
Member Author

@aeschli I apologize that I don't know the exact workflow around textmate grammar files (see #10481 for proof :).

Is it possible to just pick up the .tmLanguage and not the entire C# extension? This way we get C# colorization out of the box as we do for other languages (looks like PHP, C++, etc.).

Going forward, the C# folks can maintain the their repo and make a PR to get it into VS Code.

@egamma
Copy link
Member

egamma commented Jan 30, 2017

Is it possible to just pick up the .tmLanguage and not the entire C# extension? This way we get C# colorization out of the box as we do for other languages (looks like PHP, C++, etc.).

@chrisdias this is definitely possible and what we currently do for Go, PHP, C++.

However, we can also go in the other direction and we can also start to move the Go tmGrammar to the Go extension, the C++ tmGrammar to the C++ extension. When we separated the OmniSharp extension we intentionally did this, so that C# related artifacts can be maintained in one place and in one issue repository.

Revisiting this decision is fine, but we should then have some principles for which languages we bundle a tmGrammar with VS Code. What we must avoid is that there is grammar in both VS Code and in the language extension. This has happened for the F# extension and now a grammar is maintained twice...

@chrisdias
Copy link
Member Author

@egamma ok, i see the problem, thanks for the explanation.

From a customer perspective, having syntax colorization out of the box is goodness. From an engineering perspective it is a pain to maintain these in multiple places.

I like the model where the C# grammar is in a single repo that both VS Code and the OmniSharp extension ship:

  • the grammar is maintained in a single location
  • the grammar is owned by the language owner, not VS Code
  • the language owner (or maintainer of the grammar) can make PRs on VS Code to update
  • if the VS Code version is out of date, it will be updated when the extension is installed (I assume)
  • issues submitted in the VS Code repo can be ported to the grammar repo

Thoughts?

@egamma
Copy link
Member

egamma commented Feb 1, 2017

@chrisdias see below for comments

From a customer perspective, having syntax colorization out of the box is goodness. From an engineering perspective it is a pain to maintain these in multiple places.

This is a great summary. However, since we now provide extension recommendations, having syntax colorization out of the box is less important. A user will almost always want to use the rich language extension and not just have coloring that comes with VS Code.

the grammar is maintained in a single location
the grammar is owned by the language owner, not VS Code

This is already the common practice and we should continue to use it, Since the grammars can be used in multiple editors. I must be maintained in a separate repo, e.g., the TypeScript grammar is maintained in https://github.com/Microsoft/TypeScript-TmLanguage.

the language owner (or maintainer of the grammar) can make PRs on VS Code to update

This assumes that then VS Code becomes the integrator of grammars. This makes sense for some key languages and can include C#, but it doesn't make sense to me as a general rule. VS Code has extensions.

if the VS Code version is out of date, it will be updated when the extension is installed (I assume)

This assumes that the grammar is also included in the extension? The version in the extension will win over the version that is bundled with VS Code so this works. However, this means that the grammar is maintained in both VS Code and the extension. This is done in F# today and has resulted in the F# grammar in VS Code being out of date. The F# extension writers only update the grammar in their extension they don't care updating the grammar bundled with VS Code using a PR.

Bottom line we can do this for C#, but not as a general approach.

@kieferrm kieferrm mentioned this issue Feb 6, 2017
38 tasks
@aeschli aeschli added this to the February 2017 milestone Feb 7, 2017
@aeschli aeschli added feature-request Request for new features or functionality plan-item VS Code - planned item for upcoming labels Feb 7, 2017
@aeschli aeschli closed this as completed in 0fa326c Feb 8, 2017
@rebornix rebornix added the verification-needed Verification of issue is requested label Feb 21, 2017
@jrieken jrieken added the verified Verification succeeded label Feb 22, 2017
@mjbvz mjbvz removed the verification-needed Verification of issue is requested label Feb 24, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality plan-item VS Code - planned item for upcoming verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants