search

iulica / docx-mailmerge

Mail merge for Office Open XML (docx) files without the need for Microsoft Office Word.
MIT License
55 stars 7 forks source link

Refactoring into multiple files #18

Open dupski opened 5 months ago

dupski commented 5 months ago

Hi @iulica

How do we feel about a bit of refactoring to split the code into multiple files? (obviously keeping the external imports the same and all the tests passing).

I want to add some barcode and image functionality directly to the library but it's a pain when all the code is in one huge file.

(I saw your recent work on docx-mergefields btw, very cool!)

iulica commented 5 months ago

Hi Russel

Funnily enough, as you wrote this message I thought exactly the same thing. I was looking into the forks of this repository and the changes they made. And it would have been better for the merge if the code was separate.

At the moment I don't have any changes in mind so a refactoring would be fine. But I would wait a little bit, maybe the 2 forks having changes (regarding number formatting using locale, and something else) would first integrate those changes.

Afterwards, by all means, I would like cleaning up the code and refactoring it into multiple smaller files.

Also, the last change was quite a big one, adding footer/header files into the zip, renumbering them and so on. I don't like it very much :).

As for the future, I am thinking of removing the docx management and using the python-docx library instead. This may make things easier (or not).

About the docx-mergefields, I really hesitated adding the feature here instead, as I copied some code from here. Using the python-docx was the reason to split it. Also, I plan adding support for other fields, like IF, IF ELSE and others.

Adding those here would mean a 2 step merging. First the MERGEFIELD and related (NEXT, SKIP, NEXTIF, SKIPIF, etc) and as a second step the other outer fields.

So, I would wait for a few weeks, then we can see how it goes.

dupski commented 5 months ago

Cool, yeah all sounds good!

I will maintain my changes separately for now and look to integrate them once the most recent improvements are merged.

Yeah it would be great to make use of python-docx as it is really comprehensive library, but the API of docx-mailmerge is ideal for our needs (using word docs as templates).

Let me know if I can help with anything :)

iulica commented 2 weeks ago

No new responses to my requests to merge the changes from the forks. So I believe we can go forth with the refactoring. At the same time, switching to more modern build tools/setup would be in order I assume. I'll have to check those as well.

Would you like to do the splitting ? Do you have experience with it ?

dupski commented 1 week ago

Hi @iulica

I am happy to help with this, I do have some experience with refactoring.

I am flat out with work projects at the moment so it might not happen for a few weeks, but I will drop a message here when I've started, and please do the same if you decide to work on it yourself (or if anyone else does!)

The good news is we have the tests which will be helpful to make sure the library is still working as expected after the refactoring :)