luabagg / orcgen

Web pages and HTML content conversion
https://pkg.go.dev/github.com/luabagg/orcgen
MIT License
9 stars 4 forks source link

Modify the project to admit configurations per type of output #3

Closed magandrez closed 2 months ago

magandrez commented 2 months ago

Hi,

I am eyeballing utilizing this package for a prototype I am building at my current workplace to generate PDFs from a dynamically generated HTML.

As it stands today, the package allows to generate such PDFs nicely. However, I would like to be able to configure the output, namely, I would like to configure DisplayHeaderFooter. Currently, there is only one configuration available (for all types of output) that is SetFullPage. I have been dabbling in different ways to propose a change to the project to do properly dependency injection. However, it is clear that doing dependency injection would affect the main interface of the package becoming a breaking change.

I was wondering how would you approach this and how willing to accept these type of breaking changes. If you are positive to that, I could take a stab at it.

What I am thinking on is the following:

Do you have any thoughts on this?

magandrez commented 2 months ago

I made a quick example as part of #4 Note that the PR is just to showcase what I mean.

magandrez commented 2 months ago

The main problem with the structure of the pkg with the modification hinted above is that the configuration would be buried under the internal package, making it impossible to use.

Again, as mentioned, enabling this type of dependency injection would be a breaking change, so I would like to contribute, but I would like to have your view on this.

luabagg commented 2 months ago

Hello,

I already had something like this in mind, but dind't have the time to implement it. Great to know that you want to use this package.

We can do this, yes. There's others improvements I may work on too.

magandrez commented 2 months ago

Ok, cool.

How do you want to proceed? How can I contribute?

luabagg commented 2 months ago

I think we can do it async. I'll take a look at your PR, implement my ideas over it, then you can review and contribute if needed.

Just let me know if you got any max deadlines. I'm planning to implement it this weekend.

luabagg commented 2 months ago

Please check it out https://github.com/luabagg/orcgen/pull/5.

magandrez commented 2 months ago

Awesome, I'll take a look at the changes more in depth and test it out tomorrow.

luabagg commented 2 months ago

I suggest you to try the code and check if it works for you. If you try to see change per change it will take long, because I basically rewrote the lib.