-
Notifications
You must be signed in to change notification settings - Fork 0
2 githelp explain command #9
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
brownsarahm
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.
good start, some mostly cosmetic/ simplifyig changes
| or if any error occurs while reading/parsing. | ||
| ''' | ||
| try: | ||
| #r opens the file for reading in text mode, and encoding="utf-8" tells Python to decode the file's bytes |
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.
| #r opens the file for reading in text mode, and encoding="utf-8" tells Python to decode the file's bytes | |
| # opens the file for reading in text mode, and encoding="utf-8" tells Python to decode the file's bytes |
| def helper(): | ||
|
|
||
| data = read_yaml(MASTER_FILE) | ||
| return data |
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.
what is the point of this function
| #master dictionary file: to /src/githelp/data/overlays/tips.yml | ||
| MASTER_FILE = os.path.join(DATA_DIR, "tips.yml") |
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.
let's not use the term mater try mainor even just tips
| #set data to the master dictionary file | ||
| data = helper() | ||
| #check to see if the master is a dict and not empty |
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.
again, remove references to master and why not jsut use the read function directly?
| if isinstance(data, dict): | ||
| #get the section for this specific command and store it in section | ||
| section = data.get(command) | ||
| #check if section is a dict | ||
| if isinstance(section, dict): |
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 no else, then no need for if
| for key, value in data.items(): | ||
| if isinstance(value, dict): | ||
| names.append(key) |
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.
why do the values inside need to also be a dict?
| names.append(key) | ||
|
|
||
| names.sort() | ||
| return names |
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.
wouldn't it be equivalent to just return sorted(data.keys())? since data will always be a dict (even if empty)
| ------- | ||
| str | ||
| A formatted multi-line string | ||
| ''' |
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 this could be a lot simpler, using fstrings.
| @githelp_cli.command(name="list", help="List of available githelp tip pages.") | ||
| def list_cmd(): | ||
| '''List all available githelp tip.''' | ||
| click.echo(render_menu()) |
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.
| @githelp_cli.command(name="list", help="List of available githelp tip pages.") | |
| def list_cmd(): | |
| '''List all available githelp tip.''' | |
| click.echo(render_menu()) | |
| @githelp_cli.command( help="List of available githelp tip pages.") | |
| def list(): | |
| '''List all available githelp tip.''' | |
| click.echo(render_menu()) |
this is equivalent but slightly more compact
No description provided.