-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: Change CLI backend to Typer #19
Conversation
@abhijeetSaroha I just fixed some issues on upstream/main .. and rebased your branch |
I am going to play with your new implementation now |
@abhijeetSaroha , playing with that I found just few issus that would fix your code here. this is the diff from changes: diff --git a/src/artbox/cli.py b/src/artbox/cli.py
index 69dde42..247624d 100644
--- a/src/artbox/cli.py
+++ b/src/artbox/cli.py
@@ -92,13 +92,13 @@ def text_to_speech(
typer.Option(
"--lang", help="Choose the language for audio generation"
),
- ] = "",
+ ] = "en",
) -> None:
"""Convert text to speech."""
args_dict = {
"title": title,
"text-path": text_path,
- "output_path": output_path,
+ "output-path": output_path,
"engine": engine,
"lang": lang,
}
after that, check if that fixes the issues you had locally as well, please. |
@xmnlab , corrected that mistake. |
src/artbox/cli.py
Outdated
typer.Option( | ||
"--lang", help="Choose the language for audio generation" | ||
), | ||
] = "", |
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.
] = "", | |
] = "en", |
it is also necessary to define its default as "en"
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.
Done.
LGTM! thanks for working on that @abhijeetSaroha ! appreciate that. good job here. |
🎉 This PR is included in version 0.5.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Change CLI backend to typer.
Solves #12