Convert most of the commands from autocommand to typer.

[jaraco]

Closes #20

[jaraco]

I haven't yet been able to convert projects-run, because it

• collects any number of arguments into a list
• collects both keywords and tags into the same 'selectors' parameter
• passes unused arguments through

[jaraco]

@bswck Do you have any opinion on the implementation?

[bswck]

Thanks for the ping. Here are a few suggestions from me.

[bswck]

Interesting. I've used '.' as a default with github.Repo.detect being the parser for that thus far (as here), which as I see now is completely wrong, because github.Repo.detect() doesn't take a path as a non-default parameter (shouldn't that change? :P). Ideally IMO, the github.Repo class should be constructible from path (maybe not on the constructor-level, because that would introduce a side effect to instantiating the class, which often feels clunky and unpredictable), from which the project name would be inferred the same way as in github.Repo.detect().

[jaraco]

Because github.Repo is a subclass of str, it can be constructed from a simple string, but that string needs to be the name of the project in GitHub. I should probably document that.

I think what you're suggesting is that github.Repo.detect(...) should accept a path parameter that defaults to . instead of assuming the repo is in the cwd. I'd be okay with that, I think.

Regardless, this change is about something different - migrating from autocommand to typer, so I'm aiming for feature parity. We can address other concerns in another issue/PR.

[bswck]

Let's annotate these?

[jaraco]

Sounds good. I'd do that here as a suggestion, but I don't know the syntax (where to put the spaces to satisfy the linter), so I'm going to do it outside of this PR.

[jaraco]

On second thought, does it make sense to annotate a variable that's assigned a static string? That feels like over-specifying to me. It's so much easier to read without the annotations and even mypy knows it's a string, right? Let's go without it for now. If there's a benefit to adding the type annotations, I'm open to hear it, but I'm more inclined to keep it simple if the typing system supports that.

I wonder how typer would handle number = 0; would it need the type annotation to know to construct an integer (or float?) from the input? If not, that's maybe a good justification for indicating the str - for symmetry and consistency with other types.

[jaraco]

I did end up adding the type annotation, which also gave mypy permission to start nitpicking about the | None of email.Message.get_all. Fixed in 405317a.

[bswck]

That doesn't look correct. typer.Context is always injected when there's an annotation that inherits from click.Context (see inference implementation).

Suggested change

	args: typer.Context = typer.Option(None),
	):
	cmd = cast(List[str], args.args)
	args: typer.Context,
	):
	cmd = cast(List[str], args.args)

Additionally, I'd name the context parameter context or ctx. args.args reads weird. But that's just an opinion.

[jaraco]

Good call. Not sure how I missed this in the review. I went through the whole thing in the files dialog and didn't see it. I'll get it after the fact.

[jaraco]

I tried removing = typer.Option(None), but you can't do that because tag and keyword have default values. Maybe it would work if cmd were keyword-only.

Suggested change

	args: typer.Context = typer.Option(None),
	):
	cmd = cast(List[str], args.args)
	*,
	args: typer.Context,
	):
	cmd = cast(List[str], args.args)

[jaraco]

Yes, that works; done in 3a80b14.

[bswck]

See also jaraco/jaraco.ui@208a1c2#r145432894.

[bswck]

#21 (comment)