Skip to content

[Console] Overcomplete argument exception message tweak. #19446

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.

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

Conversation

SpacePossum
Copy link
Contributor

Q A
Branch? 2.7
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

Updates the exception message when to many arguments are passed.
From;

'Too many arguments.'

To:

'No argument expected, got "foo".'
// or
'Too many arguments, expected arguments "foo".'
// or
'Too many arguments, expected arguments "foo, bar".'
// ... turtles all the way down

@HeahDude
Copy link
Contributor

Shouldn't it target master instead of 2.7?

@SpacePossum
Copy link
Contributor Author

SpacePossum commented Jul 27, 2016

why is that, it is not a BC?
(happy to retarget btw.)

@HeahDude
Copy link
Contributor

HeahDude commented Jul 27, 2016

It might if some code rely on the message (very unlikely but you have the case with the tests), let's wait for fabpot pov anyway :)

@fabpot
Copy link
Member

fabpot commented Jul 27, 2016

We can change an exception message, which is not part of the BC promise. Nobody should ever rely on an exception message in code anyway.

@HeahDude
Copy link
Contributor

Ok then big 👍 :)

throw new \RuntimeException(sprintf('Too many arguments, expected arguments "%s".', implode(', ', array_keys($all))));
}

throw new \RuntimeException(sprintf('No argument expected, got "%s".', $token));
Copy link
Member

Choose a reason for hiding this comment

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

No arguments expected.

@@ -183,7 +183,17 @@ public function provideInvalidInput()
array(
array('cli.php', 'foo', 'bar'),
new InputDefinition(),
'Too many arguments.',
'No argument expected got "foo".',
Copy link
Member

Choose a reason for hiding this comment

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

No arguments

Copy link
Contributor

Choose a reason for hiding this comment

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

and a comma after "expected"

@fabpot
Copy link
Member

fabpot commented Jul 28, 2016

Thank you @SpacePossum.

@fabpot fabpot closed this Jul 28, 2016
fabpot added a commit that referenced this pull request Jul 28, 2016
… (SpacePossum)

This PR was squashed before being merged into the 2.7 branch (closes #19446).

Discussion
----------

[Console] Overcomplete argument exception message tweak.

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

Updates the exception message when to many arguments are passed.
From;
```php
'Too many arguments.'
```
To:
```php
'No argument expected, got "foo".'
// or
'Too many arguments, expected arguments "foo".'
// or
'Too many arguments, expected arguments "foo, bar".'
// ... turtles all the way down
```

Commits
-------

7af59cd [Console] Overcomplete argument exception message tweak.
@SpacePossum SpacePossum deleted the 2_7_overcomplete_argument_exception_message branch July 28, 2016 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants