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.

Sign up for GitHub

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

Convert most of the commands from autocommand to typer. #21

Merged
merged 3 commits into from
Aug 16, 2024
+83 −70
Merged

Convert most of the commands from autocommand to typer. #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e96a9d4
Convert most of the commands from autocommand to typer.
jaraco Aug 15, 2024
f07f22f
Convert projects-run to typer.
jaraco Aug 15, 2024
9122903
Add type annotations for name/value.
jaraco Aug 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions jaraco/develop/add-github-secret.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
import autocommand
import typer
from typing_extensions import Annotated
from jaraco.ui.main import main

from . import github


@autocommand.autocommand(__name__)
def run(name, value, project: github.Repo = github.Repo.detect()):
@main
def run(
name: str,
value: str,
project: Annotated[
github.Repo, typer.Option(parser=github.Repo)
] = github.Repo.detect(),
):
Copy link
Contributor

@bswck bswck Aug 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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().

Copy link
Owner Author

Choose a reason for hiding this comment

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

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.

project.add_secret(name, value)
15 changes: 10 additions & 5 deletions jaraco/develop/add-github-secrets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import getpass

import autocommand
import keyring

import jaraco.context
import keyring
import typer
from jaraco.ui.main import main
from typing_extensions import Annotated

from . import github

Expand Down Expand Up @@ -32,8 +33,12 @@ def _safe_getuser():
}


@autocommand.autocommand(__name__)
def run(project: github.Repo = github.Repo.detect()):
@main
def run(
project: Annotated[
github.Repo, typer.Option(parser=github.Repo)
] = github.Repo.detect(),
jaraco marked this conversation as resolved.
Show resolved Hide resolved
):
for name in project.find_needed_secrets():
source = secret_sources[name]
value = keyring.get_password(**source)
Expand Down
7 changes: 3 additions & 4 deletions jaraco/develop/add-project.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
import operator
import subprocess

import autocommand
import path

import jaraco.text
from jaraco.ui.main import main

from . import git

Expand All @@ -18,8 +17,8 @@ def add_newlines(lines):
return map('{}\n'.format, lines)


@autocommand.autocommand(__name__)
def main(name: str, target: path.Path = path.Path()):
@main
def run(name: str, target: path.Path = path.Path()):
jaraco marked this conversation as resolved.
Show resolved Hide resolved
repo, path = git.projects_repo()
project = git.Project.parse(name)
with git.temp_checkout(repo, quiet=True):
Expand Down
6 changes: 3 additions & 3 deletions jaraco/develop/checkout-all.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import functools

import autocommand
import path
from jaraco.ui.main import main
from more_itertools import consume

from . import git


@autocommand.autocommand(__name__)
def main(target: path.Path = path.Path()):
@main
def run(target: path.Path = path.Path()):
checkout = functools.partial(git.checkout_missing, root=target)
consume(map(checkout, git.projects()))
4 changes: 2 additions & 2 deletions jaraco/develop/cpython-sync.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
import re
import subprocess

import autocommand
import packaging.version
from jaraco.ui.main import main
from requests_toolbelt import sessions

