Skip to content

Pool console: full signature syntax, original exceptions, and resilient command discovery - #350

Open
techmahedy wants to merge 1 commit into
doppar:4.xfrom
techmahedy:console
Open

techmahedy wants to merge 1 commit into
doppar:4.xfrom
techmahedy:console

Conversation

@techmahedy

@techmahedy techmahedy commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Pull Request Checklist

Q A
Branch? 4.x
Bug fix? yes
New feature? yes
Deprecations? no
Issues -
License MIT

Summary

Doppar runs on Symfony Console 8.1 (^8.1, 8.1.5 installed). An audit of pool against it found no removed or deprecated API in use: all 68 commands load, --help renders for every one, and there are no warnings or deprecations. It did find three real problems, fixed here, and one gap in the signature parser, which this PR closes.

Bug fixes

  1. Exceptions lost their origin. Command::execute() caught everything, logged it and threw a bare new \Exception($e->getMessage()). Every failure was reported as "In Command.php line 150", the real class, code and trace were lost (even with -vvv), and if Log::error() itself threw, that error replaced the real one. The exception is now logged inside a guard and rethrown as is.
  2. One bad file broke every pool command. Discovery built every .php file under Commands/ and src/Schedule/Commands, so a helper, trait, interface, abstract class, or a command whose constructor throws (including a syntax error in a user command) made even pool list fail. Discovery now skips anything that is not a concrete Symfony\Component\Console\Command\Command subclass, and a command that cannot be built is skipped with Skipped command [Class]: reason on stderr while the rest load.
  3. Signature definitions were silently dropped. parseSignature() ignored any definition it could not parse. {name=guest}, {files*} and {--cache!} registered nothing (the command then failed at runtime with "argument does not exist"), and {--tag=*} became a plain string option whose default was the literal text *. Unparseable definitions now throw a LogicException naming the command class and the definition.

New: full signature syntax

Exposes the InputArgument / InputOption features that the parser did not, including Symfony 8.1's boolean default on negatable options:

Signature Result
{name=guest} optional argument with a default
{files*} / {files?*} required / optional array argument ([] when omitted)
{--tag=*} repeatable option (--tag=a --tag=b), returned as an array
{--cache!} negatable: --cache, --no-cache; null when neither is passed
{--cache!=true} / {--cache!=false} negatable with a boolean default

Everything that parsed before parses the same way: {name}, {name?}, {--flag}, {-f|--flag}, {--opt=}, {--opt=default}, and : descriptions. Symfony's own rules still apply (an array argument must be last, a required argument cannot follow an optional one) and surface as its LogicException.

Behaviour changes to be aware of

  • A command that now fails at load time with "Invalid definition" used to load with the bad definition silently missing. I checked all shipped commands: none contain a definition the parser used to drop. The finance app's own commands were not covered by that check.
  • Exceptions thrown from handle() are now reported with their real class and location instead of a generic Exception from Command.php. The exit code follows Symfony's rule for the original exception's code.
  • A skipped command is reported on stderr and is not an error exit; check the stderr line if a command seems to be missing.

Files

  • src/Phaseolies/Console/Schedule/Command.php: parseSignature() split into addArgumentFromDefinition() / addOptionFromDefinition(); execute() rethrows the original.
  • src/Phaseolies/Console/Command.php: isCommandClass(), per-command isolation in registerCommands(), overridable reportSkippedCommand().
  • tests/Console/CommandSignatureGrammarTest.php: new.

Checklist

  • Tests have been added or updated
  • Documentation has been updated
  • Code follows the project coding standards
  • All tests pass locally

@techmahedy
techmahedy requested a review from rrr63 October 2, 2026 17:59
@techmahedy techmahedy added bug Something isn't working enhancement New feature or request feat new feature labels Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request feat new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant