-
-
Notifications
You must be signed in to change notification settings - Fork 93
chore: improve interpolation documentation by making it an syntax-enum
#1400
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
base: main
Are you sure you want to change the base?
Conversation
|
Size Change: +346 B (+0.26%) Total Size: 135 kB
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1400 +/- ##
=======================================
Coverage 93.73% 93.73%
=======================================
Files 111 111
Lines 4626 4626
Branches 1557 1557
=======================================
Hits 4336 4336
Misses 290 290 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
| }); | ||
| } | ||
| if (parameter.type === 'interpolation') { |
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.
Is there a different way to achieve this? I prefer to be as generic as possible 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.
The other "length1-array with syntax-enum as value" is expressions.
Expressions are manually codegen-ed, so there is no code to reuse.
Also, the indentation and newline requirement (those two spaces before \n) would be a bit tricky to do this way.
Unless you want a major change to how this system works (which might make sense, but would be a larger change)
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.
In saying the same thing in both comments basically: is there a more generalized way to achieve the same thing? Or maybe reuse some code?
At the very least, since this is specific, it should be extracted to a method.
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.
Generalised? Not without basically rewriting the entire thing.
This is one of the core parts and the impact of this is likely larger.
I don't think that reusing code is possible.
I moved the two things you commented on to a function each.
|
I think the added information is good, this is a very complicated spec definition, so more information is better. |
I don't fully follow what you are trying to communicate 😅 |
| const type = parameterTypeToType(parameter.type) | ||
| .replaceAll('<', '<') | ||
| .replaceAll('>', '>'); | ||
| const type = parameterTypeToType(parameter.type); |
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.
interpolation documentation by making it an syntax-enum

This PR documents how interpolations work in a more structured way.
I am not sure if my docs are too verbose, I tried to be fairly precisce since cutting is mostly simpler.