nicolas-grekas
Repos
92
Followers
1735
Following
5

The Symfony PHP framework

28148
8786

A set of abstractions extracted out of the Symfony components

3534
16

Give thanks (in the form of a GitHub ★) to your fellow PHP package maintainers (not limited to Symfony components)!

7856
35

PHP polyfills

2303
117

Events

issue comment
Reorder priorities of PSR-17 factories

Looks like I missed following the links you submitted :grimacing: Let's revive those.

Created at 10 hours ago
issue comment
Reorder priorities of PSR-17 factories

nyholm/psr7 just happens to be stable. It is well maintained AFAIK. Did you submit an issue there? What about a fix?

Created at 10 hours ago

[FrameworkBundle] Workflow - Fix LogicException about a wrong configuration of "enabled" node

[FrameworkBundle] Fix wiring session.handler when handler_id is null

[HttpFoundation] Use separate caches for IpUtils checkIp4 and checkIp6

bug #49758 [HttpFoundation] Use separate caches for IpUtils checkIp4 and checkIp6 (danielburger1337)

This PR was squashed before being merged into the 5.4 branch.

Discussion

[HttpFoundation] Use separate caches for IpUtils checkIp4 and checkIp6

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #49749 | License | MIT | Doc PR | N/A

Fixed issue #49749

Commits

cf9b132425 [HttpFoundation] Use separate caches for IpUtils checkIp4 and checkIp6

[HttpClient] Add hint about timeout and max_duration options

minor #49791 [HttpClient] Add hint about timeout and max_duration options (Kocal)

This PR was merged into the 5.4 branch.

Discussion

[HttpClient] Add hint about timeout and max_duration options

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | yes/no | Tickets | Fix #... | License | MIT | Doc PR | symfony/symfony-docs#...

