Skip to content

Conversation

@sri-dhurkesh
Copy link

Description

This pull request adds a new info command to the TerminalGPT project. The info command provides users with detailed information about the current configuration and settings of the TerminalGPT application. This enhancement aims to improve user experience by allowing users to easily access and review important configuration details.

Reviewer Notes:

  • Please review the implementation of the info command in main.py.
  • Ensure that the output format of the info command is user-friendly and informative.
  • Verify that the new command does not interfere with existing functionality.

Screenshots

Screenshot from 2025-02-03 19-22-14
Screenshot from 2025-02-03 19-21-47
Screenshot from 2025-02-03 19-22-31

Thank you for reviewing this pull request. Your feedback is appreciated!

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

terminalgpt/main.py:455

  • Consider adding test cases to validate the info command's output, especially to cover scenarios where metadata retrieval fails.
@click.command(help="Information about the TerminalGPT")

Copy link
Owner

@adamyodinsky adamyodinsky left a comment

Choose a reason for hiding this comment

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

First of all thank you so much for doing a PR! it is very appreciated 🤟

I have 2 small comments...

And i like the idea of this info command, though i'm not sure there is a place for my name on the info, even though it is really nice for my ego, i don't think it helps to much for the user. maybe we can replace it with the info on the machine use the machine_info command in the config.py file.

Machine:

Version: ...

etc...

What do you think?

"""

enc_manager: EncryptionManager = ctx.obj["ENC_MNGR"]
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think we need we need to set the encryption manager here. it can be removed.


enc_manager: EncryptionManager = ctx.obj["ENC_MNGR"]
printer: Printer = ctx.obj["PRINTER"]
enc_manager.get_api_key()
Copy link
Owner

Choose a reason for hiding this comment

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

Same, we don't need to set the api key

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.

2 participants