Skip to content

Conversation

@Jimmy-seraph1080
Copy link
Collaborator

No description provided.

@Jimmy-seraph1080 Jimmy-seraph1080 linked an issue Nov 17, 2025 that may be closed by this pull request
4 tasks
Copy link
Member

@brownsarahm brownsarahm left a 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#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

Comment on lines +38 to +41
def helper():

data = read_yaml(MASTER_FILE)
return data
Copy link
Member

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

Comment on lines +7 to +8
#master dictionary file: to /src/githelp/data/overlays/tips.yml
MASTER_FILE = os.path.join(DATA_DIR, "tips.yml")
Copy link
Member

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

Comment on lines +62 to +64
#set data to the master dictionary file
data = helper()
#check to see if the master is a dict and not empty
Copy link
Member

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?

Comment on lines +65 to +69
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):
Copy link
Member

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

Comment on lines +92 to +94
for key, value in data.items():
if isinstance(value, dict):
names.append(key)
Copy link
Member

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
Copy link
Member

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
'''
Copy link
Member

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.

Comment on lines +19 to +22
@githelp_cli.command(name="list", help="List of available githelp tip pages.")
def list_cmd():
'''List all available githelp tip.'''
click.echo(render_menu())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

githelp explain <command>

3 participants