-
Notifications
You must be signed in to change notification settings - Fork 50
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
Allow multiple pdf files to be converted #40
base: develop
Are you sure you want to change the base?
Changes from all commits
2f06a2b
764a48a
5e613ac
9e5c177
fc3a371
b75d5c3
7b76501
abe6e81
a0009cf
8bce0ad
be8fe54
3a5f48c
f65c795
05cefa0
2c9664d
9f9f83c
f7a4b42
36b03ec
9d1ec42
1df7531
be438f8
0c70b8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,11 +16,11 @@ var options = { | |
var Pdf2Img = function() {}; | ||
|
||
Pdf2Img.prototype.setOptions = function(opts) { | ||
options.type = opts.type || options.type; | ||
options.size = opts.size || options.size; | ||
options.density = opts.density || options.density; | ||
options.outputdir = opts.outputdir || options.outputdir; | ||
options.outputname = opts.outputname || options.outputname; | ||
options.type = opts.type != null || opts.type != 'undefined' ? opts.type : options.type; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is broken in so many levels that I don't know where to start. Why have you changed these line at all? AFAIK the original implementation was correct. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needed to edit the lib to be used in converting list of PDF files, so far it's working on my end. Most likely my code is not clean or should have better error handling. Added a new function at the bottom that handles an array of PDF where "options" can be passed. Oh well, will need to re-visit this next time. |
||
options.size = opts.size != null || opts.size != 'undefined' ? opts.size : options.size; | ||
options.density = opts.density != null || opts.density != 'undefined' ? opts.density : options.density; | ||
options.outputdir = opts.outputdir != null || opts.outputdir != 'undefined' ? opts.outputdir : options.outputdir; | ||
options.outputname = opts.outputname != null || opts.outputname != 'undefined' ? opts.outputname : options.outputname; | ||
}; | ||
|
||
Pdf2Img.prototype.convert = function(input, callbackreturn) { | ||
|
@@ -166,4 +166,31 @@ var isFileExists = function(path) { | |
} | ||
} | ||
|
||
|
||
// Convert list of pdf files, 1st argument is array of pdf files, 2nd argument need to pass opts object to set options variable | ||
Pdf2Img.prototype.convertList = function(arr, opts, callbackreturn) { | ||
async.eachSeries(arr, function(file, callback) { | ||
Pdf2Img.prototype.setOptions(opts); | ||
Pdf2Img.prototype.convert(file, function(err, info) { | ||
if(err) { | ||
return callback({ | ||
error: 'Unable to convert file ' + file, | ||
message: err | ||
}, null); | ||
} else { | ||
callback(null, info); | ||
} | ||
}); | ||
}, function(err) { | ||
if(err) { | ||
return callbackreturn({ | ||
error: 'Unable to convert everything on array', | ||
message: err, | ||
}, null); | ||
} else { | ||
callbackreturn(null, 'All files have been converted'); | ||
} | ||
}); | ||
}; | ||
|
||
module.exports = new Pdf2Img; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
{ | ||
"name": "pdf2img", | ||
"version": "0.2.0", | ||
"version": "0.5.0", | ||
"description": "A nodejs module for converting pdf into image", | ||
"author": "Fitra Aditya <[email protected]>", | ||
"license": "MIT", | ||
|
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.
This doesn't seem right, now if options has already value and it is not given in opts it will be overridden with undefined.
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.
Thanks for the feedback, I made another commit that should handle assignments on the options variable.