-
Notifications
You must be signed in to change notification settings - Fork 77
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
Mostly graphical improvements to the explain plugin #733
Conversation
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.
Amazing!
(* `Ratio (`Float 0.8); *) | ||
(* `Concentrate true; *) |
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.
Should these be toggable with an 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.
Hm I was playing around trying to improve the layout, but from my tests they don't really provide meaningful improvements ; I certainly should cleanup all these comments though.
@@ -863,7 +863,7 @@ let translate_program | |||
match states with | |||
| D.WholeVar -> WholeVar (ScopeVar.fresh (var_name, var_pos)) | |||
| States states -> | |||
let var_prefix = var_name ^ "_" in | |||
let var_prefix = var_name ^ "#" in |
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.
Are we sure this gets correctly changed back to _
in the C or Python backends?
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.
Yes, that would otherwise be a bug!
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.
Re: indeed, there was a bug where this would appear in the backend language (but it was a symptom of a wider bug where we would cleanup unicode characters but not non-ident characters that were part of ASCII like é
).
The latest patch fixes that for all backends (just fixing the normalisation functions used by the Renaming
module).
They don't display well. Incidentally that seems to work around issue #721 on our example.
if < 1.0 and on either side of a multiplication (don't display `foo + 10%` as that could be misinterpreted)
No description provided.