-
Notifications
You must be signed in to change notification settings - Fork 238
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
Failing tests for the identifier parser #815
Conversation
@@ -86,6 +86,9 @@ spec = do | |||
it "parses identifiers enclosed within backticks" $ do | |||
"`foo`" `shouldParseTo` DocIdentifier "foo" | |||
|
|||
it "doesn't parse a composed function as an identifier" $ do | |||
"'foo.bar'" `shouldParseTo` "'foo.bar'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks potentially problematic: qualified identifiers should still be identified as DocIdentifier
, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this is not a valid and qualified haskell identifier, right?
"``infix``" `shouldParseTo` "`" <> DocIdentifier "infix" <> "`" | ||
|
||
it "can parse identifiers in infix notation enclosed within single quotes" $ do | ||
"'`infix`'" `shouldParseTo` "`" <> DocIdentifier "infix" <> "`" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the most problematic of all of these test failures, as exactly this syntax is described http://haskell-haddock.readthedocs.io/en/latest/markup.html#hyperlinked-identifiers.
Here's the test output:
expected: DocAppend (DocString "`") (DocAppend (DocIdentifier "infix") (DocString "`"))
but got: DocAppend (DocString "'") (DocAppend (DocIdentifier "infix") (DocString "'"))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just saw that this has been reported in #780.
I should add a test for #848 here. |
@sjakobi It would be nice to rebase this onto the 8.8 branch. I'm pretty sure almost all of these would now pass (except perhaps If you want to wait until after 8.8 is released, that's OK too (building all the deps is a pain...). |
Hi, thank you for this PR, but Haddock now lives full-time in the GHC repository! Let me know if you feel it is still needed, and I'll migrate it. :) |
While extending the
identifier parseridentifier parser tests I found the following test cases that I thought should pass.If someone can confirm that they should pass, I'll try to fix the parser.