Skip to content

Commit

Permalink
fix: bal --budget --layout=tall was hiding commodity symbols
Browse files Browse the repository at this point in the history
When reports want to render amounts without commmodity symbols,
they must now use AmountDisplayOpts' new displayCommodity flag.
(Previously it was a side effect of setting displayCommodityOrder.)
  • Loading branch information
simonmichael committed Dec 8, 2023
1 parent 58c5773 commit 049aa4b
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 83 deletions.
13 changes: 4 additions & 9 deletions hledger-lib/Hledger/Data/Amount.hs
Original file line number Diff line number Diff line change
Expand Up @@ -652,12 +652,12 @@ showAmount = wbUnpack . showAmountB noColour
showAmountB :: AmountDisplayOpts -> Amount -> WideBuilder
showAmountB _ Amount{acommodity="AUTO"} = mempty
showAmountB
AmountDisplayOpts{displayCost, displayColour, displayZeroCommodity,
displayDigitGroups, displayForceDecimalMark, displayCommodityOrder}
AmountDisplayOpts{displayCommodity, displayZeroCommodity, displayDigitGroups
,displayForceDecimalMark, displayCost, displayColour}
a@Amount{astyle=style} =
color $ case ascommodityside style of
L -> showsym (wbFromText comm) space <> quantity' <> price
R -> quantity' <> showsym space (wbFromText comm) <> price
L -> (if displayCommodity then wbFromText comm <> space else mempty) <> quantity' <> price
R -> quantity' <> (if displayCommodity then space <> wbFromText comm else mempty) <> price
where
color = if displayColour && isNegativeAmount a then colorB Dull Red else id
quantity = showAmountQuantity displayForceDecimalMark $
Expand All @@ -667,11 +667,6 @@ showAmountB
| otherwise = (quantity, quoteCommoditySymbolIfNeeded $ acommodity a)
space = if not (T.null comm) && ascommodityspaced style then WideBuilder (TB.singleton ' ') 1 else mempty
price = if displayCost then showAmountPrice a else mempty
-- Show a commodity symbol and its optional space, concatenated.
-- Unless there's a commodity display order, in which case show nothing. XXX for --layout=bare, but wrong for --layout=tall
showsym l r
| isJust displayCommodityOrder = mempty
| otherwise = l <> r

-- | Colour version. For a negative amount, adds ANSI codes to change the colour,
-- currently to hard-coded red.
Expand Down
14 changes: 7 additions & 7 deletions hledger-lib/Hledger/Reports/BudgetReport.hs
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,10 @@ budgetReportAsTable
padcells = maybetranspose . fmap (fmap (uncurry paddisplaycell) . zip widths) . maybetranspose
padtr = maybetranspose . fmap (fmap (uncurry paddisplaycell) . zip trwidths) . maybetranspose

-- commodities are shown with the amounts without `layout_ == LayoutBare`
-- with --layout=bare, begin with a commodity column
prependcs cs
| layout_ /= LayoutBare = id
| otherwise = zipWith (:) cs
| layout_ == LayoutBare = zipWith (:) cs
| otherwise = id

