Re: [lmi] Group quotes: empty footnotes

[Greg Chicares]

On 2015-11-07 15:56, Vadim Zeitlin wrote:
> On Sat, 10 Oct 2015 04:29:33 +0000 Greg Chicares <address@hidden> wrote:
[...]
> GC> A problem arises if, in the future, we rearrange footnotes. Suppose
> GC> we move the second one to the end. Then the PDF has an unsightly gap
> GC> due to the unconditional <br><br>, and we'd like to eliminate that gap.
> GC> I.e., if a footnote is an empty string, then its <br><br> should not be
> GC> written at all. (The criterion really is string::empty(); we will not
> GC> specify any all-blank strings.)
> GC> 
> GC> Vadim, would you please put this aside for a couple weeks at least,
> GC> and then propose a patch?
[...]
>  Here is a patch that fixes the problem by replacing all the expressions of
> the form
> 
>       footer_ += <tag> + string;
> 
> with the calls to a helper function:
> 
>       footer_ += wrap_if_not_empty<tag>(string);
> 
> IMO using such helper is better than explicitly writing
> 
>       if(!string.empty())
>           {
>           footer_ += "<tag>" + string;
>           }
> 
> many times because it eliminates the risk of stupid errors due to testing
> one string and then outputting another one and also seems more readable to
> me.

Agreed.

>  But it does make the code longer,

At least as an interim step, I shortened it in revision 6402, in a way that
I think is still consonant with your further ideas below.

> so ideally, I'd like to write just this
> instead:
> 
>       footer_ += tag << string;
> 
> with operator<<() taking care both of omitting the empty tags and of
> closing the tag if necessary before converting the result to string. I.e.
> the real code could then look like this:
> 
>       footer_ +=
>           html::tag::br
>               << html::tag::br
>                   << html::tag::b
>                       << html::text_if_not_empty(invar.GroupQuoteProspectus)
>       ;
> 
>  I didn't implement this because I didn't know if you would agree to do it
> like this, but I think it would be worth doing and I'd also like to change
> the rest of the already existing code for HTML generation to use this
> approach instead of just manipulating the raw strings, with all the
> concomitant dangers (e.g. forgetting to escape something or escaping
> something twice).

That's reasonable. Where my revision 6402 uses helper functions, you'd use
streams instead, which gives us a more flexible, easily extensible syntax.

As for layout, I'd sacrifice an eight-column maximum width to achieve
significant vertical compression, so that we can easily see what actually
varies between lines in the footer:

  footer_ += html::tag::br << html::tag::br << html::tag::b << 
html::text_if_not_empty(invar.GroupQuoteProspectus);
  footer_ += html::tag::br << html::tag::br << html::tag::b << 
html::text_if_not_empty(another_footnote);
  footer_ += html::tag::br << html::tag::br << html::tag::b << 
html::text_if_not_empty(and_another);
  ...

and I think this would be even better:

  footer_ += br << br << b << text_if_not_empty(invar.GroupQuoteProspectus);

because in this case terseness favors readability (unlike the cryptic brbr()
and brbrb() functions in my revision 6402). Or, if you like:

  footer_ += BR << BR << B << text_if_not_empty(invar.GroupQuoteProspectus);

which makes those html tags stand out.

>  For example, the following snippet
> 
>     html_table += wxString::Format
>         ("<td nowrap align=\"right\"><b>%s%s&nbsp;&nbsp;</b></td>"
>          "<td>%s&nbsp;&nbsp;&nbsp;&nbsp;</td>"
>         ,escape_for_html_elem(name)
>         ,(value.empty() ? "" : ":")
>         ,escape_for_html_elem(value)
>         );
> 
> could then be rewritten as
> 
>       html_table +=
>           (html::tag::td
>            << (html::tag::b
>                   << html::text(name)
>                   << html::text(value.empty() ? "" : ":")
>                   << html::nbsp << html::nbsp
>               )
>           )
>           <<
>           (html::tag::td
>               << html::text(name)
                              ^^^^
I think this should be "value"^^^^

>               << html::nbsp << html::nbsp << html::nbsp << html::nbsp
>           )
>           ;
> 
> which is, admittedly, still longer, but provides compile-time safety (i.e.
> a typo in "nbsp" or lack of closing tag would be detected) and is arguably
> more clear as well.

Preserving all those advantages, and changing only the tag names...

    html_table +=
        (TD << (B << html::text(name) << html::text(value.empty() ? "" : ":") 
<< NBSP << NBSP))
        << (TD << html::text(value) << NBSP << NBSP << NBSP << NBSP)
        ;

...I think we're approaching a nice syntax. A similar idea, introducing optional
arguments for tag-manipulators:

    html_table += TD("nowrap align=\"right\"") << B << html::text(name) << 
(value.empty() ? "" : ":") << NBSP(2);
    html_table += TD << html::text(value) << NBSP(4);

BTW, I remembered that cgicc did something like this, so I took a look, but 
didn't
find any better ideas in lmi's 'main_cgi.cpp', which does things the cgicc way.

>  Do you think it would be worth writing HTML generation code like this?

Yes. The big goal is replacing XSL with something expressive and maintainable.
This small group-quote TU is an appropriate laboratory for essays in the craft.

This is a good opportunity to reconsider our prior discussion of reinventing XSL
(but not poorly). What would that HTML table look like if we took that approach?
Maybe something like this, with the transformation of 'value' elided:

    <td nowrap align="right"><b>NAME</b>&nbsp;&nbsp</td>
    <td>VALUE</td>

where we'd substitute NAME and VALUE. The same thing with the C++ syntax above:

    TD("nowrap align=\"right\"") << B << name << NBSP(2);
    TD << value << NBSP(4);

I prefer the latter--advantages:
 - about equally terse;
 - self-closing tags;
 - the possibility of manipulators with arguments;
 - above all, it's native C++, so there's no need to design an XSL-like
     language and write a parser for it.

Here's another thought. It looks like we're really designing reports in HTML, 
and
almost incidentally transforming HTML to PDF, so...

 - Should we separate HTML and PDF into two distinct translation units?

Then we could have
  group_quote_pdf_gen_common.cpp
which does all the HTML work, and
  group_quote_pdf_gen_cgi.cpp
which would presumably be trivially simple, and
  group_quote_pdf_gen_cli.cpp
which might write an '.html' file; and
  group_quote_pdf_gen_wx.cpp
which would use the wxPdfDoc library to turn HTML into PDF.

This might make regression testing much easier (I'm thinking beyond group quotes
to replacing XSL), because HTML is just plain text.

 - Would htmldoc(1) work? ( http://linux.die.net/man/1/htmldoc )

If it's sufficient for our purposes, then we could just generate HTML and turn
it into PDF, so that lmi's CLI program could create PDFs...and the GUI interface
wouldn't need the wxPdfDoc library (except for displaying PDFs). But if this 
were
the magic answer to all our problems, you'd probably have suggested it already.