-
Notifications
You must be signed in to change notification settings - Fork 79
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
documentation: code block in doc comments #3771
Conversation
@@ -1704,7 +1704,9 @@ class AddOp(RdRsRsOperation[IntRegisterType, IntRegisterType, IntRegisterType]): | |||
Adds the registers rs1 and rs2 and stores the result in rd. | |||
Arithmetic overflow is ignored and the result is simply the low XLEN bits of the result. | |||
|
|||
``` |
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.
Is this code?
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.
If it is should it also be C code as below?
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.
The code below is C pseudocode, it's not currently syntax coloured, but at least the syntax colouring there would be correct. Here there's not really a close in syntax thing that would make sense IMO
xdsl/dialects/riscv.py
Outdated
@@ -1946,7 +1948,7 @@ class JalrOp(RdRsImmJumpOperation): | |||
""" | |||
Jump to address and place return address in rd. | |||
|
|||
``` | |||
``` C |
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.
Does this not have to be
```C
To work correctly?
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.
hmm, not in the markdown editors I use but you might be right
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.
You're right
@@ -1293,7 +1353,8 @@ def custom_print_attributes(self, printer: Printer) -> Set[str]: | |||
|
|||
class M_M_Operation(Generic[R1InvT], IRDLOperation, X86Instruction, ABC): | |||
""" | |||
A base class for x86 operations with a memory reference that's both a source and a destination | |||
A base class for x86 operations with a memory reference that's both a source and a |
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.
Unrelated formatting change?
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.
Yep I accidentally included it but it makes the lines fit into 88 columns so I think still a good thing to merge.
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 believe you, but it would be nice to get a screenshot?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3771 +/- ##
=======================================
Coverage 91.25% 91.25%
=======================================
Files 461 461
Lines 57478 57478
Branches 5543 5543
=======================================
Hits 52454 52454
Misses 3602 3602
Partials 1422 1422 ☔ View full report in Codecov by Sentry. |
I added one to the other PR, soon enough we'll have docs for our PRs so you won't have to take my word for it! |
Makes doc comments prettier and improves rendering in online docs (upcoming).