When reading the PHPDoc or the documentation (https://symfony.com/doc/current/http_client.html#dealing-with-network-timeouts), we don't know if we must pass seconds or milliseconds.

I believe max_duration is waiting for seconds, given the code from the CurlHttpClient:

Added hint for timeout option too.

Commits

eb4b511329 [HttpClient] Add hint about timeout and max_duration options

[Notifier] Add bridge documentation

minor #49782 [Notifier] Add bridge documentation (MrYamous)

This PR was squashed before being merged into the 5.4 branch.

Discussion

[Notifier] Add bridge documentation

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | From a doc issue | License | MIT | Doc PR |

From Fabien's comment on doc issue, move Notifier Bridge documentation to their Readme

Commits

8d2a02d139 [Notifier] Add bridge documentation

bug #49682 [FrameworkBundle] Workflow - Fix LogicException about a wrong configuration of "enabled" node (adpauly)

This PR was merged into the 5.4 branch.

Discussion

[FrameworkBundle] Workflow - Fix LogicException about a wrong configuration of "enabled" node

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | / | License | MIT | Doc PR | /

When I try to configure the YAML node "enabled" under a workflow configuration:

framework:
    workflows:
        order_approval_state:
            enabled: false # <--- HERE
            type: state_machine
            supports:
                - AppBundle\Entity\OrderApproval

The following exception is thrown:

//vendor/symfony/symfony/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
if (isset($workflow['enabled']) && false === $workflow['enabled']) {
    throw new LogicException(sprintf('Cannot disable a single workflow. Remove the configuration for the workflow "%s" instead.', $workflow['name']));
}

But it actually throws the following PHP error:

In Configuration.php line 262:

  Notice: Undefined index: name

I think we should use the $key variable instead of this undefined index. $key contains the workflow name.

When changing, I get the attempted exception:

In Configuration.php line 262:

  Cannot disable a single workflow. Remove the configuration for the workflow "order_approval_state" instead.

(I took a look in 6.2, it doesn't fix yet.)

Commits

14a4fcfedd [FrameworkBundle] Workflow - Fix LogicException about a wrong configuration of "enabled" node

[VarDumper] Disable links for IntelliJ platform

bug #49274 [VarDumper] Disable links for IntelliJ platform (SerafimArts)

This PR was squashed before being merged into the 5.4 branch.

Discussion

[VarDumper] Disable links for IntelliJ platform

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | License | MIT

IntelliJ IDEA / PHP Storm does not support hrefs (TBD: Perhaps only on Windows OS?)

This fix detects that the console was launched from an IDEA environment by IDEA_INITIAL_DIRECTORY env variable and disables href rendering.

~Unit tests are missing, because it cannot be tested in simple ways.~

UPD: Added tests by analogy with the proposal in https://github.com/symfony/symfony/pull/49274#issuecomment-1421644128

Before

image

After

image

Commits

f995db42c0 [VarDumper] Disable links for IntelliJ platform

[FrameworkBundle] Improve documentation about translation:extract --sort option

The --sort option only works with --dump-messages (i.e. not with --force). It took me some time to realize that, and the documentation didn't help me since it was showing an example that didn't work.

Contribute to https://github.com/symfony/symfony/issues/37918

bug #49189 [FrameworkBundle] Improve documentation about translation:extract --sort option (marien-probesys)

This PR was submitted for the 6.3 branch but it was merged into the 5.4 branch instead.

Discussion

[FrameworkBundle] Improve documentation about translation:extract --sort option

| Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | Contribute to #37918 | License | MIT | Doc PR | N/A

The --sort option only works with --dump-messages (i.e. not with --force). It took me some time to realize that, and the documentation didn't help me since it was showing an example that didn't work.

It would have been better to also apply the --sort option to --force as requested in #37918, but I don't have that much time to understand how to do it. At least it could save some time to people ;)

It's my first PR for Symfony, so let me know if I forget something or did something wrong!

Commits

1dbf13e43b [FrameworkBundle] Improve documentation about translation:extract --sort option

bug #49745 [FrameworkBundle] Fix wiring session.handler when handler_id is null (nicolas-grekas)

This PR was merged into the 5.4 branch.

Discussion

[FrameworkBundle] Fix wiring session.handler when handler_id is null

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | -

When the session handler_id is null, we currently set the $handler argument of the storage/factory services to null, which means to them "create your own handler internally from the native one". This means that the session.handler service is "a lie": it's not the real handler used by the storage. It also means that the %session.save_path% parameter is lying too, because it is set to %kernel.cache_dir%/sessions (by default), while the storage will use ini_get('session.save_path') in practice.

This issue is 10 years old... #5290

Commits

e23be58348 [FrameworkBundle] Fix wiring session.handler when handler_id is null

[FrameworkBundle] Add missing monolog channel tag for messenger services

bug #49843 [FrameworkBundle] Add missing monolog channel tag for messenger services (rmikalkenas)

This PR was merged into the 5.4 branch.

Discussion

[FrameworkBundle] Add missing monolog channel tag for messenger services

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | -

Commits

290c88e29d [FrameworkBundle] Add missing monolog channel tag for messenger services

[Cache] Fix storing binary keys when using pgsql

bug #49848 [Cache] Fix storing binary keys when using pgsql (nicolas-grekas)

This PR was merged into the 5.4 branch.

Discussion

[Cache] Fix storing binary keys when using pgsql

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #49748 | License | MIT | Doc PR | -

Commits

be88fd00f8 [Cache] Fix storing binary keys when using pgsql

[Cache] Removing null coalescing assignment operator on 5.4

minor #49862 [Cache] Fix php7 compat on 5.4: null coalescing assignment operator (weaverryan)

This PR was merged into the 5.4 branch.

Discussion

[Cache] Fix php7 compat on 5.4: null coalescing assignment operator

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | None | License | MIT | Doc PR | Not needed

It looks like #49848 accidentally introduced a syntax not compat with php 7.2/7.3. So, an annoying little PR to fix that.

Commits

7e5fe59c6d [Cache] Removing null coalescing assignment operator on 5.4

Created at 10 hours ago
pull request closed
[Cache] Fix php7 compat on 5.4: null coalescing assignment operator

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | None | License | MIT | Doc PR | Not needed

It looks like #49848 accidentally introduced a syntax not compat with php 7.2/7.3. So, an annoying little PR to fix that.

Created at 11 hours ago

[Cache] Removing null coalescing assignment operator on 5.4

minor #49862 [Cache] Fix php7 compat on 5.4: null coalescing assignment operator (weaverryan)

This PR was merged into the 5.4 branch.

Discussion

[Cache] Fix php7 compat on 5.4: null coalescing assignment operator

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | None | License | MIT | Doc PR | Not needed

It looks like #49848 accidentally introduced a syntax not compat with php 7.2/7.3. So, an annoying little PR to fix that.

Commits

7e5fe59c6d [Cache] Removing null coalescing assignment operator on 5.4

Created at 11 hours ago
issue comment
[Cache] Fix php7 compat on 5.4: null coalescing assignment operator

Thank you @weaverryan.

Created at 11 hours ago
delete branch
nicolas-grekas delete branch cache-pg-bin
Created at 11 hours ago

[FrameworkBundle] Fix auto-discovering validator constraints

Created at 18 hours ago
pull request closed
[Translation] Extract constraint class names instead of relying on service declaration

| Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #49397 | License | MIT | Doc PR |

This is an alternative solution to this one involving a fix translator side. See it as a discussion support first, more than a definitive proposal.

Instead of relying on service declaration to inject the constraint names to the ConstraintVisitor, we extract them from files and return all class names of classes extending Constraint.

The private method looks like the one we can find in the DebugCommand and I'm not really sure it has its place directly in the TranslatorPass but this is a first draft and I hope someone can lead me in the right direction to move stuff at the right place.

Contrary to my first PR this solution excludes userland constraints. It also exclude constraints from other places than Symfony/Component/Validator/Constraints directory (like UniqueEntity for example).

To include them all, we would probably need to:

  • add the service declaration of external constraint like UniqueEntity
  • keep the old way to get all tagged services
  • merge the result with the one provided in this PR

Userland constraint would be included at the condition of being declared as services.

Created at 18 hours ago
issue comment
[Translation] Extract constraint class names instead of relying on service declaration

Closing in favor of #49850, thanks for raising this!

Created at 18 hours ago
issue comment
[FrameworkBundle] Fix auto-discovering validator constraints

PR updated to target 6.2

A solution for the edge case UniqueEntity? As it's in the doctrine bridge and the validator is not a service, this one does not get extraction.

To me, the corresponding validator is a service, so there is nothing to do.

Created at 18 hours ago

[FrameworkBundle] Fix auto-discovering validator constraints

Created at 18 hours ago

update documentation for telegram bridge notifier

minor #49811 [Notifier] Update documentation for telegram bridge notifier (MrYamous)

This PR was merged into the 6.2 branch.

Discussion

[Notifier] Update documentation for telegram bridge notifier

| Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | From doc - Following #49782 | License | MIT | Doc PR |

Following #49782 to have up to date documentation in readme before removing it from symfony docs

Commits

1ddc90c21e update documentation for telegram bridge notifier

[Console] Add missing ZSH mention in DumpCompletionCommand help

bug #49765 [Console] Add missing ZSH mention in DumpCompletionCommand help (lyrixx)

This PR was merged into the 6.2 branch.

Discussion

[Console] Add missing ZSH mention in DumpCompletionCommand help

| Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR |


See https://github.com/symfony/symfony/blob/6.2/src/Symfony/Component/Console/Resources/completion.zsh

And I check, it doesn't exist on 5.4

Commits

fd957a98bb [Console] Add missing ZSH mention in DumpCompletionCommand help

[FrameworkBundle] Fix auto-discovering validator constraints

Created at 19 hours ago
issue comment
[HttpClient] Fix over-encoding of URL parts to match browser's behavior

OK, so this is #49579, fixed in the next release.

Created at 21 hours ago
issue comment
[HttpClient] Fix over-encoding of URL parts to match browser's behavior

Which part/character? You can know by generating the query string on your own to test.

Created at 22 hours ago
issue comment
[HttpClient] Add support for client pool

Yes OK, the request also must not start with a /. This is RFC 3986

Created at 1 day ago

[Form] fix missing static closure

Created at 1 day ago
delete branch
nicolas-grekas delete branch ser-wildcard-excludes-native
Created at 1 day ago