Skip to content
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

err Not Working as (I) Expected #29

Closed
kevinmeredith opened this issue Aug 6, 2014 · 5 comments · Fixed by #203
Closed

err Not Working as (I) Expected #29

kevinmeredith opened this issue Aug 6, 2014 · 5 comments · Fixed by #203
Assignees
Milestone

Comments

@kevinmeredith
Copy link

In the following Parser:

object Foo extends JavaTokenParsers { 

  def word(x: String) = s"\\b$x\\b".r

  lazy val expr  = aSentence | something

  lazy val aSentence = noun ~ verb ~ obj

  lazy val noun   = word("noun")
  lazy val verb   = word("verb") | err("not a verb!")
  lazy val obj    = word("object")

  lazy val something = word("FOO")
}

It will parse noun verb object.

scala> Foo.parseAll(Foo.expr, "noun verb object")
res1: Foo.ParseResult[java.io.Serializable] = [1.17] parsed: ((noun~verb)~object)

But, when entering a valid noun, but an invalid verb, why won't the err("not a verb!") return an Error with that particular error message?

scala> Foo.parseAll(Foo.expr, "noun vedsfasdf")
res2: Foo.ParseResult[java.io.Serializable] =
[1.6] failure: string matching regex `\bverb\b' expected but `v' found

noun vedsfasdf
     ^

credit: Thanks to Travis Brown for explaining the need for the word function here.

@dcsobral provides a thorough explanation here - http://stackoverflow.com/questions/25147833/using-err-in-a-child-parser as to why this behavior is occurring.

I don't completely understand his answer (re-reading it a few more times), but shouldn't err(... in my above example result in that particular Error?

@gourlaysama gourlaysama added the bug label Aug 6, 2014
@gourlaysama
Copy link
Contributor

Indeed, err should probably consume whitespace.

It doesn't always matter what err consumes since, well, it produces an error. But when two alternatives both return an error, the parser has to pick one, and it picks the one that consumes the most input (favoring the second one if they consume the same amount). And err can lose when the alternative is a regex or literal parser, since those are little cheaters that consume additional whitespace.

See #25 (comment) for more reading.

As a workaround, as shown in @dcsobral's answer, you can manually consume whitespace before err every time you use it, or possibly override err yourself to do the right thing once and for all:

// just call ws(someParser) to have it handle whitespace
private def ws[T](p: Parser[T]): Parser[T] = new Parser[T] {
  def apply(in: Input) = {
    val offset = in.offset
    val start = handleWhiteSpace(in.source, offset)
    p(in.drop (start - offset))
  }
}

override def err(msg: String) = ws(super.err(msg))

...

lazy val verb   = word("verb") | err("not a verb!") // will now do the right thing

@gourlaysama gourlaysama self-assigned this Aug 6, 2014
gourlaysama added a commit to gourlaysama/scala-parser-combinators that referenced this issue Aug 8, 2014
This overrides `err` in RegexParser to make it consume whitespace just
like regex and literal. The original motivation was:

object parser extends RegexParsers {
  def num = "\\d+".r

  def twoNums =  num ~ (num | err("error!"))
}

// succeeds
parser.parseAll(twoNums, "42    721")

// fails with a parsing Failure instead of an Error
// because err doesn't consume the whitespace but the regex does.
parser.parseAll(twoNums, "42    foo")

This may change the output of some parsers that failed to parse input
(from a Failure to an Error).

Fixes scala#29
@gourlaysama gourlaysama reopened this Dec 15, 2014
@gourlaysama
Copy link
Contributor

I'm reopening this since the fix has been reverted. See #41 for more info.

@gourlaysama gourlaysama modified the milestone: 1.1.0 Dec 18, 2014
@hrj
Copy link
Contributor

hrj commented Feb 15, 2017

Hmm... If the fix was reverted because of binary incompatibility, re-applying it for 1.1.0 should be possible?

I ask because this seems to be the only issue blocking a 1.1.0 release.

@hrj
Copy link
Contributor

hrj commented Apr 20, 2017

@gourlaysama Does this require more than a reapplication of the previous patch?

@Philippus Philippus modified the milestones: 1.1.0, 1.2.0 Feb 17, 2019
@Philippus
Copy link
Member

Moved the milestone for this one, as 1.1.0 was already released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants