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] Add specific replacement for help text in single command applications #29617

Merged
merged 1 commit into from Jan 1, 2019

Conversation

codedmonkey
Copy link
Contributor

@codedmonkey codedmonkey commented Dec 15, 2018

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

Simply omits the command name in the help text of single command applications which was wrongly displayed before.

For example, if the default command of an application is echo and the application is located at bin/echo, previously the help text would display php bin/echo echo <text> which is incorrect for single command applications since the command name can must be omitted: php bin/echo <text>.

@codedmonkey codedmonkey changed the base branch from master to 3.4 December 15, 2018 00:09
@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Dec 15, 2018
@nicolas-grekas
Copy link
Member

Thanks for submitting. Looks like a new feature to me, it should target master.

@nicolas-grekas nicolas-grekas modified the milestones: 3.4, next Dec 15, 2018
@codedmonkey codedmonkey changed the base branch from 3.4 to master December 18, 2018 12:09
@codedmonkey
Copy link
Contributor Author

I disagree because it seems to be a bug fix that could apply to 3.4 as well, but I'm glad to have it fixed either way.

*
* @return bool Whether the application is a single command application or not
*/
public function isSingleCommandApplication()
Copy link
Member

Choose a reason for hiding this comment

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

I suggest isSingleCommand(): bool. Adding the return type will remove the need for the @return phpdoc annotation

@chalasr
Copy link
Member

chalasr commented Dec 18, 2018

@codedmonkey PR body says the command name can be omitted which means that specifying it does not break, it's just useless, right?
If so, we are not fixing a bad suggestion but trying to enhance a correct one, hence this does not qualify as a bug fix.

@chalasr chalasr added Feature and removed Bug labels Dec 18, 2018
@codedmonkey
Copy link
Contributor Author

No a console command definitely can't start with an optional argument that may or may not need to be to removed. Apologies if my wording caused any confusing but calling php bin/echo echo <text> definitely isn't the same as php bin/echo <text> for a single command application. The command name must be ommitted.

@chalasr chalasr changed the base branch from master to 3.4 January 1, 2019 04:36
@chalasr chalasr modified the milestones: next, 3.4 Jan 1, 2019
@chalasr chalasr added Bug and removed Feature labels Jan 1, 2019
@chalasr
Copy link
Member

chalasr commented Jan 1, 2019

Thank you @codedmonkey.

@chalasr chalasr merged commit 7058f55 into symfony:3.4 Jan 1, 2019
chalasr pushed a commit that referenced this pull request Jan 1, 2019
… command applications (codedmonkey)

This PR was merged into the 3.4 branch.

Discussion
----------

[Console] Add specific replacement for help text in single command applications

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #...   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | <!-- required for new features -->

Simply omits the command name in the help text of single command applications which was wrongly displayed before.

For example, if the default command of an application is `echo` and the application is located at `bin/echo`, previously the help text would display `php bin/echo echo <text>` which is incorrect for single command applications since the command name ~~can~~ **must** be omitted: `php bin/echo <text>`.

Commits
-------

7058f55 [Console] Fix help text for single command applications
This was referenced Jan 6, 2019
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.

None yet

4 participants