csv: combine amount assignments better, fix 1.17.1 regression (#1226)

hledger-lib/Hledger/Read/CsvReader.hs

@@ -867,34 +867,41 @@ transactionFromCsvRecord sourcepos rules record = t
 -- If there's multiple non-zeros, or no non-zeros but multiple zeros, it throws an error.
 getAmount :: CsvRules -> CsvRecord -> String -> Bool -> Int -> Maybe MixedAmount
 getAmount rules record currency p1IsVirtual n =
-  -- The corner cases are tricky here.
+  -- Warning, many tricky corner cases here.
+  -- docs: hledger_csv.m4.md #### amount
+  -- tests: tests/csv.test ~ 13, 31-34
   let
     unnumberedfieldnames = ["amount","amount-in","amount-out"]
+
+    -- amount field names which can affect this posting
     fieldnames = map (("amount"++show n)++) ["","-in","-out"]
                  -- For posting 1, also recognise the old amount/amount-in/amount-out names.
                  -- For posting 2, the same but only if posting 1 needs balancing.
                  ++ if n==1 || n==2 && not p1IsVirtual then unnumberedfieldnames else []
-    nonemptyamounts = [(f,a') | f <- fieldnames
-                     , Just v@(_:_) <- [strip . renderTemplate rules record <$> hledgerField rules record f]
-                     , let a = parseAmount rules record currency v
-                       -- With amount/amount-in/amount-out, in posting 2,
-                       -- flip the sign and convert to cost, as they did before 1.17
-                     , let a' = if f `elem` unnumberedfieldnames && n==2 then costOfMixedAmount (-a) else a
-                     ]
-    -- If there's more than one non-empty amount, ignore the zeros.
-    -- Unless they're all zeros.
-    amounts
-      | length nonemptyamounts <= 1 = nonemptyamounts
-      | otherwise =
-          if null nonzeros then zeros else nonzeros
-          where (zeros,nonzeros) = partition (isZeroMixedAmount . snd) nonemptyamounts
 
-    -- if there's "amount" and "amountN"s, just discard the former
-    amounts'
-      | length amounts > 1 = filter ((/="amount").fst) amounts
-      | otherwise          = amounts
+    -- assignments to any of these field names with non-empty values
+    assignments = [(f,a') | f <- fieldnames
+                          , Just v@(_:_) <- [strip . renderTemplate rules record <$> hledgerField rules record f]
+                          , let a = parseAmount rules record currency v
+                          -- With amount/amount-in/amount-out, in posting 2,
+                          -- flip the sign and convert to cost, as they did before 1.17
+                          , let a' = if f `elem` unnumberedfieldnames && n==2 then costOfMixedAmount (-a) else a
+                          ]
 
-  in case amounts' of
+    -- if any of the numbered field names are present, discard all the unnumbered ones
+    assignments' | any isnumbered assignments = filter isnumbered assignments
+                 | otherwise                  = assignments
+      where
+        isnumbered (f,_) = any (flip elem ['0'..'9']) f
+
+    -- if there's more than one value and only some are zeros, discard the zeros
+    assignments''
+      | length assignments' > 1 && not (null nonzeros) = nonzeros
+      | otherwise                                      = assignments'
+      where nonzeros = filter (not . isZeroMixedAmount . snd) assignments'
+
+  in case -- dbg0 ("amounts for posting "++show n)
+          assignments'' of
       [] -> Nothing
       [(f,a)] | "-out" `isSuffixOf` f -> Just (-a)  -- for -out fields, flip the sign
       [(_,a)] -> Just a

hledger-lib/hledger_csv.m4.md

@@ -427,17 +427,28 @@ a default account name will be chosen (like "expenses:unknown" or "income:unknow
 #### amount
 
 `amountN` sets posting N's amount. 
-If the CSV uses separate fields for debit and credit amounts, you can
+If the CSV uses separate fields for inflows and outflows, you can
 use `amountN-in` and `amountN-out` instead.
+By assigning to `amount1`, `amount2`, ... etc. you can generate anywhere
+from 0 to 9 postings.
 
-An unnumbered form of these rules is also supported:
-`amount` or `amount-in`/`amount-out` sets the amount for both posting 1
-and (negated, and converted if there's a [transaction price](journal.html#transaction-prices))
-posting 2.
-This is for compatibility with pre-hledger-1.17 csv rules files,
-and can still be convenient for simple cases.
+There is also an older, unnumbered form of these names, suitable for
+2-posting transactions, which sets both posting 1's and (negated) posting 2's amount:
+`amount`, or `amount-in` and `amount-out`.
+This is still supported 
+because it keeps pre-hledger-1.17 csv rules files working, 
+and because it can be more succinct,
+and because it converts posting 2's amount to cost if there's a
+[transaction price](journal.html#transaction-prices), which can be useful.
+
+If you have an existing rules file using the unnumbered form, you
+might want to use the numbered form in certain conditional blocks,
+without having to update and retest all the old rules. 
+To facilitate this, 
+posting 1 ignores `amount`/`amount-in`/`amount-out` if any of `amount1`/`amount1-in`/`amount1-out` are assigned,
+and posting 2 ignores them if any of `amount2`/`amount2-in`/`amount2-out` are assigned,
+avoiding conflicts.
 
-If any numbered `amountN`/`amountN-in`/`amountN-out` fields are present, `amount` is ignored.
 
 #### currency
 

tests/csv.test

@@ -639,7 +639,7 @@ $  ./csvtest.sh
 
 >=0
 
-# 33. The old amount rules convert amount1 to cost in posting 2:
+# 33. The unnumbered amount rule converts posting 2's amount to cost.
 <
 2020-01-01, 1
 RULES
@@ -652,6 +652,19 @@ $  ./csvtest.sh
 
 >=0
 
+# 34. For a given posting, any numbered amount rule disables all unnumbered amount rules.
+# Here, amount-out is used for posting 1, but ignored for posting 2. (#1226)
+<
+2020-01-01,1,1
+RULES
+fields date, amount-out, amount2
+$  ./csvtest.sh
+2020-01-01
+    income:unknown                -1
+    expenses:unknown               1
+
+>=0
+
 ## . 
 #<
 #$  ./csvtest.sh