-
Notifications
You must be signed in to change notification settings - Fork 267
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
Feat: Better format in hover messages #3687
Merged
Merged
Changes from all commits
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
38568ae
Feat: Better format in hover messages
MikaelMayer c3c5516
Support for {:error} on hover also on postconditions
MikaelMayer 32cb31d
Review comments
MikaelMayer 05227f6
Review comments: Got rid of patches by integrating better with Proof …
MikaelMayer 60ec8d9
Merge branch 'master' into feat-better-error-messages
MikaelMayer 0aac9fe
Merge branch 'master' into feat-better-error-messages
MikaelMayer b136d48
Merge branch 'master' into feat-better-error-messages
MikaelMayer 8d2b241
Merge branch 'master' into feat-better-error-messages
MikaelMayer b831daa
Fixed CI
MikaelMayer d7a32d3
Ensured EnsuresDescription and PostconditionDescription share relevan…
MikaelMayer be50afe
Ensure test is deterministic
MikaelMayer 8876a81
Merge branch 'master' into feat-better-error-messages
MikaelMayer 0cd8767
Merge branch 'master' into feat-better-error-messages
MikaelMayer 89daeb4
Merge branch 'master' into feat-better-error-messages
MikaelMayer 9d8d505
Merge branch 'master' into feat-better-error-messages
MikaelMayer ab390c5
Merge branch 'master' into feat-better-error-messages
MikaelMayer 62abf54
Last review comment
MikaelMayer fea8f29
Merge remote-tracking branch 'origin/feat-better-error-messages' into…
MikaelMayer dcbd72f
Fixed latest merge.
MikaelMayer a0b2b8d
Merge branch 'master' into feat-better-error-messages
MikaelMayer 5d53a3a
Merge branch 'master' into feat-better-error-messages
MikaelMayer cfc1a1d
Fixed
MikaelMayer 154fdd3
Merge branch 'master' into feat-better-error-messages
MikaelMayer 50e3f26
Fixed the merge
MikaelMayer 38e14f7
Reverted premature rewording
MikaelMayer 616490f
Fixed another one
MikaelMayer e05ee83
Merge branch 'master' into feat-better-error-messages
MikaelMayer d15cb17
Fixed merge commit
MikaelMayer 5cef283
Fixed documentation by running "make numbers"
MikaelMayer 15cefb6
Fixed CI tests
MikaelMayer df6077a
Merge branch 'master' into feat-better-error-messages
MikaelMayer dea8cb3
Fixed CI calculation step
MikaelMayer 039ba35
Fixed some test cases
MikaelMayer 2be2ff1
Fixed CI
MikaelMayer 84e0e85
Fix error message
MikaelMayer 518194d
Fixed CI
MikaelMayer ba4099a
Fixed doc
MikaelMayer 138d8eb
Merge branch 'master' into feat-better-error-messages
MikaelMayer f4af82e
Fixed CI tests again
MikaelMayer b582fb9
Fixed CI again
MikaelMayer 985d4c8
Fixed last CI error
MikaelMayer 938a17d
Merge branch 'master' into feat-better-error-messages
MikaelMayer 5e2f51e
Self-review
MikaelMayer cdb2f77
Merge branch 'master' into feat-better-error-messages
keyboardDrummer 815d80c
Merge branch 'master' into feat-better-error-messages
MikaelMayer 76756c1
Merge branch 'master' into feat-better-error-messages
keyboardDrummer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
Oops, something went wrong.
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.
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 find the concept of having an
at path
and a regular failure description not immediately intuitive.Conceptually I view the verification failure as a stack trace. For example in:
I think in a previous discussion about squiggly lines, you explained that any part of the stack trace that falls outside of the method definition should not be taken into account, so for failing postconditions that only leaves the bottom 2 stack elements as relevant, and then I see it makes sense to have a failure description for both of those.
Feel free to ignore this comment, just me doing rubber duck analysis.
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.
Thanks for rubber duck analysis :-)
Your metaphor with stack trace is quite interesting, worth exploring from UX's perspective. And I think this PR is exploring it that way. Let me explain you why in the next paragraph.
The existence of "nested expressions" here is due only to the condition splitting at the Dafny level, that is, if we want to prove "A && B", it will prove "A" on one side and "A ==> B" (or just "B" where A is assumed actually) on the other side. If we want to prove
P(x)
, we will prove each conjunct ofP(x)
-- that's also why sometimes it's worth making predicate opaque, because ifP(x)
has 100 conjuncts, provingP(y)
whenx == y
can still be very costly -This is why I chose to replace "Could not prove" (which was technically correct, If you can't prove
A
, you can't proveA && B
) by "Inside", to better match the metaphor of the stack.My point was that they should not be underlined permanently as errors because they are relevant only in the context of the failed assertion.