[Textual] Parse constants with parens#7391
Conversation
|
I'm not sure this is better. I would rather have an inconsistent syntax with a single way of doing things, than a partially consistent syntax with non-canonical ways of doing things. |
|
I don't care either way. My justification is that for constants we mostly reuse Haskell syntax and there parsing of redundant parens is mostly supported: If any of the reviewers don't like this PR, feel free to close it. |
| @@ -145,7 +145,7 @@ conBLS12_381_G2_Element = do | |||
| -- | Parser for constants of the given type. | |||
| constantOf :: ExpectParens -> DefaultUni (Esc a) -> Parser a | |||
| constantOf expectParens uni = | |||
There was a problem hiding this comment.
I think the expectParens parameter is useless now?
There was a problem hiding this comment.
Nope, we still can't allow (con data I 42), it has to be (con data (I 42)).
While (con (list data) [I 42]) is fine.
So the distinction still exists.
| constantOf :: ExpectParens -> DefaultUni (Esc a) -> Parser a | ||
| constantOf expectParens uni = | ||
| case uni of | ||
| try (trailingWhitespace . inParens $ constantOf ExpectParensNo uni) <|> case uni of |
There was a problem hiding this comment.
This now accepts
(con (list data) [ (((((((((((I 5))))))))))), B #FF ])
and
(con unit (((((((((((((((((((()))))))))))))))))))))
Do we really want that?
| constantOf :: ExpectParens -> DefaultUni (Esc a) -> Parser a | ||
| constantOf expectParens uni = | ||
| case uni of | ||
| try (trailingWhitespace . inParens $ constantOf ExpectParensNo uni) <|> case uni of |
There was a problem hiding this comment.
Also, the prettyprinter still prints list elements without parentheses. I suppose we have to choose one way or the other, so that seems fine.
This makes the parse recognize constants with extra parens in them as requested in #7383.
I don't feel like we want to make any of those extra parens mandatory, but there's no harm in being lenient and parsing them.