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

Make pretty Terms format prettier #1464

Merged
merged 10 commits into from
Aug 25, 2023
Merged

Make pretty Terms format prettier #1464

merged 10 commits into from
Aug 25, 2023

Conversation

xsebek
Copy link
Member

@xsebek xsebek commented Aug 24, 2023

  • improve layout of terms:
    • break lines on binds, unless it fits on one line
    • lambdas go on same line, but the body can go to next line
    • def and let can have long body on next line (indented for def)
    • parens and braces have body indented if it does not fit on line
  • closes Better indentation & layout for pretty-printing #11

Example using --format from #1459:

> cabal run swarm -O0 -- format scenarios/Challenges/_blender/patrol-clockwise.sw                          
def forever = \c. c; force forever c end;
def encircle = \lDir. \rDir.
  turn lDir;
  b <- blocked;
  if b {turn rDir} {wait 1};
  fwBlocked <- blocked;
  if fwBlocked {turn rDir} {move}
end;
def patrolCW = force forever (force encircle right left) end;
force patrolCW

@xsebek xsebek requested a review from byorgey August 24, 2023 15:53
@xsebek
Copy link
Member Author

xsebek commented Aug 24, 2023

@byorgey it would be great to have this rebased on #1459 so the improvements can be tested interactively.

Should I rebase and close #1459, or should we merge --format first?

Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

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

This looks good to me, it's exactly the kind of thing I had in mind. Thanks for figuring out the right combinators to use etc.

The CI is failing because the formatting of terms has changed in a few error messages. It looks like the messages are long and then have a term on the end, so the pretty-printer is inserting newlines, but this looks bad, e.g.

1:9: Invalid atomic block: def, let, and lambda are not allowed: \c. c;
c

I think we should change it to put a newline before the (indented) term, e.g.

1:9: Invalid atomic block: def, let, and lambda are not allowed: 
  \c. c; c

As for #1459 , that one will require more work/thought, since as you point out, it is not valid to e.g. insert force when formatting. So I think we should just go ahead and merge this.

It gets grouped and does not overflow on next line.
@xsebek
Copy link
Member Author

xsebek commented Aug 24, 2023

@byorgey meanwhile, I came up with a different solution - add backticks to code in error messages (as we already did for some) and group it, so it tends to stay on one line.

Thinking about it, we could experiment with flatAlt and use triple backticks in case it goes multiline. But I am still a Prettyprinter newbie. 😅

@byorgey
Copy link
Member

byorgey commented Aug 24, 2023

Instead of enclosing in single or triple backticks etc., it would also be fun to play with using the "annotations" feature to do syntax highlighting etc. for error messages, just like we do for entity descriptions etc. But we can leave that for another PR.

@xsebek
Copy link
Member Author

xsebek commented Aug 24, 2023

@byorgey yeah, I thought about that too.

I think that would lead to a standalone function:

prettyTypeErr :: TypeErr -> Doc MarkdownAnn

The alterAnnotations function could then be used to convert our annotations to AnsiStyle or Attr.

@xsebek
Copy link
Member Author

xsebek commented Aug 24, 2023

OK, let's not be hasty this time...

Screenshot 2023-08-24 at 21 20 44

It seems some code can get messed up inside descriptions. 🤦

@xsebek
Copy link
Member Author

xsebek commented Aug 24, 2023

The descriptions should be OK again. Hopefully it was the only visible bug. 😅

FYI: After this lands, to get the one line effect use inline-code (move; move) and for multiline use code block:

move;
move;

@xsebek xsebek added the CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. label Aug 24, 2023
@xsebek xsebek mentioned this pull request Aug 24, 2023
@byorgey
Copy link
Member

byorgey commented Aug 25, 2023

@xsebek looks good to me! I like how you handled the issue with code formatting in descriptions by distinguishing between single and triple backticks.

@xsebek xsebek added the merge me Trigger the merge process of the Pull request. label Aug 25, 2023
@mergify mergify bot merged commit 98a6b75 into main Aug 25, 2023
9 checks passed
@mergify mergify bot deleted the pretty-layout branch August 25, 2023 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better indentation & layout for pretty-printing
2 participants