Re: [lmi] Group-quote PDF: whitespace changes, and enhancement

[Vadim Zeitlin]

On Mon, 12 Mar 2018 19:28:06 +0000 Greg Chicares <address@hidden> wrote:

GC> Stripped down to declarations only, it is:
GC> 
GC>     class column_info
GC>     {
GC>       public:
GC>         column_info(std::string const& header, int width);
GC> 
GC>         bool is_hidden() const;
GC>         bool is_centered() const;
GC>         bool needs_clipping() const;
GC> 
GC>         std::string const header_;
GC>         int width_;
GC> 
GC>       private:
GC>         bool const is_variable_width_;
GC>     };
GC> 
GC> I'm trying to understand the circumstances in which columns would
GC> be hidden, centered, or clipped.

 In principle, all of those are independent. In practice, fixed width
columns are centered and variable width columns are clipped and only the
former can be hidden. But I don't think this low level class should impose
such high level constraints on its use, which is why it doesn't do it.


GC> - is_hidden()
GC> 
GC> For what sort of columns does this return true?
GC> 
GC> AFAICT, it's used only for group premium quotes, e.g.:
GC> 
GC>             case e_col_total_face_amount:
GC>                 if(!has_suppl_amount)
GC>                     // Leave the header empty to hide this column.
GC>                     break;
GC>                 // Fall through
GC>             ...
GC>                 header = cd.header_;
GC> 
GC> Some columns are conditionally hidden by should_show_column():
GC> 
GC>     // May be overridden to return false if the given column shouldn't be 
shown
GC>     // for the specific ledger values (currently used to exclude individual
GC>     // columns from composite illustrations).
GC>     virtual bool should_show_column(Ledger const& ledger, int column) const
GC> 
GC> but that technique seems to be orthogonal to is_hidden() and used
GC> only for illustration PDFs.

 No, it's not orthogonal, should_show_column() is used to decide whether
the column label should be left empty, making the column hidden.

GC>  - is_centered()
GC> 
GC> This seems to be used only in one place:
GC> 
GC>     if(ci.is_centered())
GC>         {
GC>         // Centre the text for the columns configured to do it.
GC>         x_text += (width - dc_.GetTextExtent(s).x) / 2;
GC>         }
GC> 
GC> What exactly does it mean for a column to be "centered"? I think this
GC> is a different concept than using "center" alignment for cells in a
GC> spreadsheet column, which would give, e.g.:
GC>     1
GC>   11111

 No, it's exactly the same concept.

GC> In spreadsheet terminology, almost all our columns are numeric, and
GC> our numeric columns are right-aligned. But...
GC> 
GC>     // Return true if this column should be centered, rather than
GC>     // left-aligned. Notice that this is ignored for globally right-aligned
GC>     // tables.
GC> 
GC> Is it then the case that:
GC>  - for illustration PDFs, all columns are right-aligned, and
GC>  - is_centered is used only for group quotes, where it really does
GC>    mean the same thing as "center" alignment in a spreadsheet
GC> ?

 Yes.

 This is indeed not as lucid as I'd like, but the alternative would to
modify the PDF quotes code to align all the columns explicitly, which I
preferred not to do as part of illustrations work. Maybe now, that this is
merged, it's indeed worth changing this.

 OTOH, unlike a spreadsheet, this class doesn't have any notion of numeric
or text values, so its align_right() method is still useful to globally
configure all columns to be right-aligned. Perhaps we could just add a
similar align_centre() method and call it from the group PDF quotes code
and continue to handle the variable width columns specially by
left-aligning them in any case?

GC> Apparently is_centered() always returns true (but is ignored)
GC> for illustrations, and this comment applies to group quotes only:
GC> 
GC>     // Fixed width columns are centered by default, variable width ones
GC>     // are not as long strings look better with the default left
GC>     // alignment.
GC> 
GC> What sort of columns are not centered?

 Variable width ones (only used by PDF quotes).


GC> - needs_clipping()
GC> 
GC> And what sort of columns need to be clipped? As currently implemented,
GC> this function is the logical negation of is_centered(), so only columns
GC> that are not centered need clipping--but what columns are those? Only
GC> the "Participant" column on group quotes?

 Currently, yes, as it's the only variable width column.

GC> Does this all boil down to...
GC>  - left-align and clip the group-quote "Participant" column
GC>  - center all other group-quote columns
GC>  - ignore all these accessors for illustration PDFs
GC> ?

 Yes.

GC> And what does 'is_variable_width_' mean? As implemented, it means
GC> that the constructor's 'width' argument was zero at the time of
GC> construction. Is that in effect just another way of identifying
GC> the "Participant" column?

 No, as with "centered" above, it really means what it says: a variable
width column is one whose width is not fixed, i.e. not defined by the
widest string that can appear in it, but takes all the available space
remaining from the other, fixed width columns (in principle, there can be
more than one variable width column, even if currently this is not the
case).


 I'm sorry for all the confusion, but the fundamental distinction is really
between fixed and variable width columns: the latter ones are always
left-aligned and need to be clipped, while the former ones are either
centered or right-aligned (if align_right() was called) and not clipped.
And I think things are reasonably simple seen from this point of view and
this is how you're supposed to see them, because it's how this class is
used, while the various accessors discussed above are just its
implementation details.

 Please let me know if there are any questions remaining,
VZ