Re: [lmi] Goodbye to XSL-FO

[Vadim Zeitlin]

On Sat, 13 Oct 2018 23:40:02 +0000 Greg Chicares <address@hidden> wrote:

GC> I've purged XSL-FO from git..."and all I can think of...is...
GC> Thank God and Greyhound you're gone".

 Thanks for giving me an opportunity to extend my musical culture.
Unfortunately, knowing how lacking it is, we'll certainly run out of
lmi components to discard before you can really make a dent in the depths
of my ignorance.

GC> No more XSL! No more java!

 But at least I can share your joy about this!

GC> We'll go through one more round of testing before releasing it into
GC> production (at the end of next month, barring disaster). The only
GC> remaining task is to make lmi read xz-compressed MST files.

 Yes, this is the next, i.e. current, item on my TODO list.

GC> $git show 0eec56ecf
GC> 
GC> @@ -62,7 +62,7 @@ std::string attribute::as_string() const
GC>      if(!value_.empty())
GC>          {
GC>          s += "=";
GC> -->      // TODO: Escape quotes.
GC> 
GC> @@ -130,7 +130,7 @@ void wx_table_generator::do_output_horz_separator(int 
x1, int x2, int y)
GC>  
GC>  void wx_table_generator::do_output_vert_separator(int x, int y1, int y2)
GC>  {
GC> -->  // TODO: add a possibility to have a thick border between the columns.
GC> 
GC> Vadim, do you see any need to act on those two, or even to retain the
GC> markers? Escaping quotes sounds like a good idea;

 Yes. We don't actually need it right now but at the very least I think we
need to check whether escaping is needed and complain about it, to avoid
weird breakage if this is ever the case. OTOH as long as we check for
whether it's needed, we could escape it too as it doesn't cost much more.
Anyhow, doing this is simple, but I'd like to also use this as an excuse to
add a unit test for the classes in html namespace (which is something I
should have done from the beginning, of course, but didn't in my haste to
get something usable first). So it will just take some time.

 But note that this is really a "TODO" and not a "FIXME", i.e. there is no
problem with not quoting/escaping anything right now because we use only a
few attribute values and none of them requires either quoting or escaping.

GC> but I don't think we'll ever actually want a thick border between
GC> columns, so I'm inclined to purge that marker.

 This is up to you, the original group quotes mock ups had it, so I wanted
to emulate them, but, AFAIR, you told me it wasn't particularly urgent, and
so I never did. If you don't think it's useful, there is no problem with
removing this one, of course.

GC> As for this in numeric_summary_table_cell::Draw()...
GC> 
GC> $git show d9ec2b90b
GC> 
GC>         // The horizontal coordinate is unused, but should always be zero.
GC>         // PDF !! However, it isn't: this assertion fails when the
GC>         // automated GUI test is run.
GC> //      LMI_ASSERT(0 == x);
GC>         (void)&x; // PDF !! Temporary workaround pending investigation.
GC> 
GC> Does it matter at all if the x coordinate is nonzero? I haven't looked
GC> into it, but I suppose the numeric summary may be slightly indented;
GC> do we care about that?

 OK, this one was also in my TODO list, sorry for taking to long to look at
this, especially as the problem is pretty obvious: my original comment,
which you replaced with this assert later in 567e5ac51f, was just wrong,
the abscissa is not 0 as it's the value passed to writer_.output_html()
which is the same value as pdf_writer_wx::get_horz_margin(), i.e. 18.

 It also so happens that it's not actually a problem because the numeric
summary table always draws itself using wx_table_generator created by
create_table_generator() which also uses exactly the same value returned by
pdf_writer_wx::get_horz_margin() as it's offset. However I think it is
confusing and, ideally, we shouldn't ignore the "x" parameter of
numeric_summary_table_cell::Draw() and pass it to render_or_measure()
which, in turn, should pass it to create_table_generator() which should use
it instead of always using get_horz_margin().

 OTOH it all starts being a bit complicated too, so perhaps we should just
assert that "x == mixin_writer_.get_horz_margin()" instead? While this
*would* trigger the assert if the cell is ever indented in the HTML
template, it might be argued that we want this to happen as otherwise it
wouldn't fit the page horizontally.

 Which of these solutions, or which alternative solution, would you prefer?
VZ