jorisschellekens / borb

borb is a library for reading, creating and manipulating PDF files in python.
https://borbpdf.com/
Other
3.37k stars 148 forks source link

BUG #172

Closed dannyrohde closed 1 year ago

dannyrohde commented 1 year ago

Describe the bug Reimplementation of SingleColumnLayout in release 2.1.16 breaks existing workflow as it removes previously used parameters horizontal_margin and vertical_margin.

To Reproduce Up to 2.1.15, the following works:

        page = Page()
        layout: PageLayout = SingleColumnLayout(
            page,
            horizontal_margin=self.page_margin_horizontal,
            vertical_margin=self.page_margin_vertical,
        )

From 2.1.16, the two parameters are not available any more and the code breaks.

I've tried to recreate the old behaviour by directly instantiating from MultiColumnLayout, but that doesn't produce the same formatting/layout as the SingleColumnLayout did.

        layout: PageLayout = MultiColumnLayout(
            page=page,
            column_widths=[page.get_page_info().get_width() * Decimal(0.80)],
            footer_paint_method=None,
            header_paint_method=None,
            inter_column_margins=[],
            margin_bottom=self.page_margin_vertical,
            margin_top=self.page_margin_vertical,
            margin_left=self.page_margin_horizontal,
            margin_right=self.page_margin_horizontal,
        )

Expected behaviour Existing code should not break with a bug fix update from 2.1.15 to 2.1.16. SingleColumnLayout should continue to support the parameters horizontal_margin and vertical_margin.

I assume this change is unlikely to be reversed as there will likely have been good reasons to reimplement the class. In that case, could you please advise how to migrate the code in a way that horizontal and vertical margins can still be used with SingleColumnLayout? Or if I'm using it the wrong way, please advise.

I've pinned 2.1.15 for now, as it works as expected.

Desktop (please complete the following information):

Additional context Couldn't find any documentation about the change apart from the 2.1.16 change log. GitHub Blame suggests the relevant code was changed with 2.1.16.

jorisschellekens commented 1 year ago

Hi @dannyrohde ,

I'm afraid the removal of horizontal_margin and vertical_margin is final. Those parameters have been changed (split) to margin_top, margin_right, margin_bottomand margin_left. This brings the SingleColumnLayout more in line with other LayoutElementobjects, with similar properties.

You can simply set those properties according to the following logic:

That ought to be the same as what you've done previously.

If you really want these parameters to be the exact same as before, you can of course create your own class SingleColumnLayout2 and:

Kind regards, Joris Schellekens