-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Conversation
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. |
Now, I have signed the Contributor License Agreement (CLA) at https://cla.developers.google.com/ |
Yes, I have now made the necessary changes. |
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.
Thanks, these look like nice improvements!
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Ok, I have made those changes as well, please suggest if any other changes are required. |
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.
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!
// 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. |
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.
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.
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.
Yes, license and copyright header to carbon.tmLanguage.json
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>
I have also changed the formatting of all the files as per the CarbonInfraBot. |
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.
Thank you! Just a couple more things and I think this is ready to merge.
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
ok, I think all the changes have been made, Please suggest if any other changes are required. |
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.
Thanks again, this looks great!
…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>
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: