Skip to content
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

[console] Use the description when no help is available #15601

Merged
merged 1 commit into from
Aug 31, 2015

Conversation

Nicofuma
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #15498
License MIT
Doc PR

@javiereguiluz
Copy link
Member

👍

@wouterj
Copy link
Member

wouterj commented Aug 24, 2015

👍

Status: reviewed

@fabpot
Copy link
Member

fabpot commented Aug 24, 2015

👎 Help and description are not the same. If help is empty for some command, they should be added instead.

@Nicofuma
Copy link
Contributor Author

They may not be the same but in fact the help is often the description rephrased and with two example to illustrate the usage of one or two options. And people could prefer to not duplicate the information and save time for the translators. It can be done manually for each command or automatically with a fallback. Personally I truly believe it should be done automatically (especially because we don't want to display the description when there is an help block).

Have a look to help help, help debug.twig, help swiftmailer:email:send or cache:clear per example

@@ -131,6 +131,8 @@ public function testGetSetHelp()
$ret = $command->setHelp('help1');
$this->assertEquals($command, $ret, '->setHelp() implements a fluent interface');
$this->assertEquals('help1', $command->getHelp(), '->setHelp() sets the help');
$command->setHelp(null);
Copy link
Member

Choose a reason for hiding this comment

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

setHelp and getHelp both except/return a string. So setting null is not allowed. Btw, the properties in the command should be initialized as empty string. otherwise the phpdoc is also wrong before calling the setters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right, it is initialized to an empty string. Fixed

@wouterj
Copy link
Member

wouterj commented Aug 24, 2015

@fabpot help is the extended version of description. While description only describes what a command does, help also describes how to use the command. If there is no docs on how to use something, it's still usefull to know what it does.

Currently, app/console custom:command --help doesn't give you any information about what custom:command does. With this PR, it at least gives you some information about what it does. Partial documentation is better to no docs at all :)

@fabpot
Copy link
Member

fabpot commented Aug 31, 2015

Thank you @Nicofuma.

@fabpot fabpot merged commit e5d3f25 into symfony:2.3 Aug 31, 2015
fabpot added a commit that referenced this pull request Aug 31, 2015
…icofuma)

This PR was merged into the 2.3 branch.

Discussion
----------

[console] Use the description when no help is available

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

Commits
-------

e5d3f25 [console] Use the description when no help is available
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
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