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

Update integer list example #2103

Merged
merged 4 commits into from
Aug 9, 2024
Merged

Update integer list example #2103

merged 4 commits into from
Aug 9, 2024

Conversation

xsebek
Copy link
Member

@xsebek xsebek commented Aug 9, 2024

  • reformat using swarm format
  • use tydef
  • use format
  • use accumulator functions

* use `swarm format`
* use `tydef`
* use `format`
* use accumulator functions
@xsebek xsebek requested a review from byorgey August 9, 2024 17:07
@xsebek
Copy link
Member Author

xsebek commented Aug 9, 2024

@byorgey what do you think of the reformatting? 🙂 I think it is pretty good, though it does merge comments.

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.

I think it looks pretty good.

In some sense we don't need this any more since we now have actual lists, but it's still nice as an extended example. Might be nice to add a comment explaining this.

example/list.sw Outdated
, 1 + snd p
)
}
if (nextPart == 0) {(2 * i, 1 /* last part */)} {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, the reformatting moved the last part comment inside some parens...

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved the comment before the expression, but the formatted inserts an empty line before it.
I guess that is better, but will needs some tweaks if we want to use it for all files.

Copy link
Member

Choose a reason for hiding this comment

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

Thinking about this more, I am actually surprised that it does not print {(2 * i, 1)} /* last part */. I thought it was supposed to do that; I will have to think about whether this is a bug.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, getting auto-formatted comments to look good in all situations is surprisingly tough. But I would be very happy to collect more examples of where it breaks down to see if we can continue tweaking the output.

@xsebek xsebek added the merge me Trigger the merge process of the Pull request. label Aug 9, 2024
@mergify mergify bot merged commit e031863 into main Aug 9, 2024
9 checks passed
@mergify mergify bot deleted the task/xsebek/integer-list branch August 9, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants