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

feat: add Attributes keyword #48

Closed
wants to merge 1 commit into from

Conversation

shruti2522
Copy link
Contributor

@shruti2522 shruti2522 commented May 17, 2024

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

  • N
  • Y

2. What is the scope of this PR (e.g. component or file name):

vscode-kcl/syntaxes/KCL.tmLanguage.json

3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Other

Added Attributes keyword to kcl grammar to support syntax highlighting in hover.

4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):

  • N
  • Y

5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:

  • Unit test
  • Integration test
  • Benchmark (add benchmark stats below)
  • Manual test (add detailed scripts or steps below)
  • Other

Signed-off-by: shruti2522 <[email protected]>
@@ -28,7 +28,7 @@
"keywords": {
"patterns": [{
"name": "keyword.control.KCL",
"match": "\\b(as|assert|if|elif|else|lambda|for|import|schema|protocol|rule|mixin|check|and|in|is|not|or|all|any|map|filter|type)\\b"
"match": "\\b(as|assert|if|elif|else|lambda|for|import|schema|protocol|rule|mixin|check|and|in|is|not|or|all|any|map|filter|type|Attributes)\\b"
Copy link
Contributor

Choose a reason for hiding this comment

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

Attributes is not a keyword…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it as a keyword since the pretest mentioned :

pretest only needs to implement keyword highlighting, such as schema, Attribute, str, int

Copy link
Contributor

@Peefy Peefy May 18, 2024

Choose a reason for hiding this comment

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

I think the pretest expect a new better hover content with highlighting instead of adding a highlighting for the original content. cc @He1pa

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay @Peefy, got it, I will update the code accordingly and push the changes soon.

@Peefy Peefy closed this May 17, 2024
@He1pa
Copy link
Contributor

He1pa commented May 20, 2024

attribute is not a keyword. I just use it to differentiate Hover from other parts of the content. Even Attribute is not required in hover content. You can also render the hovered content into

main
---
doc
---
schema XX:
    name: str

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.

3 participants