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

Projects
None yet
4 participants
@codedmonkey
Copy link
Contributor

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 Dec 15, 2018

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Dec 15, 2018

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Dec 15, 2018

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 Dec 18, 2018

@codedmonkey codedmonkey force-pushed the codedmonkey:single-command branch from 2f46e24 to ecc2b98 Dec 18, 2018

@codedmonkey

This comment has been minimized.

Copy link
Contributor

codedmonkey commented Dec 18, 2018

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()

This comment has been minimized.

@chalasr

chalasr Dec 18, 2018

Member

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

@chalasr

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

codedmonkey commented Dec 18, 2018

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.

@codedmonkey codedmonkey force-pushed the codedmonkey:single-command branch from ecc2b98 to cf46638 Dec 18, 2018

@chalasr chalasr changed the base branch from master to 3.4 Jan 1, 2019

@chalasr chalasr force-pushed the codedmonkey:single-command branch from cf46638 to 7058f55 Jan 1, 2019

@chalasr chalasr modified the milestones: next, 3.4 Jan 1, 2019

@chalasr chalasr added Bug and removed Feature labels Jan 1, 2019

@chalasr

This comment has been minimized.

Copy link
Member

chalasr commented Jan 1, 2019

Thank you @codedmonkey.

@chalasr chalasr merged commit 7058f55 into symfony:3.4 Jan 1, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

chalasr added a commit that referenced this pull request Jan 1, 2019

bug #29617 [Console] Add specific replacement for help text in single…
… 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