Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Console] Prevent fatal error when calling Command::getHelper without helperSet #18635

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

Closed
wants to merge 1 commit into from

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Apr 25, 2016

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #18619
License MIT
Doc PR n/a

Patch attached to #18619

@javiereguiluz javiereguiluz changed the title [Console][#18619] Prevent fatal error when calling Command#getHelper without helperSet [Console] Prevent fatal error when calling Command::getHelper without helperSet Apr 25, 2016
@@ -605,6 +605,10 @@ public function getUsages()
*/
public function getHelper($name)
{
if (null === $this->helperSet) {
throw new LogicException(sprintf('Unable to retrieve helper "%s" because there is no HelperSet defined for this command. Did you forget to call %s#setHelperSet()?', $name, get_class($this)));
Copy link
Member

Choose a reason for hiding this comment

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

If I'm right, in Symfony we usually use the Class::method syntax instead of Class#method. So %s#setHelperSet() should be %s::setHelperSet() in this error message.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Fixed

@chalasr chalasr force-pushed the patch_console_ticket_18619 branch from 6b99f20 to 50816e2 Compare April 25, 2016 19:55
@nicolas-grekas
Copy link
Member

Could be applied on 2.3 also as bug fix.

@javiereguiluz
Copy link
Member

👍 ... and I agree about merging it on 2.3.

@@ -605,6 +605,10 @@ public function getUsages()
*/
public function getHelper($name)
{
if (null === $this->helperSet) {
throw new LogicException(sprintf('Unable to retrieve helper "%s" because there is no HelperSet defined for this command. Did you forget to call %s::setHelperSet()?', $name, get_class($this)));
Copy link
Member

Choose a reason for hiding this comment

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

The message can be confusing as you usually do not set the helper set directly on the command, but configure it for the application instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

@xabbuh I simplified it by removing the Did you forget... part, let me know if you think that something like:

Did you forget to set an application for your command via Command::setApplication()? You can also set an HelperSet directly from your command using Command::setHelperSet().

Could be helpful. Thanks

Copy link
Member

Choose a reason for hiding this comment

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

The first part (setting the application) can as well be related to the command not being registered at the application (which will then set itself on the command).

Maybe something like:

Did you forget to add your command to the application or to set the application on the command using the setApplication() method? You can also set the HelperSet directly using the setHelperSet() method.

Copy link
Member Author

Choose a reason for hiding this comment

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

@xabbuh Updated

@chalasr chalasr force-pushed the patch_console_ticket_18619 branch 2 times, most recently from c232ecb to 6045a23 Compare May 2, 2016 14:07
…Helper() without helperSet

Use Command::setHelperSet rather than Command#setHelperSet in exception msg

Simplify exception message

Add DidYouForget to exception msg
@chalasr chalasr force-pushed the patch_console_ticket_18619 branch from 6045a23 to ede4d92 Compare May 2, 2016 15:30
@fabpot
Copy link
Member

fabpot commented May 13, 2016

Thank you @chalasr.

fabpot added a commit that referenced this pull request May 13, 2016
…per without helperSet (chalasr)

This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #18635).

Discussion
----------

[Console] Prevent fatal error when calling Command::getHelper without helperSet

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #18619
| License       | MIT
| Doc PR        | n/a

Patch attached to #18619

Commits
-------

31285c2 [Console][#18619] Prevent fatal error when calling Command#getHelper() without helperSet
@fabpot fabpot closed this May 13, 2016
@fabpot fabpot mentioned this pull request May 13, 2016
@fabpot fabpot mentioned this pull request May 30, 2016
This was referenced Jun 6, 2016
@chalasr chalasr deleted the patch_console_ticket_18619 branch November 6, 2016 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.