Re: [lmi] PATCH: simplify attachment page handling

[Greg Chicares]

On 2018-05-26 09:26, Vadim Zeitlin wrote:
> 
>  I'm a bit afraid of having conflicts in PDF generation code in the future,
> as you're making sweeping changes to it while I'm trying to add pagination
> functionality to it at the same time, so I'd like to start submitting parts
> of my work even although it's not complete yet.

That's great news. While you've been busy elsewhere, I have been making
some broad changes, but I'm just about done with this round of them. My
remaining goals for the short term are:

(1) to rework wx_table_generator::compute_column_widths(), perhaps as a
free function that can be unit-tested, much as page_count() is extensively
tested by test_page_count(); and

(2) to enable '-Wconversion', which we last discussed here:
  https://lists.nongnu.org/archive/html/lmi/2018-03/msg00023.html
(which would have helped the other day when I was testing a code change
that inadvertently negated the number of columns--if it had been 13,
whose negative is -13, that wouldn't have been too bad, but actually it
was 13u, whose negative is 4294967283).

Maybe I should focus on (1), because (2) is likely to lead to sweeping
changes. OTOH, maybe the best strategy for (2) is to add '-Wconversion'
globally but exempt certain files for the nonce, in which case maybe I
could exempt 'ledger_pdf_generator_wx.cpp'.

>  The first one is rather trivial but I'm not sure what will you think about
> it. On one hand, you might love it because it gets rid of a template and
> makes the code simpler. OTOH you might disagree with it because it
> sacrifices conceptual purity (it pretends that attachment page IS-A
> numbered page, which is not really true) at the altar of convenience.

Cherry-picked and pushed.

Not long ago, I looked for examples of inheritance in lmi, and found
that I've hardly used it except for a few mix-ins and some auxiliary
facilities that are essentially independent libraries. Inheritance is
seductive, but it's not plastic: it's like carving stone rather than
molding clay. If we start out with a perfect, ageless hierarchy that
can never change (e.g., real numbers <-- complex numbers), inheritance
can be an elegant way of embodying it in code. But it's difficult to
refactor when the vision is imperfect or impermanent, say, when a
business requirement changes. I mention this now only to explain why
I don't think the conceptual purity of an inheritance graph like this
was ever going to be conserved.

>  The patch is at https://github.com/vadz/lmi/pull/84 and the commit message
> there should, hopefully, describe it clearly. One thing I don't really say
> in it is that the real motivation is that numbered_page::get_extra_pages()
> will take place of pagination in the upcoming commits

As mentioned above, page_count() is unit-tested by test_page_count(),
and I believe such unit testing is extremely valuable. Will the new
approach be similarly amenable to unit testing?

> and so, if we want to
> be able to paginate the attachment page as well (and I think we do; please
> correct me if I'm wrong)

I think we'd better assume that pagination must work for "attachment"
pages as for any other type of page.

I just ran a 'sample2_naic' illustration, and on the last page (the page
with "Certification Statements"), the "AGENT OR AUTHORIZED REPRESENTATIVE"
text almost touches the bluish line that separates it from the footer. If
evolving business requirements require more to be printed on this virtual
page, it will need to become two physical pages. We can't rule that out
because business requirements can be capricious, even though the motivating
idea was to produce one single sheet of paper that a customer would sign
with a pen.