gh_content = sessions.BaseUrlSession(
Expand Down Expand Up @@ -44,7 +44,7 @@ def is_stable(tag):
return not by_tag(tag).is_stable


@autocommand.autocommand(__name__)
@main
def run(pre=False):
tags = gh_api.get('tags').json()
filtered = tags if pre else filter(is_stable, tags)
Expand Down
4 changes: 2 additions & 2 deletions jaraco/develop/create-github-release.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import autocommand
from jaraco.ui.main import main

from . import github, repo


@autocommand.autocommand(__name__)
@main
def run(project: github.Repo = github.Repo.detect()):
md = repo.get_project_metadata()
Copy link
Contributor

Choose a reason for hiding this comment

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

#21 (comment)

project.create_release(tag=f'v{md.version}')
4 changes: 2 additions & 2 deletions jaraco/develop/finalize.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import pathlib
import subprocess

import autocommand
from jaraco.ui.main import main

from . import towncrier


@autocommand.autocommand(__name__)
@main
def finalize():
ver = towncrier.semver(towncrier.get_version())
# workaround for twisted/towncrier#538
Expand Down
6 changes: 3 additions & 3 deletions jaraco/develop/init-azure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
import json
import subprocess

import autocommand
import keyring
from jaraco.ui.main import main


def create_project(project, user):
Expand Down Expand Up @@ -65,8 +65,8 @@ def create_pipeline(svc, project):
subprocess.run(cmd)


@autocommand.autocommand(__name__)
def main(project, user=None):
@main
def run(project, user=None):
user = user or getpass.getuser()
create_project(project, user)
create_service_endpoint(project, user)
Expand Down
4 changes: 2 additions & 2 deletions jaraco/develop/macos-build-python.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import os
import subprocess

import autocommand
from jaraco.ui.main import main


@functools.lru_cache
Expand All @@ -27,7 +27,7 @@ def require_libs():
assert not missing, f"Need {missing}"


@autocommand.autocommand(__name__)
@main
def build_on_macOS(debug=False):
"""
Build cpython in the current directory on a mac with
Expand Down
5 changes: 2 additions & 3 deletions jaraco/develop/merge.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@
import unittest.mock
from pathlib import Path

import autocommand

import jaraco.packaging.metadata
from jaraco.functools import identity
from jaraco.ui.main import main


sample_conflict = textwrap.dedent(
Expand Down Expand Up @@ -173,7 +172,7 @@ def resolve(conflict):
raise ValueError("Unable to resolve")


@autocommand.autocommand(__name__)
@main
def merge(base: Path, local: Path, remote: Path, merge: Path):
conflicts = Conflict.read(**locals())
res = merge.read_text()
Expand Down
4 changes: 2 additions & 2 deletions jaraco/develop/migrate-config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
import re
import subprocess

import autocommand
from ini2toml.api import Translator
from jaraco.ui.main import main


def split_build_system(pyproject):
Expand Down Expand Up @@ -52,7 +52,7 @@ def bump_setuptools(build_system):
return build_system.replace('setuptools>=56', 'setuptools>=61.2')


@autocommand.autocommand(__name__)
@main
def run():
pyproject = pathlib.Path('pyproject.toml')
config = Translator().translate(pathlib.Path('setup.cfg').read_text(), 'setup.cfg')
Expand Down
7 changes: 3 additions & 4 deletions jaraco/develop/print-meta.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import autocommand

from build.util import project_wheel_metadata
from jaraco.ui.main import main


@autocommand.autocommand(__name__)
def main(path='.', field='Requires-Dist'):
@main
def run(path='.', field='Requires-Dist'):
for spec in project_wheel_metadata(path).get_all(field):
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's annotate these?

Copy link
Owner Author

Choose a reason for hiding this comment

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

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.

Copy link
Owner Author

Choose a reason for hiding this comment

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

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.

Copy link
Owner Author

Choose a reason for hiding this comment

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

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.

print(spec)
49 changes: 23 additions & 26 deletions jaraco/develop/projects-run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,40 +2,37 @@
Routine to run a command across all projects.
"""

import argparse
import functools
import subprocess
from typing import List, cast

import autocommand
import typer
from jaraco.ui.main import main
from typing_extensions import Annotated

from . import filters, git

parser = argparse.ArgumentParser()
parser.add_argument(
'--keyword',
'-k',
dest='selectors',
type=filters.Keyword,
default=filters.Selectors(),
action='append',
)
parser.add_argument(
'--tag',
'-t',
dest='selectors',
type=filters.Tag,
default=filters.Selectors(),
action='append',
)
parser.add_argument('args', nargs='*')


@autocommand.autocommand(__name__, parser=parser)
def main(
selectors: filters.Selectors,
args=None,
@functools.partial(
main,
app=typer.Typer(
context_settings=dict(allow_extra_args=True, ignore_unknown_options=True)
),
)
def run(
tag: Annotated[
List[filters.Tag], typer.Option('--tag', '-t', parser=filters.Tag)
jaraco marked this conversation as resolved.
Show resolved Hide resolved
] = [],
keyword: Annotated[
List[filters.Keyword],
typer.Option('--keyword', '-k', parser=filters.Keyword),
] = [],
args: typer.Context = typer.Option(None),
):
cmd = cast(List[str], args.args)
Comment on lines +30 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Owner Author

Choose a reason for hiding this comment

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

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.

Copy link
Owner Author

Choose a reason for hiding this comment

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

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)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, that works; done in 3a80b14.

selectors = filters.Selectors(tag + keyword)
for project in filter(selectors, git.projects()):
print(project, flush=True)
with git.temp_checkout(project, quiet=True):
subprocess.Popen(args).wait()
subprocess.Popen(cmd).wait()
print(flush=True)
4 changes: 2 additions & 2 deletions jaraco/develop/rst-header-replace.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import re
import sys

import autocommand
from jaraco.ui.main import main


def replace_len(char):
Expand All @@ -12,7 +12,7 @@ def replacer(match):
return replacer


@autocommand.autocommand(__name__)
@main
def run(before, after):
assert len(before) == 1
assert len(after) == 1
Expand Down
15 changes: 10 additions & 5 deletions jaraco/develop/update-projects.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@
import functools
import shutil
import subprocess
from typing import Optional

import autocommand
import subprocess_tee
import typer
from jaraco.ui.main import main
from typing_extensions import Annotated

from . import filters, git

Expand Down Expand Up @@ -48,10 +51,12 @@ def update_project(name, base, branch=None, dry_run=False):
return name


@autocommand.autocommand(__name__)
def main(
keyword: filters.Keyword = None, # type: ignore
tag: filters.Tag = None, # type: ignore
@main
def run(
keyword: Annotated[
Optional[filters.Keyword], typer.Option(parser=filters.Keyword)
] = None,
tag: Annotated[Optional[filters.Tag], typer.Option(parser=filters.Tag)] = None,
base='gh://jaraco/skeleton',
branch=None,
dry_run=False,
Expand Down
5 changes: 3 additions & 2 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ classifiers = [
]
requires-python = ">=3.8"
dependencies = [
"jaraco.ui",
"jaraco.ui >= 2.4",
"jaraco.context",
"keyring",
"autocommand",
"requests-toolbelt",
"PyNaCl",
"packaging",
Expand All @@ -34,6 +33,8 @@ dependencies = [
"requests",
"ini2toml[full]",
"jaraco.packaging >= 10",
"typer",
"typing-extensions",
]
dynamic = ["version"]

Expand Down
Loading