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

Why does label act only on the first set of hints? #482

Closed
j-mie6 opened this issue Jul 7, 2022 · 2 comments · Fixed by #494
Closed

Why does label act only on the first set of hints? #482

j-mie6 opened this issue Jul 7, 2022 · 2 comments · Fixed by #494
Labels

Comments

@j-mie6
Copy link

j-mie6 commented Jul 7, 2022

When you have a parser like:

label name (optional p *> optional q) *> r

If r fails, the hints from p and q are usually reported as possible expected tokens. However, in this case ps hints are relabelled to name, but qs are not. This is clearly the expected behaviour from the library as implemented explicitly by refreshLastHints. What I'm wondering, however, is what the rationale was behind this design choice?

@mrkkrp mrkkrp added the question label Jul 8, 2022
@zuqq
Copy link

zuqq commented Jul 11, 2022

This is not exactly the same situation as in your example, but I recently also encountered a case where label was less effective than desired.

Running

{-# LANGUAGE OverloadedStrings #-}

import Data.Void (Void)
import Text.Megaparsec (Parsec, errorBundlePretty, label, parse)
import Text.Megaparsec.Char (space)

main :: IO ()
main = do
    let s :: Parsec Void String ()
        s = label "spaces" space

    let p :: Parsec Void String String
        p = label "<p>" ("A" <* s)

    case parse (p *> p) mempty "A B" of
        Left e -> putStr (errorBundlePretty e)
        Right _ -> error "Unexpected `Right _`."

against 2a46074 prints the built-in label of space in the list of hints, even though the definition of s looks like it should override this label:

1:3:
  |
1 | A B
  |   ^
unexpected 'B'
expecting <p> or white space

Not knowing how labels are implemented in megaparsec, I found this behavior surprising; it is, however, explained in the documentation of label:

-- | The parser @'label' name p@ behaves as parser @p@, but whenever the
-- parser @p@ fails /without consuming any input/, it replaces names of
-- “expected” tokens with the name @name@.
label :: String -> m a -> m a

Since s succeeds, its label is disregarded.

@mrkkrp
Copy link
Owner

mrkkrp commented Oct 21, 2022

Hey @j-mie6, that's a good catch. I think there is really no reason why we shouldn't modify both p and q in this case. I have opened a PR that implements that.

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

Successfully merging a pull request may close this issue.

3 participants