rowToBudgetCells (PeriodicReportRow _ as rowtot rowavg) = as
++ [rowtot | row_total_ && not (null as)]
Expand All @@ -337,9 +337,9 @@ budgetReportAsTable
rowfuncs :: [CommoditySymbol] -> (BudgetShowMixed, BudgetPercBudget)
rowfuncs cs = case layout_ of
LayoutWide width ->
( pure . showMixedAmountB oneLine{displayColour=color_, displayMaxWidth=width}
( pure . showMixedAmountB oneLine{displayMaxWidth=width, displayColour=color_}
, \a -> pure . percentage a)
_ -> ( showMixedAmountLinesB noCost{displayCommodityOrder=Just cs, displayMinWidth=Nothing, displayColour=color_}
_ -> ( showMixedAmountLinesB noCost{displayCommodity=layout_/=LayoutBare, displayMinWidth=Nothing, displayColour=color_}
, \a b -> fmap (percentage' a b) cs)

showrow :: [BudgetCell] -> [(WideBuilder, BudgetDisplayRow)]
Expand Down Expand Up @@ -476,11 +476,11 @@ budgetReportAsCsv
| otherwise =
joinNames . zipWith (:) cs -- add symbols and names
. transpose -- each row becomes a list of Text quantities
. fmap (fmap wbToText . showMixedAmountLinesB oneLine{displayCommodityOrder=Just cs, displayMinWidth=Nothing}
.fromMaybe nullmixedamt)
. fmap (fmap wbToText . showMixedAmountLinesB dopts . fromMaybe nullmixedamt)
$ vals
where
cs = S.toList . foldl' S.union mempty . fmap maCommodities $ catMaybes vals
dopts = oneLine{displayCommodity=layout_ /= LayoutBare, displayCommodityOrder=Just cs, displayMinWidth=Nothing}
vals = flattentuples as
++ concat [[rowtot, budgettot] | row_total_]
++ concat [[rowavg, budgetavg] | average_]
Expand Down
20 changes: 14 additions & 6 deletions hledger/Hledger/Cli/Commands/Balance.hs
Original file line number Diff line number Diff line change
Expand Up @@ -434,8 +434,11 @@ balanceReportAsCsv opts (items, total) =

showName = accountNameDrop (drop_ opts)
renderAmount amt = wbToText $ showMixedAmountB bopts amt
where bopts = csvDisplay{displayCommodityOrder = order}
order = if layout_ opts == LayoutBare then Just (S.toList $ maCommodities amt) else Nothing
where
bopts = csvDisplay{displayCommodity=showcomm, displayCommodityOrder = commorder}
(showcomm, commorder)
| layout_ opts == LayoutBare = (False, Just $ S.toList $ maCommodities amt)
| otherwise = (True, Nothing)

-- | Render a single-column balance report as plain text.
balanceReportAsText :: ReportOpts -> BalanceReport -> TB.Builder
Expand Down Expand Up @@ -467,7 +470,7 @@ balanceReportAsText' opts ((items, total)) =
[ Cell TopRight damts
, Cell TopLeft (fmap wbFromText cs)
, Cell TopLeft (replicate (length damts - 1) mempty ++ [wbFromText dispname]) ]
where dopts = oneLine{displayColour=color_ opts, displayCommodityOrder=Just cs}
where dopts = oneLine{displayCommodity=layout_ opts /= LayoutBare, displayCommodityOrder=Just cs, displayColour=color_ opts}
cs = if mixedAmountLooksZero amt then [""] else S.toList $ maCommodities amt
dispname = T.replicate ((dep - 1) * 2) " " <> acctname
damts = showMixedAmountLinesB dopts amt
Expand Down Expand Up @@ -524,7 +527,12 @@ renderComponent topaligned oneline opts (acctname, dep, total) (FormatField ljus
| topaligned = TopRight
| ljust = BottomLeft
| otherwise = BottomRight
dopts = noCost{displayColour=color_ opts, displayOneLine=oneline, displayMinWidth=mmin, displayMaxWidth=mmax}
dopts = noCost{displayCommodity = layout_ opts /= LayoutBare
,displayOneLine = oneline
,displayMinWidth = mmin
,displayMaxWidth = mmax
,displayColour = color_ opts
}

-- rendering multi-column balance reports

Expand Down Expand Up @@ -737,12 +745,12 @@ multiBalanceRowAsWbs bopts ReportOpts{..} colspans (PeriodicReportRow _ as rowto
$ allamts
LayoutBare -> zipWith (:) (fmap wbFromText cs) -- add symbols
. transpose -- each row becomes a list of Text quantities
. fmap (showMixedAmountLinesB bopts{displayCommodityOrder=Just cs, displayMinWidth=Nothing})
. fmap (showMixedAmountLinesB bopts{displayCommodity=False, displayCommodityOrder=Just cs, displayMinWidth=Nothing})
$ allamts
LayoutTidy -> concat
. zipWith (map . addDateColumns) colspans
. fmap ( zipWith (\c a -> [wbFromText c, a]) cs
. showMixedAmountLinesB bopts{displayCommodityOrder=Just cs, displayMinWidth=Nothing})
. showMixedAmountLinesB bopts{displayCommodity=False, displayCommodityOrder=Just cs, displayMinWidth=Nothing})
$ as -- Do not include totals column or average for tidy output, as this
-- complicates the data representation and can be easily calculated
where
Expand Down
Loading

0 comments on commit 049aa4b

Please sign in to comment.