stew / codebase

0 stars 2 forks source link

Parser.string matches any string of the same length and doesn't return the match #6

Closed hagl closed 2 years ago

hagl commented 2 years ago

I was experimenting with the parser library and run into the following problem:

2 | > Parser.run (string "true") (Tokens.fromText "test")
          ⧩
          Right ""

Looking a bit deeper I found two issues:

Parser.Tokens.take doesn't return the accumulator acc in the base case, but always an empty array:

Tokens.take : Nat -> '{Tokens e t} () -> ([t], '{Tokens e t} ())
Tokens.take n input =
  go acc loc n input =
    use Nat ==
    if n == 0 then ([], input)
    else
      handle !input
    ...

Parser.tokens doesn't make use of the passed comparison function equiv and thus accepting any string of the same length.

tokens : ([t] -> [t] -> Boolean) -> [t] -> Parser e t [t]
tokens equiv likeThis =
  parse input =
    match Throw.toEither '(Tokens.take (List.size likeThis) input) with
      Left (TrivialError loc found _) ->
        Throw.throw
          (TrivialError loc found (base.Set.singleton (Tokens likeThis)))
      Right (ts, rest)                -> (ts, rest)
  Parser parse

I've addressed both issues in the PR below. One difficulty I saw with using Tokens.take in the tokens function was that in case that equiv returned false we don't have access to the location where the parse error happened. I solved that by copying most of the structure of Tokens.take into the tokens function. Maybe you have a better solution.

The changes summarized below are available for you to review, using the following command:

pull-request.load git@github.com:stew/codebase:.parser.trunk git@github.com:hagl/unison-tmp:.prs.parser.token_pr

Updates:

 Parser.tokens : ∀ e t g1 g. ([t] ->{g1} [t] ->{g} Boolean) -> [t] -> Parser e t [t]
 ↓
 Parser.tokens : ([t] -> [t] -> Boolean) -> [t] -> Parser e t [t]
 +  unisoncomputing2021 : License
 +  authors.stew        : Author
 +  #mkhcrshb8o         : Doc

 Parser.Tokens.take : Nat -> '{Tokens e t} () ->{Throw (Error e t)} ([t], '{Tokens e t} ())
 ↓
 Parser.Tokens.take : Nat -> '{Tokens e t} () ->{Throw (Error e t)} ([t], '{Tokens e t} ())
 +  unisoncomputing2021 : License
 +  authors.stew        : Author

There were 2 auto-propagated updates.

 patch patch (added 3 updates,
deleted 3)

Added definitions:

 Parser.string.tests.ex1 : [Result] (+1 metadata)
 Parser.string.tests.ex2 : [Result] (+1 metadata)
stew commented 2 years ago

Thank you!

I've applied your changes to trunk.