journal: require a decimal point in commodity/format/D directives

A commodity directive that doesn't specify the decimal point character
increases ambiguity and the chance of misparsing numbers, especially
as it overrides all style information inferred from the journal amounts.
In some cases it caused amounts with a decimal point to be parsed as if
with a digit group separator so 1.234 became 1234.

We could augment it with extra info from the journal amounts, when available,
but it would still be possible to be ambiguous, and that won't be obvious.

A commodity directive is what we recommend to nail down the style.
It seems the simple and really only way to do this reliably is to require
an explicit decimal point character. Most folks probably do this already.

Unfortunately, it makes another potential incompatiblity with ledger and
beancount journals. But the error message will be clear and easy to
work around.

hledger-lib/Hledger/Data/Journal.hs

@@ -737,18 +737,19 @@ journalApplyCommodityStyles j@Journal{jtxns=ts, jmarketprices=mps} = j''
       fixposting p@Posting{pamount=a} = p{pamount=styleMixedAmount styles a}
       fixmarketprice mp@MarketPrice{mpamount=a} = mp{mpamount=styleAmount styles a}
 
--- | Get all the amount styles defined in this journal, either 
+-- | Get all the amount styles defined in this journal, either declared by 
--- declared by a commodity directive (preferred) or inferred from amounts,
+-- a commodity directive or inferred from amounts, as a map from symbol to style. 
--- as a map from symbol to style. 
+-- Styles declared by commodity directives take precedence, and these also are
+-- guaranteed to know their decimal point character.
 journalCommodityStyles :: Journal -> M.Map CommoditySymbol AmountStyle
 journalCommodityStyles j = declaredstyles <> inferredstyles
   where
     declaredstyles = M.mapMaybe cformat $ jcommodities j
     inferredstyles = jinferredcommodities j
 
--- | Infer a display format for each commodity based on the amounts parsed.
+-- | Collect and save inferred amount styles for each commodity based on
--- "hledger... will use the format of the first posting amount in the
+-- the posting amounts in that commodity (excluding price amounts), ie:
--- commodity, and the highest precision of all posting amounts in the commodity."
+-- "the format of the first amount, adjusted to the highest precision of all amounts".
 journalInferCommodityStyles :: Journal -> Journal
 journalInferCommodityStyles j =
   j{jinferredcommodities =

hledger-lib/Hledger/Read/JournalReader.hs

@@ -274,15 +274,21 @@ commoditydirectivep = try commoditydirectiveonelinep <|> commoditydirectivemulti
 --
 -- >>> Right _ <- rejp commoditydirectiveonelinep "commodity $1.00"
 -- >>> Right _ <- rejp commoditydirectiveonelinep "commodity $1.00 ; blah\n"
-commoditydirectiveonelinep :: Monad m => JournalParser m ()
+commoditydirectiveonelinep :: Monad m => ErroringJournalParser m ()
 commoditydirectiveonelinep = do
   string "commodity"
   lift (skipSome spacenonewline)
+  pos <- getPosition
   Amount{acommodity,astyle} <- amountp
   lift (skipMany spacenonewline)
   _ <- followingcommentp <|> (lift eolof >> return "")
-  let comm = Commodity{csymbol=acommodity, cformat=Just astyle}
+  let comm = Commodity{csymbol=acommodity, cformat=Just $ dbg2 "style from commodity directive" astyle}
-  modify' (\j -> j{jcommodities=M.insert acommodity comm $ jcommodities j})
+  if asdecimalpoint astyle == Nothing
+  then parserErrorAt pos pleaseincludedecimalpoint
+  else modify' (\j -> j{jcommodities=M.insert acommodity comm $ jcommodities j})
+
+pleaseincludedecimalpoint :: String
+pleaseincludedecimalpoint = "to avoid ambiguity, please include a decimal point in commodity directives"
 
 -- | Parse a multi-line commodity directive, containing 0 or more format subdirectives.
 --
@@ -309,7 +315,10 @@ formatdirectivep expectedsym = do
   Amount{acommodity,astyle} <- amountp
   _ <- followingcommentp <|> (lift eolof >> return "")
   if acommodity==expectedsym
-    then return astyle
+    then 
+      if asdecimalpoint astyle == Nothing
+      then parserErrorAt pos pleaseincludedecimalpoint
+      else return $ dbg2 "style from format subdirective" astyle
     else parserErrorAt pos $
          printf "commodity directive symbol \"%s\" and format directive symbol \"%s\" should be the same" expectedsym acommodity
 
@@ -395,13 +404,16 @@ defaultyeardirectivep = do
   failIfInvalidYear y
   setYear y'
 
-defaultcommoditydirectivep :: Monad m => JournalParser m ()
+defaultcommoditydirectivep :: Monad m => ErroringJournalParser m ()
 defaultcommoditydirectivep = do
   char 'D' <?> "default commodity"
   lift (skipSome spacenonewline)
-  Amount{..} <- amountp
+  pos <- getPosition
+  Amount{acommodity,astyle} <- amountp
   lift restofline
-  setDefaultCommodityAndStyle (acommodity, astyle)
+  if asdecimalpoint astyle == Nothing
+  then parserErrorAt pos pleaseincludedecimalpoint
+  else setDefaultCommodityAndStyle (acommodity, astyle)
 
 marketpricedirectivep :: Monad m => JournalParser m MarketPrice
 marketpricedirectivep = do

hledger-lib/hledger_journal.m4.md

@@ -799,9 +799,7 @@ line containing just `end comment` ends it. See [comments](#comments).
 
 ### commodity directive
 
-The `commodity` directive predefines commodities (currently this is just informational),
+The `commodity` directive declares commodities which may be used in the journal (though currently we do not enforce this).
-and also it may define the display format for amounts in this commodity (overriding the automatically inferred format).
-
 It may be written on a single line, like this:
 
 ```journal
@@ -827,6 +825,12 @@ commodity INR
   format INR 9,99,99,999.00
 ```
 
+Commodity directives have a second purpose: they define the standard display format for amounts in the commodity. 
+Normally the display format is inferred from journal entries, but this can be unpredictable; 
+declaring it with a commodity directive overrides this and removes ambiguity. 
+Towards this end, amounts in commodity directives must always be written with a decimal point 
+(a period or comma, followed by 0 or more decimal digits). 
+
 ### Default commodity
 
 The D directive sets a default commodity (and display format), to be used for amounts without a commodity symbol (ie, plain numbers).
@@ -843,6 +847,8 @@ D $1,000.00
   b
 ```
 
+As with the `commodity` directive, the amount must always be written with a decimal point. 
+
 ### Default year
 
 You can set a default year to be used for subsequent dates which don't

tests/journal/numbers.test

@@ -126,26 +126,17 @@ commodity €1,000.00
 >>>2
 >>>=0
 
-# 11. No decimals but have hint from commodity directive with groups
+# 11. Commodity directive requires a decimal point
 hledger bal -f -
 <<<
-commodity 1,000,000 EUR
+commodity 1000 EUR
+>>>2 /please include a decimal point/
+>>>=1
 
-2017/1/1
+# 12. Commodity directive with zero precision
-	a   1,000 EUR
-	b  -1,000.00 EUR
->>>
-           1,000 EUR  a
-          -1,000 EUR  b
---------------------
-                   0
->>>2
->>>=0
-
-# 12. No decimals but have hint from commodity directive with zero precision
 hledger bal -f -
 <<<
-commodity 100 EUR
+commodity 100. EUR
 
 2017/1/1
 	a   1,000 EUR