elwerene / libreoffice-convert

MIT License
241 stars 94 forks source link

Expose parameter to override the tmp library options #35

Closed a7i closed 3 years ago

a7i commented 3 years ago

Fixes #34

elwerene commented 3 years ago

It feels strange to have arguments behind the callback argument. Could you change your merge request to provide a new method with e.g. a options Object which includes the tmpOptions, like this:

 exports.convertWithOptions = (document, format, filter, options, callback) => {

and in exports.convert you call it with some default options?

You are free to choose a name, convertWithOptions sounds like a objective-c method, not a ecmascript method :)

a7i commented 3 years ago

I agree @elwerene , did not want to break backwards compatibility. I will apply the changes per your suggestion.

elwerene commented 3 years ago

It will not break if you keep the signature of exports.convert ;)

elwerene commented 3 years ago

@a7i Please put the tmpOptions into an options object so we have some space where we can introduce new options in the future!

elwerene commented 3 years ago

looks good to me!

elwerene commented 3 years ago

I published a new version 1.2.0. Thx @a7i