-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(translate): print constant names with hover info #35134
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: master
Are you sure you want to change the base?
feat(translate): print constant names with hover info #35134
Conversation
PR summary 4071ce7432Import changes for modified filesNo significant changes to the import graph Import changes for all files
Declarations diffNo declarations were harmed in the making of this PR! 🐙 You can run this locally as follows## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>
## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>The doc-module for No changes to technical debt.You can run this locally as
|
| -- if this declaration is not `pre` and not an internal declaration, we return an error, | ||
| -- since we should have already translated this declaration. | ||
| if src != pre && !src.isInternalDetail then | ||
| throwError "The declaration {pre} depends on the declaration {src} which is in the namespace \ |
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.
Why not also use .ofConstname for pre?
| if res == src then | ||
| throwError "{t.attrName}: the generated translated name equals the original name '{src}'.\n\ | ||
| throwError "{t.attrName}: the generated translated name equals the original name \ | ||
| '{.ofConstName src}'.\n\ |
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.
| '{.ofConstName src}'.\n\ | |
| `{.ofConstName src}`.\n\ |
Let's take this opportunity to use back-ticks. The advantage of backtics is that they cannot be part of declaration names, while ' can.
Arguably, we should consistently do this in the whole file.
| unless srcDecl.levelParams.length == tgtDecl.levelParams.length do | ||
| throwError "`{t.attrName}` validation failed:\n expected {srcDecl.levelParams.length} \ | ||
| universe levels, but '{tgt}' has {tgtDecl.levelParams.length} universe levels" | ||
| universe levels, but '{.ofConstName tgt}' has {tgtDecl.levelParams.length} \ |
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.
| universe levels, but '{.ofConstName tgt}' has {tgtDecl.levelParams.length} \ | |
| universe levels, but `{.ofConstName tgt}` has {tgtDecl.levelParams.length} \ |
| throwError "`{t.attrName}` validation failed: expected{indentExpr srcType}\nbut '{tgt}' has \ | ||
| type{indentExpr tgtType}" | ||
| throwError "`{t.attrName}` validation failed: expected{indentExpr srcType}\n\ | ||
| but '{.ofConstName tgt}' has type{indentExpr tgtType}" |
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.
| but '{.ofConstName tgt}' has type{indentExpr tgtType}" | |
| but `{.ofConstName tgt}` has type{indentExpr tgtType}" |
Part of #34846.