Convert the Catalog class, in src/core/obj.js, to ES6 syntax
#9990
Conversation
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/ec727e4dc3438ec/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/136a7ca6bc078c0/output.txt |
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/ec727e4dc3438ec/output.txt Total script time: 30.58 mins
|
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/136a7ca6bc078c0/output.txt Total script time: 35.92 mins
|
src/core/obj.js
Outdated
| throw e; | ||
| } | ||
| info('Skipping invalid metadata.'); | ||
| const stream = this.xref.fetch(streamRef, !encryptMetadata); |
There was a problem hiding this comment.
You're obviously just touching old code here, but this looks a bit convoluted. How about, in a separate commit, updating this to be a bit easier to read?
const suppressEncryption = !(this.xref.encrypt &&
this.xref.encrypt.encryptMetadata);
const stream = this.xref.fetch(streamRef, suppressEncryption);
src/core/obj.js
Outdated
| var prefix = ''; | ||
| const pageLabels = new Array(this.numPages); | ||
| let style = null; | ||
| let prefix = ''; |
There was a problem hiding this comment.
Nit: These can probably just go on one line instead, i.e. let style = null, prefix = ''; (since that's how e.g. currentLabel/currentIndex below is already formatted in the old code).
src/core/obj.js
Outdated
| return total; | ||
| } | ||
| const count = args[0]; | ||
| const parentRef = args[1]; |
There was a problem hiding this comment.
You should be able to use array destructuring instead, i.e. const [count, parentRef] = args;.
src/core/obj.js
Outdated
| // Attempt to recover valid URLs from 'JS' entries with certain | ||
| // white-listed formats, e.g. | ||
| // Attempt to recover valid URLs from `JS` entries with certain | ||
| // white-listed formats, e.g.: |
There was a problem hiding this comment.
Nit: It might be a personal preference, but I find that a colon after a period looks really ugly; can we please not do that :-)
src/core/obj.js
Outdated
| get attachments() { | ||
| let attachments = null, nameTreeRef; | ||
| const obj = this.catDict.get('Names'); | ||
| if (obj) { |
There was a problem hiding this comment.
This is another potential for clean-up (again in a separate commit).
It looks like this code was basically copy-pasted from an older version of the destinations getter, but it could probably be changed to e.g.
get attachments() {
const obj = this.catDict.get('Names');
let attachments = null;
if (obj && obj.has('EmbeddedFiles')) {
const nameTree = new NameTree(obj.getRaw('EmbeddedFiles'), this.xref);
const names = nameTree.getAll();
for (let name in names) {
const fs = new FileSpec(names[name], this.xref);
if (!attachments) {
attachments = Object.create(null);
}
attachments[stringToPDFString(name)] = fs.serializable;
}
}
return shadow(this, 'attachments', attachments);
}
|
/botio-windows test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/c46e060530532dc/output.txt |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/c46e060530532dc/output.txt Total script time: 24.44 mins
Image differences available at: http://54.215.176.217:8877/c46e060530532dc/reftest-analyzer.html#web=eq.log |
|
Thank you for the review, @Snuffleupagus! The failure above is an intermittent one. |
This patch is much easier to review with the
?w=1flag: https://github.com/mozilla/pdf.js/pull/9990/files?w=1.@Snuffleupagus Would you, time permitting of course, be willing to review this?