8000 [Console] do not make the getHelp() method smart by xabbuh · Pull Request #15976 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Console] do not make the getHelp() method smart #15976

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

Merged
merged 1 commit into from
Sep 28, 2015

Conversation

xabbuh
Copy link
Member
@xabbuh xabbuh commented Sep 28, 2015
Q A
Bug fix? kind of
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

In my opinion, the way to always display a help message to the user was implemented in #15601 is not good enough. The getter method for the help should only return the actual value. Otherwise, user's would not have a way to check if a command really has a help message (for example, when building their own CLI applications or whatever). Instead, we should only return the description as a fallback of the help message when it is processed to present it to the user.

@@ -513,7 +513,7 @@ public function getProcessedHelp()
$_SERVER['PHP_SELF'].' '.$name,
);

return str_replace($placeholders, $replacements, $this->getHelp());
return str_replace($placeholders, $replacements, $this->getHelp() ? $this->getHelp() : $this->getDescription());
Copy link
Member

Choose a reason for hiding this comment

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

use ?: here

@stof
Copy link
Member
stof commented Sep 28, 2015

you should add a test covering the fallback for getProcessedHelp though

@xabbuh
Copy link
Member Author
xabbuh commented Sep 28, 2015

added a test

@fabpot
Copy link
Member
fabpot commented Sep 28, 2015

Thank you @xabbuh.

@fabpot fabpot merged commit cdf1f00 into symfony:2.3 Sep 28, 2015
fabpot added a commit that referenced this pull request Sep 28, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

[Console] do not make the getHelp() method smart

| Q             | A
| ------------- | ---
| Bug fix?      | kind of
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

In my opinion, the way to always display a help message to the user was implemented in #15601 is not good enough. The getter method for the help should only return the actual value. Otherwise, user's would not have a way to check if a command really has a help message (for example, when building their own CLI applications or whatever). Instead, we should only return the description as a fallback of the help message when it is processed to present it to the user.

Commits
-------

cdf1f00 [Console] do not make the getHelp() method smart
@xabbuh xabbuh deleted the pr-15601-tweak branch September 28, 2015 20:30
This was referenced Oct 27, 2015
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.

4 participants
0