Skip to content
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

Support optional rendering of AnnotationType type prefix like "note:", "help:", "hint:", etc. #59

Open
ironcev opened this issue Jul 31, 2023 · 1 comment
Labels
C-enhancement Category: enhancement

Comments

@ironcev
Copy link

ironcev commented Jul 31, 2023

Perhaps this functionality already exists, but I couldn't find it in any of the options, I also didn't see how it could be manipulated in the resulting DisplayList.

By default, the prefix that denotes the AnnotationType is not rendered for errors and warnings, but it is for all other types of annotations. In the output of the Sway compiler we need the flexibility to show the prefix for some annotations, and for some not.

Consider the following output of the Sway compiler:

07C Constant shadowing - Alias - After 02

We want to have the help: prefix in the footer, but not the info: prefix in the code snippet. In general, we do not want to have any prefixes in the code snippet.

Is there a way already to achieve this functionality?

If not, I would be glad to contribute and implement it.

ironcev added a commit to FuelLabs/sway that referenced this issue Aug 2, 2023
## Description

A new `Diagnostic` type is introduced for detail description of compile
errors and warnings. The change is backward compatible. The existing
`CompileWarning`s and `CompileError`s will continue render as they had
before.

The `Diagnostic` is formed out of a:
- _Level_: Error or Warning.
- _Code_: Unique error or warning code. Placeholder for future. Not used
at the moment.
- _Reason_: Short description of the diagnostic, not related to a
specific error/warning. Answers the question "Why is this an
error/warning?" E.g., Because - "Constants cannot be shadowed".
- _Issue_: Short description of the concrete case that the compiler has
found. E.g., "Variable "X" shadows imported constant with the same name"
- _Hints_: Detailed description of the diagnostic placed in the source
code.
- _Help_: Additional friendly information that helps understanding and
solving the issue, but which is not related to a place in code.

![07C Constant shadowing - Alias - After 02 -
Terminology](https://github.com/FuelLabs/sway/assets/4142833/50c639a1-43f5-4bc2-afa2-5717652f0172)

The `Diagnostic`s are defined imperatively in code right now, pretty
much the sam way we do `CompilWarning`s at the moment. Development of a
proc-macro that should make de definitions declarative is out of scope
of #21.

## Known Limitations

The `annotate-snippets` library has a bug and a missing functionality
for which I opened issues:

- Wrong display of line numbers when using folding of lines of code.
This issue is fixed but there is no patch release provided:
rust-lang/annotate-snippets-rs#52 (comment)
- No possibility to remove the "note:" prefix as shown on the image
above: rust-lang/annotate-snippets-rs#59

These two issues are not blocking. Proposal is to wait for the official
support in the library, or contribute or in the worst case make a
workaround in our code.

## Demo (Before and After)

### Errors
![07A Constant shadowing - Alias -
Before](https://github.com/FuelLabs/sway/assets/4142833/5d67c5e5-d456-4762-b7c7-6fcf7340d6b3)

![07C Constant shadowing - Alias - After
02](https://github.com/FuelLabs/sway/assets/4142833/02691b39-3d08-4776-ba93-5ffeca4546ed)

### Warnings
![06A NonScreamingSnakeCaseConstName -
Before](https://github.com/FuelLabs/sway/assets/4142833/c27279d4-0e6c-4d05-9b00-da7121bb60ad)

![06B NonScreamingSnakeCaseConstName -
After](https://github.com/FuelLabs/sway/assets/4142833/483044f3-f190-4506-9e9e-2d03d70cb3f2)

Closes #21

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
@epage
Copy link
Contributor

epage commented Mar 13, 2024

#97 would add a way to annotate "context" which is independent of Level.

@epage epage added the C-enhancement Category: enhancement label Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants