Skip to content

Allow plugin-specific CLI options before --how#4708

Open
thrix wants to merge 1 commit intomainfrom
thrix/fix-plugin-option-before-how
Open

Allow plugin-specific CLI options before --how#4708
thrix wants to merge 1 commit intomainfrom
thrix/fix-plugin-option-before-how

Conversation

@thrix
Copy link
Contributor

@thrix thrix commented Mar 17, 2026

Previously, plugin-specific options like --interactive had to come after --how on the command line. For example:

tmt run execute --interactive -h tmt  # failed
tmt run execute -h tmt --interactive  # worked

The root cause was in _find_how() which scans CLI args to locate --how. When encountering an unknown option (not in the base command params), it gave up immediately. Plugin-specific options are only registered on method subcommands, not the base command.

Fix _find_option_by_arg() to also search params across all plugin method commands. When the same short option appears in multiple plugins with different nargs, the match consuming the most arguments is used conservatively.

This is safe because _find_how() operates on a copy of args and Click re-parses the original args for actual option handling. The worst case is _find_how() returning None — identical to the current behavior.

Generated-by: Claude Code

Pull Request Checklist

  • implement the feature
  • write the documentation
  • extend the test coverage
  • update the specification
  • adjust plugin docstring
  • modify the json schema
  • mention the version
  • include a release note

Previously, plugin-specific options like `--interactive` had to come
after `--how` on the command line. For example:

```
tmt run execute --interactive -h tmt  # failed
tmt run execute -h tmt --interactive  # worked
```

The root cause was in `_find_how()` which scans CLI args to locate
`--how`. When encountering an unknown option (not in the base command
params), it gave up immediately. Plugin-specific options are only
registered on method subcommands, not the base command.

Fix `_find_option_by_arg()` to also search params across all plugin
method commands. When the same short option appears in multiple
plugins with different `nargs`, the match consuming the most arguments
is used conservatively.

This is safe because `_find_how()` operates on a copy of args and Click
re-parses the original args for actual option handling. The worst
case is `_find_how()` returning `None` — identical to the current
behavior.

Assisted-by: Claude Code
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
@thrix thrix added the ci | full test Pull request is ready for the full test execution label Mar 17, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The changes allow plugin-specific CLI options before --how by extending the option search logic. A test is added to verify this. One simplification is suggested for a test assertion.

Comment on lines +305 to +306
if result.exit_code != 0:
assert 'No such option' not in result.output
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Simplify the assertion. The if statement is unnecessary.

        assert 'No such option' not in result.output

@happz
Copy link
Contributor

happz commented Mar 17, 2026

I don't think it's that easy. Consider the following example:

diff --git a/tmt/steps/report/html.py b/tmt/steps/report/html.py
index 1480d047..7d4898a6 100644
--- a/tmt/steps/report/html.py
+++ b/tmt/steps/report/html.py
@@ -52,6 +52,12 @@ class ReportHtmlData(tmt.steps.report.ReportStepData):
              """,
     )

+    baz: int = field(
+        option='--baz',
+        is_flag=True,
+        default=False,
+    )
+

 @tmt.steps.provides_method('html')
 class ReportHtml(tmt.steps.report.ReportPlugin[ReportHtmlData]):
diff --git a/tmt/steps/report/junit.py b/tmt/steps/report/junit.py
index c16e6294..09f73226 100644
--- a/tmt/steps/report/junit.py
+++ b/tmt/steps/report/junit.py
@@ -491,6 +491,11 @@ class ReportJUnitData(tmt.steps.report.ReportStepData):
         help='Include full standard output in resulting xml file.',
     )

+    baz: str = field(
+        option='--baz',
+        default='quack',
+    )
+

 @tmt.steps.provides_method(
     'junit',

Two plugins, both define same key and option, but one plugin requires a value while the other treats it as a flag. Then:

$ tmt -vv run -a report --baz -h html

Invalid subcommand of 'run' found: 'html'.

IIUIC, new process selected --baz option from the junit plugin, as it consumes the most nargs; that makes -h value of --baz, orphaning html.

What is the use case, BTW? The PR description seems to me as if the current behavior would be a bug ("The root cause was ...").

@thrix
Copy link
Contributor Author

thrix commented Mar 18, 2026

@happz I thought so ... well, I got hit by it while manually testing and seems not properly documeted as a known behaviour (although the cmdline examples of course are correct) . So maybe I can just document it more ...

@happz
Copy link
Contributor

happz commented Mar 18, 2026

One possible improvement: does tmt really need to consume the options? All it tries to do is finding -h foo, --how foo, or -hfoo among the options. Would the first block of checks be good enough?

            def _find_how(args: list[str]) -> Optional[str]:
                while args:
                    arg = args.pop(0)

                    # Handle '--how method' or '-h method'
                    if arg in ['--how', '-h']:
                        # Found `-h/--how foo`, next argument is how'
                        return args.pop(0)

                    # Handle '--how=method'
                    if arg.startswith('--how='):
                        return re.sub('^--how=', '', arg)

                    # Handle '-hmethod'
                    if arg.startswith('-h'):
                        return re.sub('^-h ?', '', arg)

                return None

@happz happz added this to planning Mar 18, 2026
@github-project-automation github-project-automation bot moved this to backlog in planning Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci | full test Pull request is ready for the full test execution

Projects

Status: backlog

Development

Successfully merging this pull request may close these issues.

2 participants