-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Pass FormatContext
to NeedsParentheses
#5643
Conversation
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
738e9a9
to
2c04bdc
Compare
PR Check ResultsBenchmarkLinux
Windows
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it!
f06aa8e
to
0c7c03d
Compare
2c04bdc
to
0681c5e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I sneeked in another change. I renamed context.contents to source. contents is too generic and doesn't tell you anything.
that's a good change
0c7c03d
to
b754a0d
Compare
0681c5e
to
c895628
Compare
b754a0d
to
51c4306
Compare
Graphite rebased this pull request after merging its parent, because this pull request is set to merge when ready. |
c895628
to
553008e
Compare
@MichaReiser merged this pull request with Graphite. |
Summary
I started working on this because I assumed that I would need access to options inside of
NeedsParantheses
but it then turned out that I won't.Anyway, it kind of felt nice to pass fewer arguments. So I'm gonna put this out here to get your feedback if you prefer this over passing individual fiels.
Oh, I sneeked in another change. I renamed
context.contents
tosource
.contents
is too generic and doesn't tell you anything.Test Plan
It compiles