From fae6e49407872feb09353aa943021e4dd4569faa Mon Sep 17 00:00:00 2001
From: Simon Michael <simon@joyful.com>
Date: Tue, 26 Mar 2024 09:28:31 -1000
Subject: [PATCH] imp: simpler, clearer date parse error messages

When the error message repeated the invalid date at the end,
it was possible to misinterpret that as a suggested fix (reported in chat).

Instead, date errors (most of them) now rely on the highlighted data
excerpt above. This is also preferable since it shows the original
date as written, not a reconstruction with a possibly different format.

Should this be the policy for all error messages going forward ?
It would be easier.
Can we assume the data excerpt is always visible along with the error message ?
It isn't shown by flycheck-hledger in emacs, eg.
---
 hledger-lib/Hledger/Read/Common.hs       | 38 +++++++++++-------------
 hledger/test/errors/parseable-dates.test | 10 +++----
 hledger/test/journal/dates.test          |  6 ++--
 3 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/hledger-lib/Hledger/Read/Common.hs b/hledger-lib/Hledger/Read/Common.hs
index a3634aa73..6903a62ff 100644
--- a/hledger-lib/Hledger/Read/Common.hs
+++ b/hledger-lib/Hledger/Read/Common.hs
@@ -517,41 +517,39 @@ datep' mYear = do
     sep <- datesepchar <?> "date separator"
     d2 <- decimal <?> "month or day"
     case d1 of
-         Left y  -> fullDate startOffset y sep d2
-         Right m -> partialDate startOffset mYear m sep d2
+      Left y  -> fullDate startOffset y sep d2
+      Right m -> partialDate startOffset mYear m d2
     <?> "full or partial date"
   where
     fullDate :: Int -> Year -> Char -> Month -> TextParser m Day
-    fullDate startOffset year sep1 month = do
+    fullDate startOffset year sep month = do
       sep2 <- satisfy isDateSepChar <?> "date separator"
       day <- decimal <?> "day"
       endOffset <- getOffset
-      let dateStr = show year ++ [sep1] ++ show month ++ [sep2] ++ show day
-
-      when (sep1 /= sep2) $ customFailure $ parseErrorAtRegion startOffset endOffset $
-        "This date is malformed because the separators are different.\n"
-        ++"Please use consistent separators."
-
+      when (sep /= sep2) $ 
+        customFailure $ parseErrorAtRegion startOffset endOffset $
+          "This date has different separators, please use consistent separators."
       case fromGregorianValid year month day of
-        Nothing -> customFailure $ parseErrorAtRegion startOffset endOffset $
-                     "This date is invalid, please correct it: " ++ dateStr
+        Nothing -> 
+          customFailure $ parseErrorAtRegion startOffset endOffset $
+            "This is not a valid date, please fix it."
         Just date -> pure $! date
 
-    partialDate :: Int -> Maybe Year -> Month -> Char -> MonthDay -> TextParser m Day
-    partialDate startOffset myr month sep day = do
+    partialDate :: Int -> Maybe Year -> Month -> MonthDay -> TextParser m Day
+    partialDate startOffset myr month day = do
       endOffset <- getOffset
       case myr of
         Just year ->
           case fromGregorianValid year month day of
-            Nothing -> customFailure $ parseErrorAtRegion startOffset endOffset $
-                        "This date is invalid, please correct it: " ++ dateStr
+            Nothing -> 
+              customFailure $ parseErrorAtRegion startOffset endOffset $
+                "This is not a valid date, please fix it."
             Just date -> pure $! date
-          where dateStr = show year ++ [sep] ++ show month ++ [sep] ++ show day
 
-        Nothing -> customFailure $ parseErrorAtRegion startOffset endOffset $
-          "The partial date "++dateStr++" can not be parsed because the current year is unknown.\n"
-          ++"Consider making it a full date, or add a default year directive.\n"
-          where dateStr = show month ++ [sep] ++ show day
+        Nothing ->
+          customFailure $ parseErrorAtRegion startOffset endOffset $
+            "This partial date can not be parsed because the current year is unknown.\n"
+            ++"Please make it a full date, or add a default year directive."
 
 {-# INLINABLE datep' #-}
 
diff --git a/hledger/test/errors/parseable-dates.test b/hledger/test/errors/parseable-dates.test
index 86fe4f4bd..c76cb9119 100644
--- a/hledger/test/errors/parseable-dates.test
+++ b/hledger/test/errors/parseable-dates.test
@@ -1,12 +1,10 @@
-# * parseable error
-# ** 1. 
-$ hledger check -f parseable-dates.j
->2 /hledger: Error: .*parseable-dates.j:3:1:
+$$$ hledger check -f parseable-dates.j
+>>>2 /hledger: Error: .*parseable-dates.j:3:1:
   \|
 3 \| 2022\/1\/32
   \| \^\^\^\^\^\^\^\^\^
 
-This date is invalid, please correct it: 2022\/1\/32
+This is not a valid date, please fix it.
 
 /
->= 1
+>>>= 1
diff --git a/hledger/test/journal/dates.test b/hledger/test/journal/dates.test
index de9434823..b6dec88e7 100644
--- a/hledger/test/journal/dates.test
+++ b/hledger/test/journal/dates.test
@@ -8,7 +8,7 @@
    a  1
    b
 $ hledger -f- print
->2 /date is invalid/
+>2 /is not a valid date/
 >= 1
 
 # ** 2. too-large day
@@ -17,7 +17,7 @@ $ hledger -f- print
    a  1
    b
 $ hledger -f- print
->2 /date is invalid/
+>2 /is not a valid date/
 >= 1
 
 # ** 3. 29th feb on leap year should be ok
@@ -38,7 +38,7 @@ $ hledger -f- print
    a  1
    b
 $ hledger -f- print
->2 /date is invalid/
+>2 /is not a valid date/
 >= 1
 
 # ** 5. dates must be followed by whitespace or newline