-
-
Notifications
You must be signed in to change notification settings - Fork 347
global --verbose flag #326 done #361
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: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR implements a global --verbose
flag for the CLI application to enable consistent debug output across all commands. The implementation introduces centralized state management for the verbose flag and adds a new list
command for browsing available Linux command lessons.
Key changes:
- Added global verbose flag support with cascade to subcommands
- Introduced a new
list
command with limit functionality and verbose output - Extended test coverage for both verbose modes and new list command functionality
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
cli/cli.py | Added global verbose flag callback and registered new list command |
cli/state.py | New module for managing verbose state across CLI contexts |
cli/commands/hello.py | Updated to support verbose output and use centralized state management |
cli/commands/listing.py | New list command with lesson discovery and verbose debug output |
cli/test_cli.py | Enhanced test suite with helper function and verbose/list command coverage |
cli/README.md | Updated documentation with new command examples and verbose flag usage |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
def _format_title(path: Path) -> str: | ||
stem = path.stem | ||
prefix, _, slug = stem.partition("-") | ||
title = slug.replace("-", " ").strip().title() if slug else prefix.replace("-", " ") | ||
if prefix.isdigit(): | ||
return f"{prefix} {title}".strip() | ||
return title | ||
|
||
|
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.
[nitpick] The title formatting logic is complex and handles multiple cases in a single function. Consider splitting this into separate functions for prefix extraction and title formatting to improve readability and maintainability.
def _format_title(path: Path) -> str: | |
stem = path.stem | |
prefix, _, slug = stem.partition("-") | |
title = slug.replace("-", " ").strip().title() if slug else prefix.replace("-", " ") | |
if prefix.isdigit(): | |
return f"{prefix} {title}".strip() | |
return title | |
def _extract_prefix_and_slug(stem: str) -> tuple[str, str]: | |
prefix, _, slug = stem.partition("-") | |
return prefix, slug | |
def _format_title_from_parts(prefix: str, slug: str) -> str: | |
title = slug.replace("-", " ").strip().title() if slug else prefix.replace("-", " ") | |
if prefix.isdigit(): | |
return f"{prefix} {title}".strip() | |
return title | |
def _format_title(path: Path) -> str: | |
stem = path.stem | |
prefix, slug = _extract_prefix_and_slug(stem) | |
return _format_title_from_parts(prefix, slug) |
Copilot uses AI. Check for mistakes.
def _format_title(path: Path) -> str: | ||
stem = path.stem | ||
prefix, _, slug = stem.partition("-") | ||
title = slug.replace("-", " ").strip().title() if slug else prefix.replace("-", " ") |
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.
This line contains duplicated logic for replacing dashes with spaces. Extract this transformation into a variable or helper function to avoid repetition.
def _format_title(path: Path) -> str: | |
stem = path.stem | |
prefix, _, slug = stem.partition("-") | |
title = slug.replace("-", " ").strip().title() if slug else prefix.replace("-", " ") | |
def _dashes_to_spaces(s: str) -> str: | |
return s.replace("-", " ") | |
def _format_title(path: Path) -> str: | |
stem = path.stem | |
prefix, _, slug = stem.partition("-") | |
title = _dashes_to_spaces(slug).strip().title() if slug else _dashes_to_spaces(prefix) |
Copilot uses AI. Check for mistakes.
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.
@Manjunath3155 this looks perfect! Thank you for working on it. Just the CI is failing could you take a look at that? And also maybe look into some of the suggestions from the Copilot bot.
Remove unnecessary blank lines in hello.py
What type of PR is this? (check all applicable)
Description
--verbose
flag on the root Typer app so every command can emit consistent debug outputlist
subcommand that surfaces available lessons, supports--limit
, and participates in verbose modeRelated Tickets & Documents
Added to documentation?