-
Notifications
You must be signed in to change notification settings - Fork 636
TypeSpec: new parser #4243
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
TypeSpec: new parser #4243
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4243 +/- ##
==========================================
+ Coverage 85.89% 85.95% +0.05%
==========================================
Files 245 246 +1
Lines 63149 63459 +310
==========================================
+ Hits 54240 54544 +304
- Misses 8909 8915 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Well written. Thank you.
Could you add the name of the new parser to docs/news/HEAD.rst?
I think we can simplify the code with "cork API",
See https://docs.ctags.io/en/latest/internal.html#cork-api
I will show you how to apply the CORK API to your parser in the next comment.
Let's use CORK API. |
@doc("API Versions") | ||
enum Versions { | ||
@doc("May 01, 2024 Preview API Version") | ||
v2024_05_01_preview: "2024-05-01-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.
Shouldn't we extract v2024_05_01_preview
?
NYY-choice.
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, I have thinked about this. I can do in another PR. BTW, @masatake , could you suggest TypeSpecKinds for this type? I don't find much examples of such kind.
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.
As far as reading https://typespec.io/docs/language-basics/enums/, the enum in TypeSpec is not so different from that in C. So we can reuse what the C parser defines.
$ ./ctags --list-kinds-full=C | grep enum
e enumerator yes no 0 C enumerators (values inside an enumeration)
g enum yes no 0 C enumeration names
e,enumerator,enumerators (values inside an enumeration)
looks suitable here.
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 refer to TypeScript instead of C.
$ ./ctags --list-kinds-full=TypeScript | grep enum
e enumerator yes no 0 NONE enumerators (values inside an enumeration)
g enum yes no 0 NONE enums
TypeScript also uses g
.
import "./common.tsp"; | ||
|
||
using TypeSpec; | ||
using TypeSpec.Rest; |
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.
ModelB<ModelA[]> | ||
>; | ||
|
||
interface InterfaceA extends InterfaceB { |
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 can fill inherits:
field of interfaceA:
with InterfaceB
.
NYY-choice.
Filling such a field makes your work applicable to broader use cases.
Here is an example of C++.
$ cat /tmp/foo.cc
class A: B {
int x;
};
$ ./ctags -o - /tmp/foo.cc
A /tmp/foo.cc /^class A: B {$/;" c file:
x /tmp/foo.cc /^ int x;$/;" m class:A typeref:typename:int file:
$ ./ctags --fields=+i -o - /tmp/foo.cc
A /tmp/foo.cc /^class A: B {$/;" c file: inherits:B
x /tmp/foo.cc /^ int x;$/;" m class:A typeref:typename:int file:
$ ./ctags --fields=+i -o foo.tags /tmp/foo.cc
$ ~/bin/querytags --tag-file foo.tags class-hierarchy A
A
B
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, I can do it in another PR
{ true, 'u', "union", "unions" }, | ||
{ true, 'a', "alias", "aliases" }, | ||
{ true, 'p', "property", "properties" } | ||
}; |
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.
My reviewing log: These are good. They align with the kinds of TypeScript.
Thank you for updating. Could you add the name of the new parser to docs/news/HEAD.rst? |
a234d44
to
a155321
Compare
Thank you very much. |
Add parser for TypeSpec
Test passed with
make units LANGUAGES=TypeSpec