-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Use square brackets for optional properties in the JSDoc comments of src/display/api.js
#11209
Use square brackets for optional properties in the JSDoc comments of src/display/api.js
#11209
Conversation
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.
Please see https://github.com/mozilla/pdf.js/wiki/Squashing-Commits, and it would be a good idea to update the commit message to actually include JSDoc (since otherwise it's really impossible to understand what the patch does without looking at the code).
src/display/api.js
Outdated
* file data. When range requests are enabled PDF.js will automatically keep | ||
* fetching more data even if it isn't needed to display the current page. | ||
* @property {boolean} [disableStream] - (optional) Disable streaming of PDF | ||
* file data. By default PDF.js attempts to load PDFs in chunks. |
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.
Please don't remove the The default value is `false`.
line 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.
I think the line is not removed.
baba97a
to
b812c8c
Compare
The commits are squashed, and the word |
b812c8c
to
d1e75ac
Compare
src/display/api.js
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/c86997eac19cc98/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/c86997eac19cc98/output.txt Total script time: 1.69 mins Published |
Here is how this renders now: http://54.67.70.0:8877/c86997eac19cc98/api/draft/global.html#DocumentInitParameters I agree that the double optional bit is not necessary anymore, given that |
This is obviously a pre-existing issue, however: |
Yes, all properties should be optional as far as I can tell. If that and #11209 (comment) are done, this patch looks good to me. |
…of src/display/api.js
d1e75ac
to
d5ee083
Compare
All the properties of DocumentInitParameters are marked as optional.
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/9c13641ada151c0/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/9c13641ada151c0/output.txt Total script time: 1.66 mins Published |
Nice work; thank you for improving the documentation! |
use square brackets to indicate properties are optional in JSDoc. Related to #10575. Without the square brackets, that PR would generate an incorrect
d.ts
file.The diff becomes lengthy due to the restriction of maximal line length. If I can remove the word
(optional)
, it becomes much simpler.