Skip to content

Commit

Permalink
feat: pass the app_config to pre_parse_args() (#541)
Browse files Browse the repository at this point in the history
This lets commands be able to always rely on `self.config` existing, even
when running `fill_parser()` for help output.

Fixes #530
  • Loading branch information
tigarmo authored Oct 22, 2024
1 parent 81eb21d commit 384153d
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 9 deletions.
9 changes: 6 additions & 3 deletions craft_application/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,15 +425,18 @@ def _get_dispatcher(self) -> craft_cli.Dispatcher:

try:
craft_cli.emit.trace("pre-parsing arguments...")
app_config = self.app_config
# Workaround for the fact that craft_cli requires a command.
# https://github.com/canonical/craft-cli/issues/141
if "--version" in sys.argv or "-V" in sys.argv:
try:
global_args = dispatcher.pre_parse_args(["pull", *sys.argv[1:]])
global_args = dispatcher.pre_parse_args(
["pull", *sys.argv[1:]], app_config
)
except craft_cli.ArgumentParsingError:
global_args = dispatcher.pre_parse_args(sys.argv[1:])
global_args = dispatcher.pre_parse_args(sys.argv[1:], app_config)
else:
global_args = dispatcher.pre_parse_args(sys.argv[1:])
global_args = dispatcher.pre_parse_args(sys.argv[1:], app_config)

if global_args.get("version"):
craft_cli.emit.message(f"{self.app.name} {self.app.version}")
Expand Down
8 changes: 6 additions & 2 deletions craft_application/commands/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import abc
import argparse
import warnings
from typing import Any, Optional, Protocol, final

from craft_cli import BaseCommand, emit
Expand Down Expand Up @@ -55,8 +56,11 @@ class AppCommand(BaseCommand):

def __init__(self, config: dict[str, Any] | None) -> None:
if config is None:
# This should only be the case when the command is not going to be run.
# For example, when requesting help on the command.
warnings.warn(
"Creating an AppCommand without a config dict is pending deprecation.",
PendingDeprecationWarning,
stacklevel=3,
)
emit.trace("Not completing command configuration")
return

Expand Down
9 changes: 9 additions & 0 deletions docs/reference/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@
Changelog
*********

4.4.0 (2024-Oct-XX)
-------------------

Application
===========

- ``AppCommand`` subclasses now will always receive a valid ``app_config``
dict.

4.3.0 (2024-Oct-11)
-------------------

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ description = "A framework for *craft applications."
dynamic = ["version", "readme"]
dependencies = [
"craft-archives>=2.0.0",
"craft-cli>=2.6.0",
"craft-cli>=2.9.0",
"craft-grammar>=2.0.0",
"craft-parts>=2.1.1",
"craft-platforms>=0.3.1",
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/commands/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ def test_get_managed_cmd(fake_command, verbosity, app_metadata):

def test_without_config(emitter):
"""Test that a command can be initialised without a config.
This is necessary for providing per-command help.
This is pending deprecation but still supported.
"""

command = base.AppCommand(None)
with pytest.deprecated_call():
command = base.AppCommand(None)

emitter.assert_trace("Not completing command configuration")
assert not hasattr(command, "_app")
Expand Down
33 changes: 33 additions & 0 deletions tests/unit/test_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
services,
util,
)
from craft_application.commands import AppCommand
from craft_application.models import BuildInfo
from craft_application.util import (
get_host_architecture, # pyright: ignore[reportGeneralTypeIssues]
Expand Down Expand Up @@ -2141,3 +2142,35 @@ def test_clean_platform(monkeypatch, tmp_path, app_metadata, fake_services, mock
base=bases.BaseName("ubuntu", "24.04"),
)
mocked_clean.assert_called_once_with(mocker.ANY, mocker.ANY, expected_info)


class AppConfigCommand(AppCommand):

name: str = "app-config"
help_msg: str = "Help text"
overview: str = "Overview"

def fill_parser(self, parser: argparse.ArgumentParser) -> None:

name = self._app.name
parser.add_argument(
"app-name",
help=f"The name of the app, which is {name!r}.",
)


@pytest.mark.usefixtures("emitter")
def test_app_config_in_help(
monkeypatch,
capsys,
app,
):
app.add_command_group("Test", [AppConfigCommand])
monkeypatch.setattr(sys, "argv", ["testcraft", "app-config", "-h"])

with pytest.raises(SystemExit):
app.run()

expected = "app-name: The name of the app, which is 'testcraft'."
_, err = capsys.readouterr()
assert expected in err

0 comments on commit 384153d

Please sign in to comment.