Re: [lmi] Can we remove unused elements of the columns title map?

[Greg Chicares]

On 2017-09-26 23:12, Vadim Zeitlin wrote:
> 
>  AFAICS, we currently have a lot of unused elements in the "title_map"
> defined in Ledger::write(xml::element&) function in ledger_xml_io.cpp
> because many of the columns for which titles are defined in this map do
> not, in fact, correspond to any element of mcenum_report_column and hence
> can't appear in any supplemental report.

I try to avoid reading that code--it's painful.

>  I'd like to remove these unused titles because I plan to change the format
> of all of them: it doesn't make sense to keep using the hack with
> underscores which (in conjunction with the recursive "text-word-wrap"
> function in fo_common.xsl which is almost as easy to understand as
> Ackermann function), was used to break the titles into lines when we can
> simply use "\n" characters in them now. And, of course, being lazy, I'd
> like to update only half of the strings that are actually used, rather than
> all of them.
> 
>  Would anything be lost if I removed them? Or would it be easier for you if
> I preserved them for now? Please notice that I don't think that it's going
> to make reviewing the changes any easier because all these unused titles
> would still need to be changed, instead of being removed.

I'd like to know which ones you want to remove. Probably many are just
garbage, but maybe not all. Would it make sense to ask you for a patch
to 'ledger_xml_io.cpp' that demonstrates what would be removed? I'm
assuming that this entire source file will soon enough be expunged,
and that changing it directly in the savannah repository now would
make your PDF-replacement work (and its review) easier.

>  Of course, if there are any plans to actually extend mcenum_report_column
> to include all the columns for which titles are defined even though they
> don't appear in any supplemental reports, it would be better to keep them.
> This could also make sense if you plan to make any other, more sweeping,
> changes to this area in near future as handling of these column titles,
> with different definitions here, in ledger_metadata_map() in
> ledger_text_formats.cpp and the names defined by the enum itself, with a
> "TODO" comment saying that they should be made "nicer-looking", is
> definitely not ideal and could be improved.

Using the "nicer-looking" names would be an improvement, but it would
entail a user-visible change; combining those two considerations may
yield a net negative. Some sentiment has been expressed that might
support a grand renaming, but it's probably better to keep that a
separate project, to be undertaken later.

>  Final possibility would be to keep all this code completely unchanged
> and create _another_ set of column titles in the PDF generation code
> itself. This would make the upcoming review simpler, and would allow me to
> reuse the same map for all columns used in the illustration tables, whether
> they correspond to mcenum_report_column elements or not. But it would also
> mean that the columns appearing both in "normal" and "supplemental" reports
> would use the same name in both, which actually seems like an advantage to
> me but which is _not_ the case currently. E.g. consider a basic column such
> as "AcctVal_Guaranteed" which uses "Account Value" labels in all the
> different tables it appears in, but "Guar Account Value" in supplemental
> reports. So I'm not really sure whether it would be a good idea.

In that example, I imagine (though I haven't checked) that the
"Account Value" header would appear only under a "Guaranteed"
multi-column super-header. I'll need to ask whether something like

  ------Guaranteed------
  Guar Acct  Guar Death
    Value      Benefit
  ---------  ----------

would be acceptable.