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] Display console application name even when no version set #17326

Closed
wants to merge 3 commits into from
Closed

[Console] Display console application name even when no version set #17326

wants to merge 3 commits into from

Conversation

polc
Copy link
Contributor

@polc polc commented Jan 10, 2016

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

When displaying help of an Application with a name but no version, it show "Console Tool" instead of the application name.

@@ -324,6 +324,8 @@ public function getLongVersion()
{
if ('UNKNOWN' !== $this->getName() && 'UNKNOWN' !== $this->getVersion()) {
return sprintf('<info>%s</info> version <comment>%s</comment>', $this->getName(), $this->getVersion());
} else if ('UNKNOWN' !== $this->getName()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

else is not required here.

@nicolas-grekas
Copy link
Member

👍
Status: reviewed

@@ -325,6 +325,9 @@ public function getLongVersion()
if ('UNKNOWN' !== $this->getName() && 'UNKNOWN' !== $this->getVersion()) {
return sprintf('<info>%s</info> version <comment>%s</comment>', $this->getName(), $this->getVersion());
}
if ('UNKNOWN' !== $this->getName()) {
return sprintf('<info>%s</info>', $this->getName());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

More clear would be IMO:

if ('UNKNOWN' !== $this->getName()) {
    if ('UNKNOWN' !== $this->getVersion()) {
        return sprintf('<info>%s</info> version <comment>%s</comment>', $this->getName(), $this->getVersion());
    }

    return sprintf('<info>%s</info>', $this->getName());
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's what I had done at first but I thought I was less clear. Anyway I can change no problems !

@dunglas
Copy link
Member

dunglas commented Jan 14, 2016

👍

@fabpot
Copy link
Member

fabpot commented Jan 14, 2016

Thank you @polc.

fabpot added a commit that referenced this pull request Jan 14, 2016
…rsion set (polc)

This PR was squashed before being merged into the 2.3 branch (closes #17326).

Discussion
----------

[Console] Display console application name even when no version set

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

When displaying help of an Application with a name but no version, it show "Console Tool" instead of the application name.

Commits
-------

61e810e [Console] Display console application name even when no version set
@fabpot fabpot closed this Jan 14, 2016
@fabpot fabpot mentioned this pull request Jan 14, 2016
@polc polc deleted the console_application_name branch January 14, 2016 10:02
@polc polc restored the console_application_name branch January 14, 2016 10:02
@polc polc deleted the console_application_name branch January 14, 2016 10:02
This was referenced Jan 14, 2016
@fabpot fabpot mentioned this pull request Feb 3, 2016
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

7 participants