Skip to content

Improve default error #57

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 9, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions src/Text/Parsing/StringParser.purs
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,25 @@ type Pos = Int
type PosString = { str :: String, pos :: Pos }

-- | The type of parsing errors.
newtype ParseError = ParseError String

instance showParseError :: Show ParseError where
show (ParseError msg) = msg

derive instance eqParseError :: Eq ParseError

derive instance ordParseError :: Ord ParseError
type ParseError = { error :: String, pos :: Pos }

-- | A parser is represented as a function which takes a pair of
-- | continuations for failure and success.
newtype Parser a = Parser (PosString -> Either { pos :: Pos, error :: ParseError } { result :: a, suffix :: PosString })
newtype Parser a = Parser (PosString -> Either ParseError { result :: a, suffix :: PosString })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could potentially create a type alias for this, too:

Suggested change
newtype Parser a = Parser (PosString -> Either ParseError { result :: a, suffix :: PosString })
type ParseResult a = { result :: a, suffix :: PosString }
newtype Parser a = Parser (PosString -> Either ParseError (ParseResult a))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same record is used in several places, so this would just reduce a bit of duplication. It's not necessary, though.


-- | Run a parser by providing success and failure continuations.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These documentation comments seem a bit funky to me; they keep talking about continuations, but that's not what I interpret from this function. Am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking the same thing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think this can be removed.

unParser :: forall a. Parser a -> PosString -> Either { pos :: Pos, error :: ParseError } { result :: a, suffix :: PosString }
unParser :: forall a. Parser a -> PosString -> Either ParseError { result :: a, suffix :: PosString }
unParser (Parser p) = p

-- | Run a parser for an input string, returning either an error or a result.
runParser :: forall a. Parser a -> String -> Either ParseError a
runParser (Parser p) s = bimap _.error _.result (p { str: s, pos: 0 })
-- | Run a parser for an input string. If the parser succeeds, the
-- | result will be returned (i.e. `Right a`). If it fails, the message and
-- | the position where the parser failed will be outputted
-- | as a `String` (i.e. `Left (error <> " ; pos = " <> pos)`)
runParser :: forall a. Parser a -> String -> Either String a
runParser (Parser p) s = bimap printError _.result (p { str: s, pos: 0 })
where
printError :: ParseError -> String
printError rec = rec.error <> "; pos = " <> show rec.pos
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about having runParser be forall a. Parser a -> String -> Either ParseError a, and then having a separate printError function? That's a bit more flexible than the current definition (you can recover the current definition with lmap printError).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed in latest commits.


instance functorParser :: Functor Parser where
map f (Parser p) = Parser (map (\{ result, suffix } -> { result: f result, suffix }) <<< p)
Expand Down Expand Up @@ -93,7 +92,7 @@ instance lazyParser :: Lazy (Parser a) where

-- | Fail with the specified message.
fail :: forall a. String -> Parser a
fail msg = Parser \{ pos } -> Left { pos, error: ParseError msg }
fail error = Parser \{ pos } -> Left { pos, error }

-- | In case of error, the default behavior is to backtrack if no input was consumed.
-- |
Expand Down
12 changes: 6 additions & 6 deletions src/Text/Parsing/StringParser/CodeUnits.purs
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,14 @@ import Data.String.CodeUnits as SCU
import Data.String.Pattern (Pattern(..))
import Data.String.Regex as Regex
import Data.String.Regex.Flags (noFlags)
import Text.Parsing.StringParser (Parser(..), ParseError(..), try, fail)
import Text.Parsing.StringParser (Parser(..), try, fail)
import Text.Parsing.StringParser.Combinators (many, (<?>))

-- | Match the end of the file.
eof :: Parser Unit
eof = Parser \s ->
case s of
{ str, pos } | pos < SCU.length str -> Left { pos, error: ParseError "Expected EOF" }
{ str, pos } | pos < SCU.length str -> Left { pos, error: "Expected EOF" }
_ -> Right { result: unit, suffix: s }

-- | Match any character. This is limited by `Char` to any code points
Expand All @@ -54,14 +54,14 @@ anyChar :: Parser Char
anyChar = Parser \{ str, pos } ->
case charAt pos str of
Just chr -> Right { result: chr, suffix: { str, pos: pos + 1 } }
Nothing -> Left { pos, error: ParseError "Unexpected EOF" }
Nothing -> Left { pos, error: "Unexpected EOF" }

-- | Match any code point, including those above `0xFFFF`
anyCodePoint :: Parser SCP.CodePoint
anyCodePoint = Parser \rec@{ str, pos } ->
case SCP.codePointAt 0 (SCU.drop pos str) of
Just cp -> Right { result: cp, suffix: { str, pos: pos + SCU.length (SCP.singleton cp) } }
Nothing -> Left { pos, error: ParseError "Unexpected EOF" }
Nothing -> Left { pos, error: "Unexpected EOF" }

-- | Match any digit.
anyDigit :: Parser Char
Expand All @@ -76,7 +76,7 @@ string :: String -> Parser String
string nt = Parser \s ->
case s of
{ str, pos } | SCU.indexOf' (Pattern nt) pos str == Just pos -> Right { result: nt, suffix: { str, pos: pos + SCU.length nt } }
{ pos } -> Left { pos, error: ParseError ("Expected '" <> nt <> "'.") }
{ pos } -> Left { pos, error: "Expected '" <> nt <> "'." }

-- | Match a character satisfying the given predicate.
satisfy :: (Char -> Boolean) -> Parser Char
Expand Down Expand Up @@ -158,4 +158,4 @@ regex pat =
Just (Just matched) ->
Right { result: matched, suffix: { str, pos: pos + SCU.length matched } }
_ ->
Left { pos, error: ParseError "no match" }
Left { pos, error: "no match" }