lib: refactor, prepare to drop showTransactionUnelided functionality

It seems we don't use it at all, and ony says it's not robust with
prices either.

Merge remote-tracking branch 'ony/chores/drop-elide-in-showTransaction'
This commit is contained in:
Simon Michael 2019-11-18 17:27:30 -08:00
commit 68b1cacf0f
9 changed files with 74 additions and 149 deletions

View File

@ -38,7 +38,7 @@ main = do
q = queryFromOpts d ropts q = queryFromOpts d ropts
ts = filter (q `matchesTransaction`) $ jtxns $ journalSelectingAmountFromOpts ropts j ts = filter (q `matchesTransaction`) $ jtxns $ journalSelectingAmountFromOpts ropts j
ts' = map transactionSwapDates ts ts' = map transactionSwapDates ts
mapM_ (putStrLn . showTransactionUnelided) ts' mapM_ (putStrLn . showTransaction) ts'
transactionSwapDates :: Transaction -> Transaction transactionSwapDates :: Transaction -> Transaction
transactionSwapDates t@Transaction{tdate2=Nothing} = t transactionSwapDates t@Transaction{tdate2=Nothing} = t

View File

@ -44,6 +44,7 @@ module Hledger.Data.Transaction (
-- payeeAndNoteFromDescription, -- payeeAndNoteFromDescription,
-- * rendering -- * rendering
showTransaction, showTransaction,
showTransactionOneLineAmounts,
showTransactionUnelided, showTransactionUnelided,
showTransactionUnelidedOneLineAmounts, showTransactionUnelidedOneLineAmounts,
-- showPostingLine, -- showPostingLine,
@ -146,40 +147,29 @@ The output will be parseable journal syntax.
To facilitate this, postings with explicit multi-commodity amounts To facilitate this, postings with explicit multi-commodity amounts
are displayed as multiple similar postings, one per commodity. are displayed as multiple similar postings, one per commodity.
(Normally does not happen with this function). (Normally does not happen with this function).
If there are multiple postings, all with explicit amounts,
and the transaction appears obviously balanced
(postings sum to 0, without needing to infer conversion prices),
the last posting's amount will not be shown.
-} -}
-- XXX why that logic ?
-- XXX where is/should this be still used ?
-- XXX rename these, after amount expressions/mixed posting amounts lands
-- eg showTransactionSimpleAmountsElidingLast, showTransactionSimpleAmounts, showTransaction
showTransaction :: Transaction -> String showTransaction :: Transaction -> String
showTransaction = showTransactionHelper True False showTransaction = showTransactionHelper False
-- | Like showTransaction, but does not change amounts' explicitness. -- | Deprecated alias for 'showTransaction'
-- Explicit amounts are shown and implicit amounts are not.
-- The output will be parseable journal syntax.
-- To facilitate this, postings with explicit multi-commodity amounts
-- are displayed as multiple similar postings, one per commodity.
-- Most often, this is the one you want to use.
showTransactionUnelided :: Transaction -> String showTransactionUnelided :: Transaction -> String
showTransactionUnelided = showTransactionHelper False False showTransactionUnelided = showTransaction -- TODO: drop it
-- | Like showTransactionUnelided, but explicit multi-commodity amounts -- | Like showTransaction, but explicit multi-commodity amounts
-- are shown on one line, comma-separated. In this case the output will -- are shown on one line, comma-separated. In this case the output will
-- not be parseable journal syntax. -- not be parseable journal syntax.
showTransactionUnelidedOneLineAmounts :: Transaction -> String showTransactionOneLineAmounts :: Transaction -> String
showTransactionUnelidedOneLineAmounts = showTransactionHelper False True showTransactionOneLineAmounts = showTransactionHelper True
-- | Deprecated alias for 'showTransactionOneLineAmounts'
showTransactionUnelidedOneLineAmounts = showTransactionOneLineAmounts -- TODO: drop it
-- | Helper for showTransaction*. -- | Helper for showTransaction*.
showTransactionHelper :: Bool -> Bool -> Transaction -> String showTransactionHelper :: Bool -> Transaction -> String
showTransactionHelper elide onelineamounts t = showTransactionHelper onelineamounts t =
unlines $ [descriptionline] unlines $ [descriptionline]
++ newlinecomments ++ newlinecomments
++ (postingsAsLines elide onelineamounts t (tpostings t)) ++ (postingsAsLines onelineamounts (tpostings t))
++ [""] ++ [""]
where where
descriptionline = rstrip $ concat [date, status, code, desc, samelinecomment] descriptionline = rstrip $ concat [date, status, code, desc, samelinecomment]
@ -211,10 +201,6 @@ renderCommentLines t =
-- --
-- Explicit amounts are shown, any implicit amounts are not. -- Explicit amounts are shown, any implicit amounts are not.
-- --
-- Setting elide to true forces the last posting's amount to be implicit, if:
-- there are other postings, all with explicit amounts, and the transaction
-- appears balanced.
--
-- Postings with multicommodity explicit amounts are handled as follows: -- Postings with multicommodity explicit amounts are handled as follows:
-- if onelineamounts is true, these amounts are shown on one line, -- if onelineamounts is true, these amounts are shown on one line,
-- comma-separated, and the output will not be valid journal syntax. -- comma-separated, and the output will not be valid journal syntax.
@ -227,11 +213,8 @@ renderCommentLines t =
-- Posting amounts will be aligned with each other, starting about 4 columns -- Posting amounts will be aligned with each other, starting about 4 columns
-- beyond the widest account name (see postingAsLines for details). -- beyond the widest account name (see postingAsLines for details).
-- --
postingsAsLines :: Bool -> Bool -> Transaction -> [Posting] -> [String] postingsAsLines :: Bool -> [Posting] -> [String]
postingsAsLines elide onelineamounts t ps postingsAsLines onelineamounts ps = concatMap (postingAsLines False onelineamounts ps) ps
| elide && length ps > 1 && all hasAmount ps && isTransactionBalanced Nothing t -- imprecise balanced check
= concatMap (postingAsLines False onelineamounts ps) (init ps) ++ postingAsLines True onelineamounts ps (last ps)
| otherwise = concatMap (postingAsLines False onelineamounts ps) ps
-- | Render one posting, on one or more lines, suitable for `print` output. -- | Render one posting, on one or more lines, suitable for `print` output.
-- There will be an indented account name, plus one or more of status flag, -- There will be an indented account name, plus one or more of status flag,
@ -417,7 +400,7 @@ balanceTransactionHelper mstyles t = do
sep = if not (null rmsg) && not (null bvmsg) then "; " else "" :: String sep = if not (null rmsg) && not (null bvmsg) then "; " else "" :: String
annotateErrorWithTransaction :: Transaction -> String -> String annotateErrorWithTransaction :: Transaction -> String -> String
annotateErrorWithTransaction t s = intercalate "\n" [showGenericSourcePos $ tsourcepos t, s, showTransactionUnelided t] annotateErrorWithTransaction t s = intercalate "\n" [showGenericSourcePos $ tsourcepos t, s, showTransaction t]
-- | Infer up to one missing amount for this transactions's real postings, and -- | Infer up to one missing amount for this transactions's real postings, and
-- likewise for its balanced virtual postings, if needed; or return an error -- likewise for its balanced virtual postings, if needed; or return an error
@ -579,39 +562,6 @@ tests_Transaction =
tests tests
"Transaction" "Transaction"
[ tests [ tests
"showTransactionUnelided"
[ showTransactionUnelided nulltransaction `is` "0000/01/01\n\n"
, showTransactionUnelided
nulltransaction
{ tdate = parsedate "2012/05/14"
, tdate2 = Just $ parsedate "2012/05/15"
, tstatus = Unmarked
, tcode = "code"
, tdescription = "desc"
, tcomment = "tcomment1\ntcomment2\n"
, ttags = [("ttag1", "val1")]
, tpostings =
[ nullposting
{ pstatus = Cleared
, paccount = "a"
, pamount = Mixed [usd 1, hrs 2]
, pcomment = "\npcomment2\n"
, ptype = RegularPosting
, ptags = [("ptag1", "val1"), ("ptag2", "val2")]
}
]
} `is`
unlines
[ "2012/05/14=2012/05/15 (code) desc ; tcomment1"
, " ; tcomment2"
, " * a $1.00"
, " ; pcomment2"
, " * a 2.00h"
, " ; pcomment2"
, ""
]
]
, tests
"postingAsLines" "postingAsLines"
[ postingAsLines False False [posting] posting `is` [""] [ postingAsLines False False [posting] posting `is` [""]
, let p = , let p =
@ -651,55 +601,43 @@ tests_Transaction =
"postingsAsLines" "postingsAsLines"
[ test "null-transaction" $ [ test "null-transaction" $
let t = nulltransaction let t = nulltransaction
in postingsAsLines True False t (tpostings t) `is` [] in postingsAsLines False (tpostings t) `is` []
, test "implicit-amount-elide-false" $ , test "implicit-amount" $
let t = timp let t = timp
in postingsAsLines False False t (tpostings t) `is` in postingsAsLines False (tpostings t) `is`
[ " a $1.00" [ " a $1.00"
, " b" -- implicit amount remains implicit , " b" -- implicit amount remains implicit
] ]
, test "implicit-amount-elide-true" $ , test "explicit-amounts" $
let t = timp
in postingsAsLines True False t (tpostings t) `is`
[ " a $1.00"
, " b" -- implicit amount remains implicit
]
, test "explicit-amounts-elide-false" $
let t = texp let t = texp
in postingsAsLines False False t (tpostings t) `is` in postingsAsLines False (tpostings t) `is`
[ " a $1.00" [ " a $1.00"
, " b $-1.00" -- both amounts remain explicit , " b $-1.00"
] ]
, test "explicit-amounts-elide-true" $ , test "one-explicit-amount" $
let t = texp
in postingsAsLines True False t (tpostings t) `is`
[ " a $1.00"
, " b" -- explicit amount is made implicit
]
, test "one-explicit-amount-elide-true" $
let t = texp1 let t = texp1
in postingsAsLines True False t (tpostings t) `is` in postingsAsLines False (tpostings t) `is`
[ " (a) $1.00" -- explicit amount remains explicit since only one posting [ " (a) $1.00"
] ]
, test "explicit-amounts-two-commodities-elide-true" $ , test "explicit-amounts-two-commodities" $
let t = texp2 let t = texp2
in postingsAsLines True False t (tpostings t) `is` in postingsAsLines False (tpostings t) `is`
[ " a $1.00" [ " a $1.00"
, " b" -- explicit amount is made implicit since txn is explicitly balanced , " b -1.00h @ $1.00"
] ]
, test "explicit-amounts-not-explicitly-balanced-elide-true" $ , test "explicit-amounts-not-explicitly-balanced" $
let t = texp2b let t = texp2b
in postingsAsLines True False t (tpostings t) `is` in postingsAsLines False (tpostings t) `is`
[ " a $1.00" [ " a $1.00"
, " b -1.00h" -- explicit amount remains explicit since a conversion price would have be inferred to balance , " b -1.00h"
] ]
, test "implicit-amount-not-last" $ , test "implicit-amount-not-last" $
let t = t3 let t = t3
in postingsAsLines True False t (tpostings t) `is` in postingsAsLines False (tpostings t) `is`
[" a $1.00", " b", " c $-1.00"] [" a $1.00", " b", " c $-1.00"]
, _test "ensure-visibly-balanced" $ , _test "ensure-visibly-balanced" $
let t = t4 let t = t4
in postingsAsLines False False t (tpostings t) `is` in postingsAsLines False (tpostings t) `is`
[" a $-0.01", " b $0.005", " c $0.005"] [" a $-0.01", " b $0.005", " c $0.005"]
] ]
, tests , tests
@ -712,26 +650,37 @@ tests_Transaction =
] ]
, tests , tests
"showTransaction" "showTransaction"
[ test "show a balanced transaction, eliding last amount" $ [ showTransaction nulltransaction `is` "0000/01/01\n\n"
let t = , showTransaction
Transaction nulltransaction
0 { tdate = parsedate "2012/05/14"
"" , tdate2 = Just $ parsedate "2012/05/15"
nullsourcepos , tstatus = Unmarked
(parsedate "2007/01/28") , tcode = "code"
Nothing , tdescription = "desc"
Unmarked , tcomment = "tcomment1\ntcomment2\n"
"" , ttags = [("ttag1", "val1")]
"coopportunity" , tpostings =
"" [ nullposting
[] { pstatus = Cleared
[ posting {paccount = "expenses:food:groceries", pamount = Mixed [usd 47.18], ptransaction = Just t} , paccount = "a"
, posting {paccount = "assets:checking", pamount = Mixed [usd (-47.18)], ptransaction = Just t} , pamount = Mixed [usd 1, hrs 2]
, pcomment = "\npcomment2\n"
, ptype = RegularPosting
, ptags = [("ptag1", "val1"), ("ptag2", "val2")]
}
] ]
in showTransaction t `is` } `is`
unlines unlines
["2007/01/28 coopportunity", " expenses:food:groceries $47.18", " assets:checking", ""] [ "2012/05/14=2012/05/15 (code) desc ; tcomment1"
, test "show a balanced transaction, no eliding" $ , " ; tcomment2"
, " * a $1.00"
, " ; pcomment2"
, " * a 2.00h"
, " ; pcomment2"
, ""
]
, test "show a balanced transaction" $
(let t = (let t =
Transaction Transaction
0 0
@ -747,14 +696,13 @@ tests_Transaction =
[ posting {paccount = "expenses:food:groceries", pamount = Mixed [usd 47.18], ptransaction = Just t} [ posting {paccount = "expenses:food:groceries", pamount = Mixed [usd 47.18], ptransaction = Just t}
, posting {paccount = "assets:checking", pamount = Mixed [usd (-47.18)], ptransaction = Just t} , posting {paccount = "assets:checking", pamount = Mixed [usd (-47.18)], ptransaction = Just t}
] ]
in showTransactionUnelided t) `is` in showTransaction t) `is`
(unlines (unlines
[ "2007/01/28 coopportunity" [ "2007/01/28 coopportunity"
, " expenses:food:groceries $47.18" , " expenses:food:groceries $47.18"
, " assets:checking $-47.18" , " assets:checking $-47.18"
, "" , ""
]) ])
-- document some cases that arise in debug/testing:
, test "show an unbalanced transaction, should not elide" $ , test "show an unbalanced transaction, should not elide" $
(showTransaction (showTransaction
(txnTieKnot $ (txnTieKnot $
@ -778,22 +726,6 @@ tests_Transaction =
, " assets:checking $-47.19" , " assets:checking $-47.19"
, "" , ""
]) ])
, test "show an unbalanced transaction with one posting, should not elide" $
(showTransaction
(txnTieKnot $
Transaction
0
""
nullsourcepos
(parsedate "2007/01/28")
Nothing
Unmarked
""
"coopportunity"
""
[]
[posting {paccount = "expenses:food:groceries", pamount = Mixed [usd 47.18]}])) `is`
(unlines ["2007/01/28 coopportunity", " expenses:food:groceries $47.18", ""])
, test "show a transaction with one posting and a missing amount" $ , test "show a transaction with one posting and a missing amount" $
(showTransaction (showTransaction
(txnTieKnot $ (txnTieKnot $

View File

@ -62,7 +62,7 @@ tsDraw UIState{aopts=UIOpts{cliopts_=copts@CliOpts{reportopts_=ropts}}
where where
maincontent = Widget Greedy Greedy $ do maincontent = Widget Greedy Greedy $ do
render $ defaultLayout toplabel bottomlabel $ str $ render $ defaultLayout toplabel bottomlabel $ str $
showTransactionUnelidedOneLineAmounts $ showTransactionOneLineAmounts $
-- (if real_ ropts then filterTransactionPostings (Real True) else id) -- filter postings by --real -- (if real_ ropts then filterTransactionPostings (Real True) else id) -- filter postings by --real
t t
where where

View File

@ -371,7 +371,7 @@ showDefault s = " [" ++ s ++ "]"
journalAddTransaction :: Journal -> CliOpts -> Transaction -> IO Journal journalAddTransaction :: Journal -> CliOpts -> Transaction -> IO Journal
journalAddTransaction j@Journal{jtxns=ts} opts t = do journalAddTransaction j@Journal{jtxns=ts} opts t = do
let f = journalFilePath j let f = journalFilePath j
appendToJournalFileOrStdout f $ showTransactionUnelided t appendToJournalFileOrStdout f $ showTransaction t
-- unelided shows all amounts explicitly, in case there's a price, cf #283 -- unelided shows all amounts explicitly, in case there's a price, cf #283
when (debug_ opts > 0) $ do when (debug_ opts > 0) $ do
putStrLn $ printf "\nAdded transaction to %s:" f putStrLn $ printf "\nAdded transaction to %s:" f

View File

@ -45,7 +45,7 @@ checkdates CliOpts{rawopts_=rawopts,reportopts_=ropts} j = do
(show $ date previous) (show $ date previous)
(show $ date error) (show $ date error)
(show $ tsourcepos error) (show $ tsourcepos error)
(showTransactionUnelided error) (showTransaction error)
data FoldAcc a b = FoldAcc data FoldAcc a b = FoldAcc
{ fa_error :: Maybe a { fa_error :: Maybe a

View File

@ -21,7 +21,6 @@ closemode = hledgerCommandMode
$(embedFileRelative "Hledger/Cli/Commands/Close.txt") $(embedFileRelative "Hledger/Cli/Commands/Close.txt")
[flagNone ["opening"] (setboolopt "opening") "show just opening transaction" [flagNone ["opening"] (setboolopt "opening") "show just opening transaction"
,flagNone ["closing"] (setboolopt "closing") "show just closing transaction" ,flagNone ["closing"] (setboolopt "closing") "show just closing transaction"
-- ,flagNone ["explicit","x"] (setboolopt "explicit") "show all amounts explicitly"
] ]
[generalflagsgroup1] [generalflagsgroup1]
hiddenflags hiddenflags
@ -88,12 +87,6 @@ close CliOpts{rawopts_=rawopts, reportopts_=ropts} j = do
} }
] ]
-- With -x, show all amounts explicitly (ie, also in the balancing equity posting(s)). when closing $ putStr $ showTransaction (nulltransaction{tdate=closingdate, tdescription="closing balances", tpostings=closingps})
-- print also does it for -B; I think that isn't needed here. when opening $ putStr $ showTransaction (nulltransaction{tdate=openingdate, tdescription="opening balances", tpostings=openingps})
-- showtxn | boolopt "explicit" rawopts = showTransactionUnelided
-- | otherwise = showTransaction
showtxn = showTransactionUnelided
when closing $ putStr $ showtxn (nulltransaction{tdate=closingdate, tdescription="closing balances", tpostings=closingps})
when opening $ putStr $ showtxn (nulltransaction{tdate=openingdate, tdescription="opening balances", tpostings=openingps})

View File

@ -50,7 +50,7 @@ importcmd opts@CliOpts{rawopts_=rawopts,inputopts_=iopts} j = do
printf "; would import %d new transactions from %s:\n\n" (length newts) inputstr printf "; would import %d new transactions from %s:\n\n" (length newts) inputstr
-- TODO how to force output here ? -- TODO how to force output here ?
-- length (jtxns newj) `seq` print' opts{rawopts_=("explicit",""):rawopts} newj -- length (jtxns newj) `seq` print' opts{rawopts_=("explicit",""):rawopts} newj
mapM_ (putStr . showTransactionUnelided) newts mapM_ (putStr . showTransaction) newts
newts | catchup -> do newts | catchup -> do
printf "marked %s as caught up, skipping %d unimported transactions\n\n" inputstr (length newts) printf "marked %s as caught up, skipping %d unimported transactions\n\n" inputstr (length newts)
newts -> do newts -> do

View File

@ -59,7 +59,7 @@ printEntries opts@CliOpts{reportopts_=ropts} j = do
writeOutput opts $ render $ entriesReport ropts' q j writeOutput opts $ render $ entriesReport ropts' q j
entriesReportAsText :: CliOpts -> EntriesReport -> String entriesReportAsText :: CliOpts -> EntriesReport -> String
entriesReportAsText opts = concatMap (showTransactionUnelided . gettxn) entriesReportAsText opts = concatMap (showTransaction . gettxn)
where where
gettxn | useexplicittxn = id -- use fully inferred amounts & txn prices gettxn | useexplicittxn = id -- use fully inferred amounts & txn prices
| otherwise = originalTransaction -- use original as-written amounts/txn prices | otherwise = originalTransaction -- use original as-written amounts/txn prices
@ -162,7 +162,7 @@ printMatch CliOpts{reportopts_=ropts} j desc = do
let q = queryFromOpts d ropts let q = queryFromOpts d ropts
case similarTransaction' j q desc of case similarTransaction' j q desc of
Nothing -> putStrLn "no matches found." Nothing -> putStrLn "no matches found."
Just t -> putStr $ showTransactionUnelided t Just t -> putStr $ showTransaction t
where where
-- Identify the closest recent match for this description in past transactions. -- Identify the closest recent match for this description in past transactions.

View File

@ -132,7 +132,7 @@ diffTxn j t t' =
diffs = map mapDiff $ D.getDiff source changed' diffs = map mapDiff $ D.getDiff source changed'
source | Just contents <- lookup fp $ jfiles j = map T.unpack . drop (line-1) . take line' $ T.lines contents source | Just contents <- lookup fp $ jfiles j = map T.unpack . drop (line-1) . take line' $ T.lines contents
| otherwise = [] | otherwise = []
changed = lines $ showTransactionUnelided t' changed = lines $ showTransaction t'
changed' | null changed = changed changed' | null changed = changed
| null $ last changed = init changed | null $ last changed = init changed
| otherwise = changed | otherwise = changed