Skip to content

[Console] Only show namespaces with commands #19776

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

Closed
wants to merge 1 commit into from

Conversation

JhonnyL
Copy link
Contributor

@JhonnyL JhonnyL commented Aug 29, 2016

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

Only namespaces which has commands to show should be shown. Like orm:convert:mapping being an alias to doctrine:mapping:convert making orm namespace showing up empty.

screen shot 2016-08-29 at 14 59 25

return isset($commands[$name]);
});

if (empty($namespace['commands'])) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we use 0 === count(...) rather than empty for checking that an array has no element (refs #19320, #19312)

Copy link
Member

Choose a reason for hiding this comment

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

Actually just if (!$namespace['commands']) {

@chalasr
Copy link
Member

chalasr commented Aug 30, 2016

👍 Nice catch, command aliases are already listed in the description of the original command, no need for listing them twice.

@JhonnyL JhonnyL force-pushed the feature/empty-namespaces branch from 65a187f to 7214bdd Compare August 31, 2016 20:45

if (!$namespace['commands']) {
continue;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You should add a test case for this new logic

@ro0NL
Copy link
Contributor

ro0NL commented Sep 17, 2016

Oh dear... #19954

@JhonnyL any thoughts? I truly missed this one 😞

@fabpot
Copy link
Member

fabpot commented Sep 17, 2016

@ro0NL This one looks really good... but tests need to be added.

@ro0NL
Copy link
Contributor

ro0NL commented Sep 17, 2016

Agree, but im considering how this ends up when the alias related fixes from #19954 are applied afterwards.

Maybe this can go first and use my PR for the tests + alias fixes?

I can also refactor to this approach.. but i let @JhonnyL decide that.

@JhonnyL
Copy link
Contributor Author

JhonnyL commented Sep 20, 2016

@fabpot @ro0NL I wont have time to work on this atm. If tests do exist in the other PR maybe this one can go first as suggested?

If tests are needed for merge, then it's ok refactor to this approach and this PR can be closed.

@ro0NL
Copy link
Contributor

ro0NL commented Sep 20, 2016

@JhonnyL i more or less refactored to your approach 1459492

Shall we close this and continue in #19954?

@fabpot
Copy link
Member

fabpot commented Oct 5, 2016

Closing in favor of #19954

@fabpot fabpot closed this Oct 5, 2016
fabpot added a commit that referenced this pull request Mar 22, 2017
…(ro0NL)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[Console] Exclude empty namespaces in text descriptor

| Q | A |
| --- | --- |
| Branch? | "master" |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | comma-separated list of tickets fixed by the PR, if any |
| License | MIT |
| Doc PR | reference to the documentation PR, if any |

Before:

```
$ bin/console

 doctrine
  doctrine:mapping:convert                [orm:convert:mapping] Convert mapping information between supported formats.
 orm <----
 router
  router:match                            Helps debug routes by simulating a path info match

$ bin/console list orm

  [Symfony\Component\Debug\Exception\ContextErrorException]
  Warning: max(): Array must contain at least one element

$ bin/console list generate

Available commands for the "generate" namespace:
  generate:bundle      Generates a bundle
  generate:command     Generates a console command
  generate:controller  Generates a controller
```

After:

```
$ bin/console

 doctrine
  doctrine:mapping:convert                [orm:convert:mapping] Convert mapping information between supported formats.
 router
  router:match                            Helps debug routes by simulating a path info match

$ bin/console list orm

Available commands for the "orm" namespace:
  orm:convert:mapping  Convert mapping information between supported formats.

$ bin/console list generate

Available commands for the "generate" namespace:
  generate:bundle             Generates a bundle
  generate:command            Generates a console command
  generate:controller         Generates a controller
  generate:doctrine:crud      Generates a CRUD based on a Doctrine entity
  generate:doctrine:entities  Generates entity classes and method stubs from your mapping information
  generate:doctrine:entity    Generates a new Doctrine entity inside a bundle
  generate:doctrine:form      Generates a form type class based on a Doctrine entity
```

Overrules #19776 but also includes other fixes related to aliases that popped up when writing tests 👍

Commits
-------

d5a7608 [Console] Exclude empty namespaces in text descriptor
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