-
Notifications
You must be signed in to change notification settings - Fork 61
Combine cursor inspector and error print #1991
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
base: dev
Are you sure you want to change the base?
Conversation
…t views into a single make_status_view function
…ng in CursorInspector
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1991 +/- ##
==========================================
- Coverage 51.44% 51.41% -0.03%
==========================================
Files 209 209
Lines 21721 21722 +1
==========================================
- Hits 11175 11169 -6
- Misses 10546 10553 +7
🚀 New features to boost your workflow:
|
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.
Pull Request Overview
This PR refactors error and status message generation by extracting message building logic from CursorInspector.re into a new shared ErrorMessage.re module. The refactoring enables both UI rendering (in CursorInspector.re) and string formatting (in ErrorPrint.re) to use the same underlying message generation logic.
Key Changes
- Created a new
ErrorMessage.remodule with a fragment-based representation that abstracts message components (text, code, types, terms, labels) - Updated
CursorInspector.reto use the new message building functions instead of local view generation - Modified
ErrorPrint.reto use the shared message building logic for string output - Added
join_maputility function toListUtil.re
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/haz3lcore/ErrorMessage.re | New module containing shared message building logic with fragment-based representation for all info types (exp, pat, typ, tpat) |
| src/web/app/inspector/CursorInspector.re | Removed local message building functions and replaced with calls to shared ErrorMessage module, added render_ui to convert fragments to UI nodes |
| src/haz3lcore/TyDi/ErrorPrint.re | Replaced individual error printing functions with shared message building and fragment rendering to strings |
| src/util/ListUtil.re | Added join_map utility function for joining mapped lists with separators |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
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.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
|
@7h3kk1d comments as requested regarding llms: I'm fine with this approach. in practice we might override some of this in an ad hoc way when iterating on llm behavior. this could in principle be generic in that mostly we just want better error messages, but the constraints are somewhat different, ie the size of the cursor inspector versus practically unbounded. there is definitely a principled way to handle this, eg short messages in the bar, longer ones in an expanded dialog, but i'm not highly motivated to solve this in a principled way atm. |
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.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Text("is a sum type constuctor of type"), | ||
| Type(sum_ty), | ||
| ] | ||
| | VariantIncomplete(sum_ty) => [ | ||
| Text("An incomplete sum type constuctor of type"), |
Copilot
AI
Nov 18, 2025
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.
Corrected spelling of 'constuctor' to 'constructor'.
| Text("is a sum type constuctor of type"), | |
| Type(sum_ty), | |
| ] | |
| | VariantIncomplete(sum_ty) => [ | |
| Text("An incomplete sum type constuctor of type"), | |
| Text("is a sum type constructor of type"), | |
| Type(sum_ty), | |
| ] | |
| | VariantIncomplete(sum_ty) => [ | |
| Text("An incomplete sum type constructor of type"), |
| Text("is a sum type constuctor of type"), | ||
| Type(sum_ty), | ||
| ] | ||
| | VariantIncomplete(sum_ty) => [ | ||
| Text("An incomplete sum type constuctor of type"), |
Copilot
AI
Nov 18, 2025
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.
Corrected spelling of 'constuctor' to 'constructor'.
| Text("is a sum type constuctor of type"), | |
| Type(sum_ty), | |
| ] | |
| | VariantIncomplete(sum_ty) => [ | |
| Text("An incomplete sum type constuctor of type"), | |
| Text("is a sum type constructor of type"), | |
| Type(sum_ty), | |
| ] | |
| | VariantIncomplete(sum_ty) => [ | |
| Text("An incomplete sum type constructor of type"), |
Uses a single
InfoMessagerepresentation that gets passed toErrorPrintandCursorInspector.A message is a list of
InfoMessage.fragmentwhich is a variant that has different stylings coupled with a is_error flag that determines whether or not an error is present.In the future I think the
InfoMessageformat could be extended to any information that is displayed in some contextual menu like the sidebar.