fix parse() throws error on valid ReflectOnly Blink extended attributes #443
Conversation
Signed-off-by: tushar goel <[email protected]>
|
@saschanaz please have a look on it 😀 |
There was a problem hiding this comment.
Thanks for the great work again, I think this is in the right path!
A suggestion though: I would move the new functions to ``extended-attributes.jsbecause currentlytokens` and `identifiersOrStrings` are only used within there. A principle: Keep them where they are used 😁
identifiers should be also moved to extended-attributes.js per the same principle.
lib/productions/helpers.js
Outdated
| * Returns a proxy that auto-assign `parent` field. | ||
| * @template T | ||
| * @param {T} tokeniser | ||
| * @param {*} [tokenName] either can be a string or identifier | ||
| * @return {T} |
There was a problem hiding this comment.
Hmm, the prose doesn't exactly describes what tokens() is... Should probably say this consumes identifiers and strings.
For the JSDoc, tokeniser should be import("../tokeniser").Tokeniser instead of T and tokenName should be string instead of *. I think we don't need @return here because it will be autodetected by TypeScript language service.
|
How can I merge the changes that you done in gh-pages @saschanaz |
That worked! Though the new (expected) error:
|
Signed-off-by: tushar goel <[email protected]>
Signed-off-by: tushar goel <[email protected]>
| @@ -43,9 +42,7 @@ export function list(tokeniser, { parser, allowDangler, listName = "list" }) { | |||
| return items; | |||
| } | |||
|
|
|||
| /** | |||
| * @param {import("../tokeniser").Tokeniser} tokeniser | |||
| */ | |||
There was a problem hiding this comment.
Oops, this one is still needed!
| /** | ||
| * @param {import("../tokeniser").Tokeniser} tokeniser | ||
| */ | ||
| function identifiers(tokeniser) { |
There was a problem hiding this comment.
Oops, forgot that we are replacing identifiers with identifiersOrStrings. That means it's safe to remove this!
|
Closing in favor of #445 |
Signed-off-by: tushar goel [email protected]
This patch closes #256 and includes: