Indent code blocks to make it part of ordered list#1570
Conversation
Summary of ChangesHello @abhro, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a documentation formatting issue by adjusting the indentation of Julia code blocks within ordered lists. The goal is to improve the rendering of these sections, making the code blocks appear as sub-elements of their respective list items, thereby enhancing readability and adherence to common Markdown interpretation, despite potential rendering quirks in the immediate build environment. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request aims to fix a Markdown rendering issue in the PairwiseFusion docstring by indenting code blocks to make them part of an ordered list. The change correctly applies a 4-space indent to the code blocks, which is standard practice. While this is a step in the right direction, the complete fix likely requires also correcting inconsistent indentation in the paragraphs of the list items themselves, as noted in the detailed comment.
| ```julia | ||
| y = x[1] | ||
| for i in 1:N | ||
| y = connection(x[i + 1], layers[i](y)) | ||
| end | ||
| ``` |
There was a problem hiding this comment.
Indenting this code block by 4 spaces is the correct approach to associate it with the list item according to Markdown standards, and is consistent with other docstrings in this file (e.g., RepeatedLayer).
However, as you've noted, this may not fully resolve the rendering issue. The problem likely lies in the inconsistent indentation of the paragraph preceding this block. On line 390, the text is indented by 3 spaces, while the list item on line 389 effectively starts its content at column 5 (after 1.).
To fix the list rendering completely, the paragraph on line 390 should also be indented by 4 spaces to align with the text on line 389. While that line isn't part of this change, adjusting it should make your indentation of this code block work as expected.
5bc9a35 to
193794e
Compare
193794e to
4508673
Compare
4508673 to
9511ac3
Compare
It doesn't work. The rendered list is still fragmented, with a single item list, then a code block, then another single item list, then another code block. However, the second list does start at 2 as opposed to 1 in the current build. Unsure of the build intricacies involved here. But for most Markdown parsers, it should appear as one list.
9511ac3 to
783d8f9
Compare
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #1570 +/- ##
==========================================
- Coverage 82.28% 74.19% -8.10%
==========================================
Files 172 172
Lines 7141 7130 -11
==========================================
- Hits 5876 5290 -586
- Misses 1265 1840 +575 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Note: the fix in this commit doesn't even work. The rendered list is still fragmented, with a single item list, then a code block, then another single item list, then another code block.
However, the second list does start at 2 as opposed to 1 in the current build. Unsure of the build intricacies involved here. But for most Markdown parsers, it should appear as one list.