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

Syntax highlighter in VScode for carbon programming language #3953

Merged
merged 39 commits into from
May 21, 2024

Conversation

RohanVashisht1234
Copy link
Contributor

@RohanVashisht1234 RohanVashisht1234 commented May 16, 2024

I have made the necessary changes to https://github.com/carbon-language/carbon-lang/tree/trunk/utils/vscode for the VScode syntax highlighter to work:

  • Changes have been tested and found to be working.

Copy link

google-cla bot commented May 16, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot requested a review from jonmeow May 16, 2024 22:42
@RohanVashisht1234
Copy link
Contributor Author

Now, I have signed the Contributor License Agreement (CLA) at https://cla.developers.google.com/

utils/vscode/package.json Outdated Show resolved Hide resolved
utils/vscode/package.json Outdated Show resolved Hide resolved
@RohanVashisht1234
Copy link
Contributor Author

If there's some way we can merge these two grammars together, that'd be great.

Yes, I have now made the necessary changes.

Copy link
Contributor

@zygoloid zygoloid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, these look like nice improvements!

utils/vscode/syntaxes/carbon.tmLanguage.json Outdated Show resolved Hide resolved
utils/vscode/syntaxes/carbon.tmLanguage.json Outdated Show resolved Hide resolved
utils/vscode/syntaxes/carbon.tmLanguage.json Outdated Show resolved Hide resolved
utils/vscode/syntaxes/carbon.tmLanguage.json Outdated Show resolved Hide resolved
utils/vscode/syntaxes/carbon.tmLanguage.json Outdated Show resolved Hide resolved
utils/vscode/language-configuration.json Outdated Show resolved Hide resolved
utils/vscode/language-configuration.json Outdated Show resolved Hide resolved
utils/vscode/language-configuration.json Outdated Show resolved Hide resolved
utils/vscode/language-configuration.json Outdated Show resolved Hide resolved
utils/vscode/package.json Outdated Show resolved Hide resolved
RohanVashisht1234 and others added 3 commits May 18, 2024 12:18
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
@RohanVashisht1234
Copy link
Contributor Author

Ok, I have made those changes as well, please suggest if any other changes are required.

utils/textmate/Syntaxes/carbon.tmLanguage.json Outdated Show resolved Hide resolved
utils/textmate/Syntaxes/carbon.tmLanguage.json Outdated Show resolved Hide resolved
utils/vscode/language-configuration.json Outdated Show resolved Hide resolved
utils/vscode/language-configuration.json Outdated Show resolved Hide resolved
utils/vscode/language-configuration.json Outdated Show resolved Hide resolved
utils/vscode/language-configuration.json Outdated Show resolved Hide resolved
utils/vscode/language-configuration.json Outdated Show resolved Hide resolved
utils/vscode/package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will leave most of this review to @zygoloid but wanted to quickly follow up that I enabled running our pre-commit checks and they flagged some formatting issues. You can accept the suggested edits from our bot, and you can also replicate the formatting we use with prettier or by running pre-commit run -a locally. The tools here are documented in our contribution tools page: https://github.com/carbon-language/carbon-lang/blob/trunk/docs/project/contribution_tools.md

Thanks again for working on our VSCode integration! It's really awesome to see!

Comment on lines 1 to 3
// Part of the Carbon Language project, under the Apache License v2.0 with LLVM.
// Exceptions. See /LICENSE for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should try to keep as many license headers as we can, and where we can we should keep them formatted canonically.

Sadly, our license and copyright header checking was skipping all JSON files. I've sent #3959 to fix this and exclude the two JSON files that seem to need to be strict (no comments allow) JSON: package.json and package-lock.json.

Would be good to revert these changes, and add the license header to the carbon.tmLanguage.json file as well I suspect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, license and copyright header to carbon.tmLanguage.json

RohanVashisht1234 and others added 7 commits May 19, 2024 14:21
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
@RohanVashisht1234
Copy link
Contributor Author

RohanVashisht1234 commented May 19, 2024

I have also changed the formatting of all the files as per the CarbonInfraBot.
Please suggest if any other changes are required.

Copy link
Contributor

@zygoloid zygoloid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Just a couple more things and I think this is ready to merge.

utils/textmate/Syntaxes/carbon.tmLanguage.json Outdated Show resolved Hide resolved
utils/vscode/language-configuration.json Outdated Show resolved Hide resolved
utils/textmate/Syntaxes/carbon.tmLanguage.json Outdated Show resolved Hide resolved
RohanVashisht1234 and others added 2 commits May 21, 2024 00:34
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
@RohanVashisht1234
Copy link
Contributor Author

ok, I think all the changes have been made, Please suggest if any other changes are required.

Copy link
Contributor

@zygoloid zygoloid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again, this looks great!

@zygoloid zygoloid enabled auto-merge May 20, 2024 21:22
@zygoloid zygoloid added this pull request to the merge queue May 21, 2024
Merged via the queue into carbon-language:trunk with commit 5c5a687 May 21, 2024
7 checks passed
CJ-Johnson pushed a commit to CJ-Johnson/carbon-lang that referenced this pull request May 23, 2024
…language#3953)

I have made the necessary changes to
https://github.com/carbon-language/carbon-lang/tree/trunk/utils/vscode
for the VScode syntax highlighter to work:

- [x] Changes have been tested and found to be working.

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
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.

4 participants