-
Notifications
You must be signed in to change notification settings - Fork 269
docs(cli): improve docstring formatting for --help and markdown
#1210
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
docs(cli): improve docstring formatting for --help and markdown
#1210
Conversation
|
@georgeh0 @badmonster0, kindly help me review when you have time. Thank You |
--help and markdown
docs/docs/core/cli-commands.md
Outdated
| `APP_FLOW_SPECIFIER`: Specifies the application and optionally the target flow. Can be one of the following formats: | ||
|
|
||
| - an_installed.module_name | ||
| - `path/to/your_app.py` - `an_installed.module_name` - `path/to/your_app.py:SpecificFlowName` - `an_installed.module_name:SpecificFlowName` |
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 format here is not right.
dev/generate_cli_docs.py
Outdated
| elif in_description: | ||
| if line.strip(): | ||
| # Non-empty line | ||
| description_lines.append(line.strip()) | ||
| last_was_empty = False | ||
| else: | ||
| # Empty line - only add one blank line to separate paragraphs | ||
| if description_lines and not last_was_empty: | ||
| description_lines.append("") | ||
| last_was_empty = True | ||
|
|
||
| # Join lines, treating consecutive lines as same paragraph unless separated by blank line | ||
| result = [] | ||
| current_paragraph = [] | ||
|
|
||
| for line in description_lines: | ||
| if line == "": | ||
| # Blank line - end current paragraph | ||
| if current_paragraph: | ||
| result.append(" ".join(current_paragraph)) | ||
| current_paragraph = [] | ||
| else: | ||
| current_paragraph.append(line) | ||
|
|
||
| description = "\n\n".join(description_lines) if description_lines else "" | ||
| # Add any remaining paragraph | ||
| if current_paragraph: | ||
| result.append(" ".join(current_paragraph)) | ||
|
|
||
| # Join paragraphs with double newline | ||
| description = "\n\n".join(result) if result else "" |
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 think it'll just work if we simply join description lines with \n (i.e. 1 \n instead of 2), without any filtering logic above.
Because Markdown already allows single line break within a paragraph.
This means as long as the docstring for cli.py is written in the Markdown-compatible way, it'll just work.
The logic here can be simplified a lot, and the incorrect format for bullets (as I comment below) will also be fixed.
Does this make sense?
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.
@georgeh0 - Yes, this makes perfect sense! You're absolutely right.
I was over-engineering the solution by trying to intelligently join lines and create paragraphs. Your suggestion to simply join with \n and let Markdown handle it naturally is much cleaner and will fix both issues:
- Bullet list formatting - Preserving the original line breaks will keep bullets on separate lines
- Simpler logic - Trusting Markdown's natural behavior instead of complex paragraph detection
I'll simplify the extract_description() function as you suggested and ensure the docstrings in cli.py are Markdown-compatible. This will resolve the formatting issues.
Thank you for the clear feedback!
1489701 to
81736f3
Compare
- Fix bullet list formatting: each bullet now on separate line - Simplify extract_description logic as suggested by reviewer - Intelligently join regular paragraphs while preserving bullet lists - Maintain all Issue cocoindex-io#1108 requirements (backticks, no mid-sentence breaks) Addresses feedback from PR cocoindex-io#1210
9ac6352 to
9f2b9e2
Compare
- Simplify description extraction to use single newline joins - Fix bullet list formatting: each bullet now on separate line - Remove complex paragraph joining logic (45 lines -> 28 lines) - Let Markdown handle formatting naturally - Maintain all Issue cocoindex-io#1108 requirements (backticks, no redundant blank lines) Addresses feedback from @georgeh0 in PR cocoindex-io#1210
- Add backticks around technical terms (APP_TARGET, APP_FLOW_SPECIFIER, paths, modules) - Fix docstring formatting in cli.py for better rendering in both terminal and markdown - Simplify extract_description() logic in generate_cli_docs.py (join with \n, let Markdown handle formatting) - Fix bullet list formatting: each bullet now on separate line - Eliminate redundant blank lines that break sentences mid-paragraph - Ensure consistent, professional rendering across all CLI commands Fixes cocoindex-io#1108
648eafe to
d3d3385
Compare
Remove unnecessary last_was_empty tracking logic. Click's help formatter already produces well-formatted output, so we only need to: - Append non-empty stripped lines - Preserve blank lines for paragraph separation (if we have content) This addresses reviewer feedback and makes the code cleaner and more maintainable.
georgeh0
left a comment
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.
Thanks!
generate_cli_docs.pyFixes #1108