-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Improve how the PDF.js version/commit information is exposed in the *built* files #19956
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
Merged
+14
−59
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6bbe81a
to
2adee93
Compare
…built* files To make it easier to tell which PDF.js version/commit that the *built* files correspond to, they have (since many years) included `pdfjsVersion` and `pdfjsBuild` constants with that information. As currently implemented this has a few shortcomings: - It requires manually adding the code, with its preprocessor statements, in all relevant files. - It requires ESLint disable statements, since it's obviously unused code. - Being unused, this code is removed in the minified builds. - This information would be more appropriate as comments, however Babel discards all comments during building. - It would be helpful to have this information at the top of the *built* files, however it's being moved during building. To address all of these issues, we'll instead utilize Webpack to insert the version/commit information as a comment placed just after the license header.
2adee93
to
97a4248
Compare
/botio-linux test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/236d97279486376/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/236d97279486376/output.txt Total script time: 30.09 mins
|
calixteman
approved these changes
May 22, 2025
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.
LGTM. Thank you.
toofar
added a commit
to qutebrowser/qutebrowser
that referenced
this pull request
Jun 2, 2025
It's changed in this PR: mozilla/pdf.js#19956 to have the version string as a comment in the file, instead of the variable. Making the regex more forgiving increases the chance of matching on the wrong string on a past or future version. I've only tested with the previous version (v5.2.133) and it seemed to still work there. Changes I made to the regex: * add the literal * to the match group of possible prefixes of pdfjsVersion * make the quotes around the version optional * make the semicolon at the end of the line optional * add newline to the list of characters that can terminate the match group (otherwise it was matching across the end of the line up to the first string, kinda odd when there was a $ after the match group)
toofar
added a commit
to qutebrowser/qutebrowser
that referenced
this pull request
Jun 2, 2025
It's changed in this PR: mozilla/pdf.js#19956 to have the version string as a comment in the file, instead of the variable. Making the regex more forgiving increases the chance of matching on the wrong string on a past or future version. I've only tested with the previous version (v5.2.133) and it seemed to still work there. Changes I made to the regex: * add the literal * to the match group of possible prefixes of pdfjsVersion * make the quotes around the version optional * make the semicolon at the end of the line optional * add newline to the list of characters that can terminate the match group (otherwise it was matching across the end of the line up to the first string, kinda odd when there was a $ after the match group)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To make it easier to tell which PDF.js version/commit that the built files correspond to, they have (since many years) included
pdfjsVersion
andpdfjsBuild
constants with that information.As currently implemented this has a few shortcomings:
It requires manually adding the code, with its preprocessor statements, in all relevant files.
It requires ESLint disable statements, since it's obviously unused code.
Being unused, this code is removed in the minified builds.
This information would be more appropriate as comments, however Babel discards all comments during building.
It would be helpful to have this information at the top of the built files, however it's being moved during building.
To address all of these issues, we'll instead utilize Webpack to insert the version/commit information as a comment placed just after the license header.