Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for Typer #14

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

evanmags
Copy link

Hey @willmcgugan, You don't have a CONTRIBUTING.md on this repo, so I'm not sure if/how I can contribute to this, but these are the changes I made on my fork to get Trogon working in my Typer app. I have a few TODOs below but would like your input for desired usage and best practice. Thank you 馃檹

TODO:

  • match desired usage
  • tests based on best practice

trogon/trogon.py Outdated
@@ -23,6 +23,7 @@
Footer,
)
from textual.widgets.tree import TreeNode
import typer
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't want to force a dependancy on typer, for those who just want to use click...

@evanmags evanmags force-pushed the main branch 3 times, most recently from b05c7fe to 49e8807 Compare May 22, 2023 16:58
@evanmags
Copy link
Author

Removed the dependency requirement on typer and created an extra for it. This pattern of init_tui(app) feels more intuitive than shoehorning the decorator to work with a library that suggests creating many nested apps.

I'm weighing the benefits of being able to add this to a single command in typer, as the pattern set out in the docs would be to create a nested app for that.

@evanmags evanmags marked this pull request as ready for review May 24, 2023 11:47
README.md Outdated
Comment on lines 106 to 111
2. Add the `tui` decorator above your typer app. e.g.
```python
cli = typer.Typer(...)
init_tui(cli)
```
3. Your click app will have a new `tui` command available.
Copy link
Member

Choose a reason for hiding this comment

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

Think this needs some work - it still talks about Click and decorators etc.

Fix the description for Typer support.

Signed-off-by: Michael Gielda <[email protected]>
@mgielda
Copy link

mgielda commented Dec 28, 2023

Hi @darrenburns - I made a PR against @evanmags' branch fixing the docs. When he merges that, your review will be included; is there anything else that needs to be done to make this land?

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.

None yet

4 participants