search

elwerene / libreoffice-convert

MIT License
241 stars 94 forks source link

async/await vs. callback #38

Open a7i opened 3 years ago

a7i commented 3 years ago

Would you be open to supporting promises vs. callbacks? I realize there are many considerations including API backward compatibility.

I would be happy to contribute an async approach if this aligns with future plans for this library (i.e. async/await).

elwerene commented 3 years ago

Honestly I'm just keeping the library alive by answering to issues/pull requests and releasing new versions without coding myself. I'm open to release a major/breaking update with promises. But we should cleanup the API while doing so and have a look on all open issues like https://github.com/elwerene/libreoffice-convert/issues/17 and https://github.com/elwerene/libreoffice-convert/issues/22 @a7i Would that be something you want to do? I will happily review your changes and release them once they are ready :)

ilkohoffmann commented 3 years ago

@elwerene -> I have played around a bit with your library on OSX and did some refactoring. I have basically translated the code to Typescript and removed the async-library to improve its readability. I also tried to simplify the Converter-API. It´s now expecting an ConverterOptions object to be passed in which can be further extended. I didn´t write any tests so far but it works fine at least on my OS. I also experienced performance issues on Windows OS using the master branch although I think this is rather caused by the binary and runtime environment and not by the library itself.

Feel free to have a look at: https://github.com/ilkohoffmann/libreoffice-convert/tree/refactoring

elwerene commented 3 years ago

@ilkohoffmann oh wow, that could form the basis for a promising (pun intended) 2.0 release :)

ilkohoffmann commented 3 years ago

@elwerene 2.0 sounds good to me ;) Let me know how you want to proceed. I have done some more refactoring and added the missing test. I have removed the Promise<Buffer> return value and a few (imo) unecessary buffer-to-file and file-to-buffer conversions. Now the library is doing the conversion and the storage on the filesystem only. Before my changes it converted the office file to a pdf file and generated a buffer of this file afterwards.

elwerene commented 2 years ago

I'm open for pull requests :)