Re: [lmi] Header pagination PR

[Greg Chicares]

On 2018-09-18 14:26, Vadim Zeitlin wrote:
[...]
>       https://github.com/vadz/lmi/pull/96

* First step: reviewing only the commit messages, not the code...

1c398a86 (HEAD -> master, origin/master, origin/HEAD) \
         Implement support for headers for normal PDF illustration pages

    Show this tag in action in the "Explanatory Notes" page of reg_d
    individual illustration: with sample2ipp its second page correctly
    repeats the logo and the title now, while it didn't have them before.

After testing it with 'sample2ipp', I believe I should use the
new <header> tag in all other '*.mst' files that have a logo, right?

71dc467e Pass the illustration and interpolator to the page ctor

In uncommitted work that I've put aside for the moment, I believe
I can demonstrate that the 'illustration_' member can provide
everything we've been passing as arguments to so many functions:
  {Ledger, interpolator, pdf_writer}
I'll take another look in light of this commit 71dc467e. I was about
to explore the idea of setting the 'illustration_' member upon
construction, i.e., in add<>(); perhaps you've already done something
like that.

* Second step: reviewing the code

'install*.sh': I've studied these changes in detail and have no concerns;
as noted in a separate email, we'll know next week whether the autoreconf
issue arises again on machines in the office.

'*.mst': See question on commit 1c398a86 above.

'pdf_writer_wx.?pp': I've never studied any version of this module,
so I won't be able to comment on it.

'ledger_pdf_generator_wx.cpp': 92 hunks of changes, so this will take
a little longer. It all seems fine at a superficial glance; the only
question I have is here:

  class reg_d_indiv_curr : public page_with_tabular_report
  {
    public:
      using page_with_tabular_report::page_with_tabular_report;

and in the various similar hunks preceding. I suppose this is a modern
C++ idiom that I can't recall seeing previously. Is it really just a
C++98 using declaration? Why does a derived class need to declare its
base class's ctor in such a way? I traced the chain all the way up to
class numbered_page, whose ctor is public, so I don't understand why
'using' is needed.