Skip to content

make raw yna call redirect to yna ynamazon but without the option to add command-line args#21

Merged
DanielKarp merged 2 commits intomainfrom
cli_callback
Apr 9, 2025
Merged

make raw yna call redirect to yna ynamazon but without the option to add command-line args#21
DanielKarp merged 2 commits intomainfrom
cli_callback

Conversation

@DanielKarp
Copy link
Owner

The reason for not allowing command line args is that it is very hard to do within Typer, as the callback is run every time and it gets messy to figure out if you are running a subcommand or the base command but with args. The apprach I took is to just use a check for the length of sys.argv to check if yna is being run without any other arguements, and in that case calling ynamazon with the function arguements retrieved from settings directly. Users wanting to use command line args would continue to call yna ynamazon [args]

@DanielKarp
Copy link
Owner Author

@WoosterInitiative could you take a look and let me know what you think?

@WoosterInitiative
Copy link
Collaborator

Clever! I'll do a quick check, but I like it better than I thought I would.

Copy link
Collaborator

@WoosterInitiative WoosterInitiative left a comment

Choose a reason for hiding this comment

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

I know it's being nitpicky and likely doesn't make any difference, but I think I'd prefer this:

from typer import Context
from typer import run as typer_run
from rich import print as rprint

@cli.callback(invoke_without_command=True)
def yna_callback(ctx: Context):
  rprint("[bold cyan]Starting YNAmazon processing...[/]"
  if ctx.invoked_subcommand is None:
    typer_run(ynamazon)

Using the renamed "run" and "print" commands are just my preference to avoid having "common" things in my namespace. The print is also just as a clue to the user (me) about what's happening.

Again, it doesn't change the functionality at all, but it seems a little more "native" to Typer and avoids sys.

Either way works, though, choose what makes sense to you.

@DanielKarp
Copy link
Owner Author

I think that's better. Just a question, does typer_run call 'ynamazon' in a way that triggers the default factories? Also, will running 'yna --help' not trigger this, since a subcommands wasn't invoked, or is there some catch for that internally?

I'll try this out on my local branch and if it works I'll push it to the PR.

@WoosterInitiative
Copy link
Collaborator

Just a question, does typer_run call 'ynamazon' in a way that triggers the default factories?

Yes, it behaves exactly as if it had been run as yna ynamazon, though it won't take explicit parameters from the user, same as yours.

will running 'yna --help' not trigger this, since a subcommands wasn't invoked

It seems to treat '--help' with a special case and documents the top-level (showing all of the subcommands).

I've played with it a bit and it seems to work as I would expect it to.

You can also use the docstring of the callback to say something about running it without any commands or options and it will run ynamazon. I don't have good verbiage in mind, but it might be helpful?

@DanielKarp DanielKarp merged commit 5b5adca into main Apr 9, 2025
1 check passed
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