[DI][FrameworkBundle] Hide service ids that start with a dot #26921
Conversation
nicolas-grekas
force-pushed
the
container.hidden
branch
from
df5b6c3
to
c689088
Compare
|
ping @symfony/deciders this would be great in 4.1, the current state is clumsy. |
|
I like the idea. But Why did you choose a dot? I think it would be better to choose an underscore even if the meaning of a name starting with a dot in the unix FS is to hide it. The underscore is already used in the framework (or other lib) to mean "it's internal / private". I'm thinking to |
|
Good question :) |
|
I really like the idea as well. And tbh, hiding things that started with The result of the |
If we were to answer yes to this question, we couldn't do any improvement to any command we publish. The answer must be no. |
okay, go for it ;) |
| @@ -250,7 +255,7 @@ protected function findDefinitionsByTag(ContainerBuilder $builder, $showPrivate) | |||
| foreach ($builder->findTaggedServiceIds($tag) as $serviceId => $attributes) { | |||
| $definition = $this->resolveServiceDefinition($builder, $serviceId); | |||
|
|
|||
| if (!$definition instanceof Definition || !$showPrivate && !$definition->isPublic()) { | |||
| if ($showHidden xor '.' === $serviceId[0] ?? null) { | |||
There was a problem hiding this comment.
Can we use parentheses here? At least I have no idea by reading the code whether xor takes precedence over ?? or vice versa.
There was a problem hiding this comment.
Is this actually what we want at all? Won't we skip non hidden service now if $showHidden is true?
There was a problem hiding this comment.
parentheses added
There was a problem hiding this comment.
Can we do that in the other files too that use the same condition? :)
There was a problem hiding this comment.
never mind, I was looking at an outdated diff
nicolas-grekas
force-pushed
the
container.hidden
branch
from
c689088
to
f809d72
Compare
That's correct: by default, you get non-hidden services, and if you add |
| using the <info>--show-private</info> flag: | ||
|
|
||
| <info>php %command.full_name% --show-private</info> | ||
|
|
There was a problem hiding this comment.
You should add some docs about the new flag.
nicolas-grekas
force-pushed
the
container.hidden
branch
from
f809d72
to
cea051e
Compare
|
Thank you @nicolas-grekas. |
… a dot (nicolas-grekas) This PR was merged into the 4.1-dev branch. Discussion ---------- [DI][FrameworkBundle] Hide service ids that start with a dot | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #26630 | License | MIT | Doc PR | - Now that services are private by default, `debug:container` should display them by default. In fact, the public/private concept should not trigger a difference in visibility for this command. Instead, I propose to handle service ids that start with a dot as hidden services. PR should be ready. (For reference, I tried using a tag instead in #26891, but that doesn't work in practice when working on the implementation.) Commits ------- cea051e [DI][FrameworkBundle] Hide service ids that start with a dot
…as-grekas) This PR was merged into the 4.1-dev branch. Discussion ---------- [FrameworkBundle] Hide some lock-related services | Q | A | ------------- | --- | Branch? | 4.1 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - I don't know why, but I missed these in #26921 Commits ------- d06d8b2 [FrameworkBundle] Hide some lock-related services
Now that services are private by default,
debug:containershould display them by default.In fact, the public/private concept should not trigger a difference in visibility for this command.
Instead, I propose to handle service ids that start with a dot as hidden services.
PR should be ready.
(For reference, I tried using a tag instead in #26891, but that doesn't work in practice when working on the implementation.)