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

Add new pretty-printer precedence level for comma-separated lists. #1286

Merged
merged 3 commits into from
May 5, 2021

Conversation

brianhuffman
Copy link
Contributor

This lets us print pair values using infix ',' with parens in the right places.

Fixes #1147.

@@ -114,28 +114,19 @@ depthAllowed _ _ = True

-- | Precedence levels, each of which corresponds to a parsing nonterminal
data Prec
= PrecNone -- ^ Nonterminal 'Term'
= PrecNone -- ^ Nonterminal 'Term' or "sepBy(Term, ',')"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor Haddock nit: text enclosed in double quotes denote links to names of modules, which I don't think you intended. Use this to make it render using a plain monospace font:

Suggested change
= PrecNone -- ^ Nonterminal 'Term' or "sepBy(Term, ',')"
= PrecNone -- ^ Nonterminal 'Term' or @sepBy(Term, \',\')@

Escaping the single quotes is also necessary to prevent Haddock from trying to hyperlink , as an identifier name.

@@ -114,28 +114,19 @@ depthAllowed _ _ = True

-- | Precedence levels, each of which corresponds to a parsing nonterminal
data Prec
= PrecNone -- ^ Nonterminal 'Term'
= PrecNone -- ^ Nonterminal 'Term' or "sepBy(Term, ',')"
| PrecTerm -- ^ Nonterminal 'Term'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both PrecTerm and PrecNone have the description "Nonterminal Term", but is there ever a situation where you'd use PrecNone for something besides a tuple?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think about it, no, actually there's not. In that case it's probably a good idea to rename the constructor to something like PrecCommas, to reflect its actual purpose.

@brianhuffman brianhuffman merged commit 1fb5f4e into master May 5, 2021
@brianhuffman brianhuffman deleted the issue1147 branch May 5, 2021 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Term printer error
2 participants