From f6feef7f80ba80b95844b311f93303980a3fe36d Mon Sep 17 00:00:00 2001 From: Stephen Morgan Date: Tue, 6 Apr 2021 19:18:46 +1000 Subject: [PATCH] lib,cli: Strip prices in MultiBalanceReport and PostingsReport whenever we know we won't need them. Knowing whether we need them is accomplished by pulling the "show-costs" option used by the Close command up into ReportOpts. --- .../Hledger/Reports/MultiBalanceReport.hs | 22 ++++++++++----- hledger-lib/Hledger/Reports/PostingsReport.hs | 5 ++-- hledger-lib/Hledger/Reports/ReportOptions.hs | 3 ++ hledger/Hledger/Cli/Commands/Balance.hs | 28 +++++++++++-------- hledger/Hledger/Cli/Commands/Close.hs | 13 +++------ hledger/Hledger/Cli/Commands/Register.hs | 12 +++++--- hledger/test/journal/transaction-prices.test | 14 +++++++++- 7 files changed, 63 insertions(+), 34 deletions(-) diff --git a/hledger-lib/Hledger/Reports/MultiBalanceReport.hs b/hledger-lib/Hledger/Reports/MultiBalanceReport.hs index 589d4a520..3bdb5d2db 100644 --- a/hledger-lib/Hledger/Reports/MultiBalanceReport.hs +++ b/hledger-lib/Hledger/Reports/MultiBalanceReport.hs @@ -558,14 +558,22 @@ cumulativeSum value start = snd . M.mapAccumWithKey accumValued start postingAndAccountValuations :: ReportSpec -> Journal -> PriceOracle -> (DateSpan -> Posting -> Posting, DateSpan -> Account -> Account) postingAndAccountValuations ReportSpec{rsToday=today, rsOpts=ropts} j priceoracle = case value_ ropts of - Just (AtEnd _) -> (const id, avalue' (cost_ ropts) (value_ ropts)) - _ -> (pvalue' (cost_ ropts) (value_ ropts), const id) + -- If we're doing AtEnd valuation, we may need to value the same posting at different dates + -- (for example, when preparing a ValueChange report). So we should only convert to cost and + -- maybe strip prices from the Posting, and should do valuation on the Accounts. + Just v@(AtEnd _) -> (pvalue Nothing, avalue v) + -- Otherwise, all costing and valuation should be done on the Postings. + _ -> (pvalue (value_ ropts), const id) where - avalue' c v span a = a{aibalance = value (aibalance a), aebalance = value (aebalance a)} - where value = mixedAmountApplyCostValuation priceoracle styles (end span) today (error "multiBalanceReport: did not expect amount valuation to be called ") c v -- PARTIAL: should not happen - pvalue' c v span = postingApplyCostValuation priceoracle styles (end span) today c v - end = fromMaybe (error "multiBalanceReport: expected all spans to have an end date") -- XXX should not happen - . fmap (addDays (-1)) . spanEnd + -- For a Posting: convert to cost, apply valuation, then strip prices if we don't need them (See issue #1507). + pvalue v span = maybeStripPrices . postingApplyCostValuation priceoracle styles (end span) today (cost_ ropts) v + -- For an Account: Apply valuation to both the inclusive and exclusive balances. + avalue v span a = a{aibalance = value (aibalance a), aebalance = value (aebalance a)} + where value = mixedAmountApplyValuation priceoracle styles (end span) today (error "multiBalanceReport: did not expect amount valuation to be called ") v -- PARTIAL: should not happen + + maybeStripPrices = if show_costs_ ropts then id else postingStripPrices + end = maybe (error "multiBalanceReport: expected all spans to have an end date") -- PARTIAL: should not happen + (addDays (-1)) . spanEnd styles = journalCommodityStyles j -- tests diff --git a/hledger-lib/Hledger/Reports/PostingsReport.hs b/hledger-lib/Hledger/Reports/PostingsReport.hs index 600d1bcac..3ba522ff2 100644 --- a/hledger-lib/Hledger/Reports/PostingsReport.hs +++ b/hledger-lib/Hledger/Reports/PostingsReport.hs @@ -91,8 +91,9 @@ postingsReport rspec@ReportSpec{rsOpts=ropts@ReportOpts{..}} j = items fromMaybe (error' "postingsReport: expected a non-empty journal") $ -- PARTIAL: shouldn't happen reportPeriodOrJournalLastDay rspec j - -- Posting report does not use prices after valuation, so remove them. - displaypsnoprices = map (\(p,md) -> (postingStripPrices p, md)) displayps + -- Strip prices from postings if we won't need them. + displaypsnoprices = map (\(p,md) -> (maybeStripPrices p, md)) displayps + where maybeStripPrices = if show_costs_ then id else postingStripPrices -- Posting report items ready for display. items = diff --git a/hledger-lib/Hledger/Reports/ReportOptions.hs b/hledger-lib/Hledger/Reports/ReportOptions.hs index f2ee8b338..df10d586a 100644 --- a/hledger-lib/Hledger/Reports/ReportOptions.hs +++ b/hledger-lib/Hledger/Reports/ReportOptions.hs @@ -119,6 +119,7 @@ data ReportOpts = ReportOpts { ,drop_ :: Int ,row_total_ :: Bool ,no_total_ :: Bool + ,show_costs_ :: Bool -- ^ Whether to show costs for reports which normally don't show them ,pretty_tables_ :: Bool ,sort_amount_ :: Bool ,percent_ :: Bool @@ -166,6 +167,7 @@ defreportopts = ReportOpts , drop_ = 0 , row_total_ = False , no_total_ = False + , show_costs_ = False , pretty_tables_ = False , sort_amount_ = False , percent_ = False @@ -215,6 +217,7 @@ rawOptsToReportOpts rawopts = do ,drop_ = posintopt "drop" rawopts ,row_total_ = boolopt "row-total" rawopts ,no_total_ = boolopt "no-total" rawopts + ,show_costs_ = boolopt "show-costs" rawopts ,sort_amount_ = boolopt "sort-amount" rawopts ,percent_ = boolopt "percent" rawopts ,invert_ = boolopt "invert" rawopts diff --git a/hledger/Hledger/Cli/Commands/Balance.hs b/hledger/Hledger/Cli/Commands/Balance.hs index 5b880fe23..85146d707 100644 --- a/hledger/Hledger/Cli/Commands/Balance.hs +++ b/hledger/Hledger/Cli/Commands/Balance.hs @@ -388,11 +388,11 @@ balance opts@CliOpts{reportspec_=rspec} j = case reporttype_ of balanceReportAsCsv :: ReportOpts -> BalanceReport -> CSV balanceReportAsCsv opts (items, total) = ["account","balance"] : - [[accountNameDrop (drop_ opts) a, wbToText $ showMixedAmountB oneLine b] | (a, _, _, b) <- items] + [[accountNameDrop (drop_ opts) a, wbToText $ showMixedAmountB (balanceOpts False opts) b] | (a, _, _, b) <- items] ++ if no_total_ opts then [] - else [["total", wbToText $ showMixedAmountB oneLine total]] + else [["total", wbToText $ showMixedAmountB (balanceOpts False opts) total]] -- | Render a single-column balance report as plain text. balanceReportAsText :: ReportOpts -> BalanceReport -> TB.Builder @@ -461,12 +461,12 @@ renderComponent topaligned opts (acctname, depth, total) (FormatField ljust mmin DepthSpacerField -> Cell align [WideBuilder (TB.fromText $ T.replicate d " ") d] where d = maybe id min mmax $ depth * fromMaybe 1 mmin AccountField -> textCell align $ formatText ljust mmin mmax acctname - TotalField -> Cell align . pure $ showamt total + TotalField -> Cell align . pure $ showMixedAmountB dopts total _ -> Cell align [mempty] where align = if topaligned then (if ljust then TopLeft else TopRight) else (if ljust then BottomLeft else BottomRight) - showamt = showMixedAmountB noPrice{displayColour=color_ opts, displayMinWidth=mmin, displayMaxWidth=mmax} + dopts = (balanceOpts True opts){displayOneLine=False, displayMinWidth=mmin, displayMaxWidth=mmax} -- rendering multi-column balance reports @@ -482,7 +482,7 @@ multiBalanceReportAsCsv opts@ReportOpts{average_, row_total_} ++ ["average" | average_] ) : [displayName a : - map (wbToText . showMixedAmountB oneLine) + map (wbToText . showMixedAmountB (balanceOpts False opts)) (amts ++ [rowtot | row_total_] ++ [rowavg | average_]) @@ -491,7 +491,7 @@ multiBalanceReportAsCsv opts@ReportOpts{average_, row_total_} if no_total_ opts then [] else ["total" : - map (wbToText . showMixedAmountB oneLine) ( + map (wbToText . showMixedAmountB (balanceOpts False opts)) ( coltotals ++ [tot | row_total_] ++ [avg | average_] @@ -656,13 +656,19 @@ balanceReportAsTable opts@ReportOpts{average_, row_total_, balancetype_} -- console output. Amounts with more than two commodities will be elided -- unless --no-elide is used. balanceReportTableAsText :: ReportOpts -> Table T.Text T.Text MixedAmount -> TB.Builder -balanceReportTableAsText ReportOpts{..} = +balanceReportTableAsText ropts@ReportOpts{..} = Tab.renderTableB def{tableBorders=False, prettyTable=pretty_tables_} - (Tab.textCell TopLeft) (Tab.textCell TopRight) showamt - where - showamt = Cell TopRight . pure . showMixedAmountB oneLine{displayColour=color_, displayMaxWidth=mmax} - mmax = if no_elide_ then Nothing else Just 32 + (Tab.textCell TopLeft) (Tab.textCell TopRight) $ + Cell TopRight . pure . showMixedAmountB (balanceOpts True ropts) +-- | Amount display options to use for balance reports +balanceOpts :: Bool -> ReportOpts -> AmountDisplayOpts +balanceOpts isTable ReportOpts{..} = oneLine + { displayColour = isTable && color_ + , displayMaxWidth = if isTable && not no_elide_ then Just 32 else Nothing + , displayPrice = True -- multiBalanceReport strips prices from Amounts if they are not being used, + -- so we can display prices here without fear. + } tests_Balance = tests "Balance" [ diff --git a/hledger/Hledger/Cli/Commands/Close.hs b/hledger/Hledger/Cli/Commands/Close.hs index 3bab077c9..49e5a9a0d 100755 --- a/hledger/Hledger/Cli/Commands/Close.hs +++ b/hledger/Hledger/Cli/Commands/Close.hs @@ -82,14 +82,9 @@ close CliOpts{rawopts_=rawopts, reportspec_=rspec} j = do -- should we show the amount(s) on the equity posting(s) ? explicit = boolopt "explicit" rawopts - -- should we preserve cost information ? - normalise = case boolopt "show-costs" rawopts of - True -> normaliseMixedAmount - False -> normaliseMixedAmount . mixedAmountStripPrices - -- the balances to close (acctbals,_) = balanceReport rspec_ j - totalamt = maSum $ map (\(_,_,_,b) -> normalise b) acctbals + totalamt = maSum $ map (\(_,_,_,b) -> b) acctbals -- since balance assertion amounts are required to be exact, the -- amounts in opening/closing transactions should be too (#941, #1137) @@ -117,13 +112,13 @@ close CliOpts{rawopts_=rawopts, reportspec_=rspec} j = do | -- get the balances for each commodity and transaction price (a,_,_,mb) <- acctbals - , let bs = amounts $ normalise mb + , let bs = amounts $ normaliseMixedAmount mb -- mark the last balance in each commodity with True , let bs' = concat [reverse $ zip (reverse bs) (True : repeat False) | bs <- groupBy ((==) `on` acommodity) bs] , (b, islast) <- bs' ] - + -- or a final multicommodity posting transferring all balances to equity -- (print will show this as multiple single-commodity postings) ++ [posting{paccount=closingacct, pamount=if explicit then mapMixedAmount precise totalamt else missingmixedamt} | not interleaved] @@ -143,7 +138,7 @@ close CliOpts{rawopts_=rawopts, reportspec_=rspec} j = do ++ [posting{paccount=openingacct, pamount=Mixed [precise $ negate b]} | interleaved] | (a,_,_,mb) <- acctbals - , let bs = amounts $ normalise mb + , let bs = amounts $ normaliseMixedAmount mb -- mark the last balance in each commodity with the unpriced sum in that commodity (for a balance assertion) , let bs' = concat [reverse $ zip (reverse bs) (Just commoditysum : repeat Nothing) | bs <- groupBy ((==) `on` acommodity) bs diff --git a/hledger/Hledger/Cli/Commands/Register.hs b/hledger/Hledger/Cli/Commands/Register.hs index 326dfc6bd..e318ed621 100644 --- a/hledger/Hledger/Cli/Commands/Register.hs +++ b/hledger/Hledger/Cli/Commands/Register.hs @@ -93,8 +93,9 @@ postingsReportItemAsCsvRecord (_, _, _, p, b) = [idx,date,code,desc,acct,amt,bal BalancedVirtualPosting -> wrap "[" "]" VirtualPosting -> wrap "(" ")" _ -> id - amt = wbToText . showMixedAmountB oneLine $ pamount p - bal = wbToText $ showMixedAmountB oneLine b + -- Since postingsReport strips prices from all Amounts when not used, we can display prices. + amt = wbToText . showMixedAmountB oneLine{displayPrice=True} $ pamount p + bal = wbToText $ showMixedAmountB oneLine{displayPrice=True} b -- | Render a register report as plain text suitable for console output. postingsReportAsText :: CliOpts -> PostingsReport -> TL.Text @@ -107,7 +108,8 @@ postingsReportAsText opts items = TB.toLazyText $ foldMap first3 linesWithWidths -- balwidth = maximum $ 12 : map third3 linesWithWidths amtwidth = maximumStrict $ 12 : widths (map itemamt items) balwidth = maximumStrict $ 12 : widths (map itembal items) - widths = map wbWidth . concatMap (showAmountsLinesB noPrice) + -- Since postingsReport strips prices from all Amounts when not used, we can display prices. + widths = map wbWidth . concatMap (showAmountsLinesB oneLine{displayPrice=True}) itemamt (_,_,_,Posting{pamount=a},_) = amounts a itembal (_,_,_,_,a) = amounts a @@ -190,7 +192,9 @@ postingsReportItemAsText opts preferredamtwidth preferredbalwidth (mdate, mendda _ -> (id,acctwidth) amt = showAmountsLinesB dopts . (\x -> if null x then [nullamt] else x) . amounts $ pamount p bal = showAmountsLinesB dopts $ amounts b - dopts = noPrice{displayColour=color_ . rsOpts $ reportspec_ opts} + -- Since postingsReport strips prices from all Amounts when not used, we can display prices. + dopts = oneLine{displayColour=color_, displayPrice=True} + where ReportOpts{..} = rsOpts $ reportspec_ opts -- Since this will usually be called with the knot tied between this(amt|bal)width and -- preferred(amt|bal)width, make sure the former do not depend on the latter to avoid loops. thisamtwidth = maximumDef 0 $ map wbWidth amt diff --git a/hledger/test/journal/transaction-prices.test b/hledger/test/journal/transaction-prices.test index eff5c3be4..3b525f857 100644 --- a/hledger/test/journal/transaction-prices.test +++ b/hledger/test/journal/transaction-prices.test @@ -210,7 +210,19 @@ hledger -f - balance --no-total -E -1Y b >>>= 0 -# 18. the above with -B +# 18. Without -E, a should be hidden because its balance is zero, even though it has a non-zero cost. +hledger -f - balance --no-total +<<< +1/1 + a 1X @@ 1Y + a 1X @@ 1Y + a -2X @@ 1Y + b +>>> + -1Y b +>>>= 0 + +# 19. the above with -B hledger -f - balance --no-total -E -B <<< 1/1