Simplify define-pretty macro #63
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit changes the `pretty` syntax parameter to an ordinary function that calls whatever function is in `current-pretty`. This removes the need for the `#:default` keyword in `define-pretty`, and as a result the `#:let` keyword isn't needed anymore either. This makes `define-pretty` macro uses a bit more readable, since they can now use ordinary default function arguments and ordinary local definitions.
Owner
|
Oh, I think I did it that way to break the cycle, because https://github.com/sorawee/fmt/blob/master/core.rkt#L136 The fact that your PR passes the CI is puzzling to me though. Maybe things work and there's no need to break the cycle after all? |
Contributor
Author
|
Yeah the cycle is fine. Indirection through the box is enough to break it. Though I am surprised it uses a box at all instead of a parameter. |
Owner
|
I recall I benchmarked at some point and found |
Contributor
Author
|
Ah, that makes sense. Parameters have the overhead of thread cells to worry about. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request changes the
prettysyntax parameter to an ordinary function that calls whatever function is incurrent-pretty. This removes the need for the#:defaultkeyword indefine-pretty, and as a result the#:letkeyword isn't needed anymore either. This makesdefine-prettymacro uses a bit more readable since they can now use ordinary default function arguments and local definitions.