Alignment when using parentheses for negative values (#144)

[steveputman]

@rich-iannone Here's a first cut. Code changes in format_data.R (I conformed the order of the latex/html/default functions in what I dealt with--that might have screwed up the diff, so happy to try to put that back together). Basically just takes the nonneg vector from x_str and applies leading and trailing hidden spaces, using hphantom{} in LaTeX and <span> in HTML. Changes applied for LaTeX and HTML in fmt_number(), fmt_percent, and fmt_currency.

[jcheng5]

@steveputman Thanks for this contribution! It'd be great if this change didn't result in the default formatter code being copy/pasted for html and latex, but instead to use the "function factory" pattern--see fmt_scientific for an example of this pattern at work.

[steveputman]

@jcheng5 — got it. It was bugging me when I was copying and pasting! Will have a look and revert.

[steveputman]

Added factory functions for fmt_number, fmt_percent, and fmt_currency. In addition to resolving the issues with repetition of my code, doing this also cleaned up some legacy duplication (avoided the need for currency_str vs currency_str_html vs markdown_to_latex(currency_str), for example).

I suppose there could be a megafactory function for just about all the functions in format_data but already felt I was overstepping.

I assigned the HTML and LaTeX used for all the nonneg formatting to vars near the top of the file but wonder if the HTML and LaTeX should just be repeated in the calls to the factory functions to avoid a sort of Where's Waldo problem.

Any further guidance would be appreciated.

[rich-iannone]

This looks really good. I sort of agree with the idea of a larger/better function for all of the functions here. However, there is still some not-yet-merged PRs that need to be integrated here first. I think that this could be part of a separate refactoring PR for this file.

I would put this inside the utils.R file:

# Strings for alignment of non-negative number in formatting functions
 paren_nonneg_start_html <- "<span style=\"visibility: hidden;\">(</span>"
 paren_nonneg_end_html <- "<span style=\"visibility: hidden;\">)</span>"
 paren_nonneg_start_latex <- "\\hphantom{(}"
 paren_nonneg_end_latex <- "\\hphantom{)}"

Thanks again for your continued work on this. This will get merged soon!

[rich-iannone]

Thanks for the quick turnaround on this. We will review again and merge soon after that. A final ask: could you please sign the individual or corporate contributor agreement as appropriate. Then, send the signed copy over to [email protected]. (This is something we ask of any significant outside contribution.)

[steveputman]

Thanks! Contributor agreement sent.

[steveputman]

Closing this PR; will replace with new PR to reflect refactoring of formatters