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

Special treatment of attribute followed by underscore in pretty-printer #6281

Closed
nad opened this issue Nov 3, 2022 · 1 comment · Fixed by #6282
Closed

Special treatment of attribute followed by underscore in pretty-printer #6281

nad opened this issue Nov 3, 2022 · 1 comment · Fixed by #6282
Assignees
Labels
modalities type: bug Issues and pull requests about actual bugs ux: display Issues relating to how terms are printed for display ux: printing Issues relating to how terms are printed for display
Milestone

Comments

@nad
Copy link
Contributor

nad commented Nov 3, 2022

Why is the following code used to print things with certain attributes?

prettyRelevance :: LensRelevance a => a -> Doc -> Doc
prettyRelevance a d =
if render d == "_" then d else pretty (getRelevance a) <> d
prettyQuantity :: LensQuantity a => a -> Doc -> Doc
prettyQuantity a d =
if render d == "_" then d else pretty (getQuantity a) <+> d
prettyErased :: Erased -> Doc -> Doc
prettyErased = prettyQuantity . asQuantity
prettyCohesion :: LensCohesion a => a -> Doc -> Doc
prettyCohesion a d =
if render d == "_" then d else pretty (getCohesion a) <+> d

I think this code is based on code for irrelevance added by @andreasabel back in 2010 in a commit (8fd48ca) with the following commit message:

Added DontCare to Internal and Abstract syntax to print erased stuff more nicely as underscore instead of Prop.

I'm guessing that the original motivation no longer applies, and I doubt that this motivation ever applied for @0. Furthermore it might be inefficient to call render d in this way (depending on how lazy render is). Could the special treatment of "_" be removed?

@nad nad added ux: display Issues relating to how terms are printed for display ux: printing Issues relating to how terms are printed for display type: task Concerning the development of Agda (not in changelog) modalities labels Nov 3, 2022
@nad nad added this to the 2.6.4 milestone Nov 3, 2022
@nad
Copy link
Contributor Author

nad commented Nov 3, 2022

I removed the special treatment of "_" and ran make fail, and one test case was affected. Now λ (@0 _) → Set is printed as λ (@0 _) → Set instead of as λ (_) → Set. The old behaviour is incorrect.

@nad nad added type: bug Issues and pull requests about actual bugs and removed type: task Concerning the development of Agda (not in changelog) labels Nov 3, 2022
@nad nad self-assigned this Nov 3, 2022
nad added a commit that referenced this issue Nov 3, 2022
@andreasabel andreasabel linked a pull request Nov 4, 2022 that will close this issue
nad added a commit that referenced this issue Nov 4, 2022
nad added a commit that referenced this issue Nov 5, 2022
@nad nad closed this as completed in #6282 Nov 5, 2022
@nad nad modified the milestones: 2.6.4, 2.6.3 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modalities type: bug Issues and pull requests about actual bugs ux: display Issues relating to how terms are printed for display ux: printing Issues relating to how terms are printed for display
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant