-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DI][FrameworkBundle] Hide service ids that start with a dot #26921
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
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parentheses added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do that in the other files too that use the same condition? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
never mind, I was looking at an outdated diff
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should add some docs about the new flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
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: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.)