Add more comments #1993
Add more comments #1993
Conversation
|
Thanks for the PR! This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged. |
|
|
Sure |
|
Hello @saschanaz |
| @@ -13,10 +13,7 @@ interface URLSearchParamsIterator<T> extends IteratorObject<T, BuiltinIteratorRe | |||
|
|
|||
| interface URLSearchParams { | |||
| [Symbol.iterator](): URLSearchParamsIterator<[string, string]>; | |||
| /** Returns an array of key, value pairs for every entry in the search params. */ | |||
There was a problem hiding this comment.
This because the iterators don't have mdn links. So I have fixed it by adding the comments json back just for the iterators.
src/build/emitter.ts
Outdated
| @@ -906,7 +907,10 @@ export function emitWebIdl( | |||
| comments.push("Available only in secure contexts."); | |||
| } | |||
| if (entity.mdnUrl) { | |||
| if (comments.length > 0) comments.push(""); | |||
| if (comments.length == 0) { | |||
| comments.push(extractSummaryFromFile(entity.mdnUrl)); | |||
There was a problem hiding this comment.
Emitter should just emit the typescript lib based on the given data, rather than generating data. Any required generation should be done in build.ts and passed to this function.
src/build/mdn-comments.ts
Outdated
| const relativePath = url | ||
| .replace("https://developer.mozilla.org/docs/", "") | ||
| .split("#")[0] | ||
| .toLowerCase(); |
There was a problem hiding this comment.
We should compute the path rather than using the URL this way. Basically for Foo.bar the path should be /api/foo/bar/index.md.
And why do we need this when we already have generateDescriptions above that collects index files?
|
I have updated the context to not call the API in the emitter, but that cause the test to fail, I will fix it later |
|
Done @saschanaz |
| if (comments.length == 0) { | ||
| const key = entity.mdnUrl | ||
| .split("/API/") | ||
| .pop() | ||
| ?.replace("/", ".") | ||
| .split("#")[0] | ||
| .toLowerCase(); | ||
| if (key && descriptions[key]) { | ||
| comments.push(descriptions[key]); | ||
| } | ||
| } | ||
| comments.push(""); |
There was a problem hiding this comment.
This kind of processing should also be done outside emitter, namely here:
TypeScript-DOM-lib-generator/src/build.ts
Lines 130 to 146 in 7ca5f39
| ) { | ||
| const exposed = getExposedTypes(webidl, options.global, forceKnownTypes); | ||
| mergeNamesakes(exposed); | ||
| exposed.events = webidl.events; | ||
|
|
||
| const result = emitWebIdl( | ||
| const result = await emitWebIdl( |
There was a problem hiding this comment.
emitWebidl is not async anymore.
| console.error("Error reading directories:", error); | ||
| return []; | ||
| } | ||
| return sentenceMatch ? sentenceMatch[0] : normalizedText.split(" ")[0] || ""; |
There was a problem hiding this comment.
This is doing too much in one line. Multiple lines are often easier to read.
| try { | ||
| const content = await fs.readFile(fileURL, "utf-8"); | ||
|
|
||
| const titleMatch = content.match(/title:\s*["']?([^"'\n]+)["']?/); |
There was a problem hiding this comment.
Maybe keep this processing done in a separate function?
related to #1940
Hello:)
In my last PR I have used MDN to generate comments for interface declarations, in this pr I add it for the properties.