Remove print_assigment for some utilities and less dry code #2010
+8,250
−9,656
Conversation
This file contains 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
ospencer
reviewed
Feb 11, 2024
| @@ -260,6 +260,94 @@ let needs_grouping = (~parent, ~side: infix_side, expr) => { | |||
| }; | |||
| }; | |||
|
|
|||
| let get_op_and_assignment = new_value => { | |||
There was a problem hiding this comment.
This name feels confusing to me out of context. I want a name that communicates that it gets the operator and assignment value from an infix assignment expression, but obviously that seems unnecessarily wordy with a name like get_infix_assignment_op_and_value.
There was a problem hiding this comment.
What about get_infix_parts?
phated
force-pushed
the
phated/formatter-record-pattern
branch
from
1899825
to
5ea244f
Compare
Base automatically changed from
phated/formatter-record-pattern
to
oscar/formatter
February 19, 2024 20:29
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 removes the
print_assignmentfunction, as we don't have the concept of an assignment in the parsetree.As we saw with the previous formatter, having a bunch of flags that you pass to a function is a code smell that makes it harder to tweak specific behavior on specific nodes of the tree. Instead, I've inlined the Doc creation code to the match branches, which is a little bit of duplication right now but will make our lives easier if one branch needs to change.
I've also made helpers to encapsulate the collapsible logic, which already lead to a question during the formatter code review.