Skip to content

Commit

Permalink
imp: simpler, clearer date parse error messages
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
simonmichael committed Mar 26, 2024
1 parent 5519f4a commit fae6e49
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 29 deletions.
38 changes: 18 additions & 20 deletions hledger-lib/Hledger/Read/Common.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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' #-}

Expand Down
10 changes: 4 additions & 6 deletions hledger/test/errors/parseable-dates.test
Original file line number Diff line number Diff line change
@@ -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
6 changes: 3 additions & 3 deletions hledger/test/journal/dates.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit fae6e49

Please sign in to comment.