search

claird / PyPDF4

A utility to read and write PDFs with Python
obsolete-https://pythonhosted.org/PyPDF2/
Other
328 stars 61 forks source link

Embed multiple files #27

Open michd89 opened 5 years ago

michd89 commented 5 years ago

Currently the addAttachment function is able to embed one single file only. If this function is called for another file, an existing attachment is overwritten. It would be useful to be able to attach multiple files (either with this or with a separate function).

claird commented 5 years ago

I understand the appeal.

I'm reluctant to change this in a simple-minded way; I wonder whether any existing user is relying on the overwrite feature.

Cameron Laird, vice president We make computers work for people.

On Fri, Feb 15, 2019 at 1:36 AM Michael Dietrich notifications@github.com wrote:

Currently the addAttachment function is able to embed one single file only. If this function is called for another file, an existing attachment is overwritten. It would be useful to be able to attach multiple files (either with this or with a separate function).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/claird/PyPDF4/issues/27, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbN9A1TrtkcoGvuCfHl_SSipxD0koFmks5vNnF9gaJpZM4a9LiR .

michd89 commented 5 years ago

I'd suggest an additional function (e.g. appendAttachment) as a prevention of confusion.

Since I need such a functionality at work I'm trying to implement it by myself. As far as I see the only issue might be the automatical selection of new object-IDs without overlapping with existing ones.

claird commented 5 years ago

A new entry point sounds to me like the right approach.

I look forward to your pull request.

Cameron Laird, vice president We make computers work for people.

On Mon, Feb 18, 2019 at 12:51 AM Michael Dietrich notifications@github.com wrote:

I'd suggest an additional function (e.g. appendAttachment) as a prevention of confusion.

Since I need such a functionality at work I'm trying to implement it by myself. As far as I see the only issue might be the automatical selection of new object-IDs without overlapping with existing ones.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/claird/PyPDF4/issues/27#issuecomment-464622623, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbN9Phkh5-SCZuO_HW60eUQqjr9VmaLks5vOlt5gaJpZM4a9LiR .

michd89 commented 5 years ago

Does PyPDF somewhere make arrangements for updating the xref table when the PDF content is changed?

michd89 commented 5 years ago

I see it does. By now I added a function which receives a file path or a list of file paths and adds them to the PDF (2210451). However existing attachments will be overwritten. Passing file paths instead of streams might be a bit more convenient.

acsor commented 5 years ago

@michd89 I glanced over your code and that sounds about great. If on the future you plan to do a PR for some of your enhancements, would you be able to provide some unit tests for your code? It's very important.

A full listing of the contribution guidelines can be found here (in particular, you might consider adding a docstring documentation to your newly added method).

Keep up the good work :+1:.

michd89 commented 5 years ago

@newnone Thank you very much for your hints. I (hopefully correctly) implemented them and did a PR.

acsor commented 5 years ago

I notice some unit tests, which looks quite good. Just leave me a few days and will be able to review your PR.