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.
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
Eliminate Boolean properties #649
Eliminate Boolean properties #649
Changes from 5 commits
e657848
e496bbd
f730932
09ad23f
0790397
a8ad4d7
e711a66
a59d86e
bb42c69
9933cef
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Here we have an interesting problem: this code belongs to the visible section of the PDF and we shouldn't show stuff like that because we'd have to explain what it means (and the meaning isn't particularly interesting). However, marking
isTwoPhaseScriptAddress?
as an instance isn't a good idea and wouldn't even work anyway.I think the only reasonable solution here is to turn
isTwoPhaseScriptAddress
into adata
type and then provide a⁇
instance for that type. As I see it, there are two options to do that:isTwoPhaseScriptAddress
, i.e. something of this form:The second option has the upside that we can cheat a little and just hide this wrapper behind the curtain when rendering the PDF. We currently don't have the machinery to do that perfectly because we'd have to substitute
isTwoPhaseScriptAddress'
withisTwoPhaseScriptAddress
in the line above when rendering the PDF, but we probably need a mechanism for that anyway. So I'm currently leaning more towards that option.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.
How would we go about substituting
isTwoPhaseScriptAddress'
by its unprimed version in the pdf?