Fix BC after drop twig/extensions #6145

wbloszyk · 2020-06-16T14:48:18Z

Subject

This issue resolve problem with missing TextExtension after drop CoreBundle and upgrate sonata-project/twig-extensions to 1.x when sonata_admin.options.legacy_twig_text_extension is set to true. It is resolve by copy truncate function to new DeprecatedTextExtension class.

I am targeting this branch, because this change repsect BC.

Changelog

### Deprecated
- Deprecated `sonata_admin.options.legacy_twig_text_extension` configuration

wbloszyk · 2020-06-16T15:17:20Z

@VincentLanglet Can you check this is working for you? Also better add next parameter like here or @iternal class based on AbstractExtension to provide StringExtension::twigTruncateFilter() ?

VincentLanglet · 2020-06-16T18:29:41Z

@VincentLanglet Can you check this is working for you?

I forgot... Where did we talk about this issue ?
I need to find the context to reproduce the issue and try the fix.

Changelog here is not needed. This PR fix issue which can not be install in stable version.

Changelog is always needed when you fix something.

wbloszyk · 2020-06-16T18:39:26Z

Even if it not possible to install in stable? It is little strange, something like say: hey i fix unexisting bug.

You can find here:
#6140

VincentLanglet · 2020-06-16T18:58:38Z

Even if it not possible to install in stable? It is little strange, something like say: hey i fix unexisting bug.

In this PR, you

Fix something, it maybe can't actually occur, but could in the futur. Or it could have impact that we didn't see. It always better to log them.
Add deprecation. This has an impact on the user and should be log to.

The more verbose are commit, Pr, Changelog, Upgrade note, the more this project will be easy to maintain.

Even if it not possible to install in stable

Then how do you want that I test this ?
Merging into master and running tests ?

wbloszyk · 2020-06-17T09:05:48Z

@core23 @VincentLanglet @jordisala1991 RTM. Can you check it?

src/DependencyInjection/SonataAdminExtension.php

composer.json

wbloszyk · 2020-06-19T09:51:04Z

@VincentLanglet @jordisala1991 @core23 @greg0ire
I need:

merge this PR,
then merge 3.x branch to master,
drop some deprecations from master,

After that i can rebase #5461
After merge encore webpack and decide what we do with x-editable i will finish #5989

VincentLanglet · 2020-06-19T09:55:02Z

src/Twig/Extension/StringExtension.php

@@ -29,18 +29,28 @@
 * @internal
 *
 * @see https://github.com/symfony/symfony/pull/35649
+ * @deprecated since sonata-project/admin-bundle 3.69, to be removed in 4.0.


You should usually use sonata-project/admin-bundle 3.x because you can't be sure that he release will not be created before this merge.

I know, but https://github.com/sonata-project/SonataAdminBundle/pull/6145/files/733c89a78260f6cfa810a2d9d8d34bb2564a7383#diff-b96bf7cefcf2cc23077357469e7d42cbR23 ( * NEXT_MAJOR: Remove this class.) was add in 3.69, so in this case it should be OK.

src/DependencyInjection/Configuration.php

This commit replace using TextExtension from `twig/extensions` by copy truncate function to `DeprecatedTextExtension` method. Update src/DependencyInjection/Configuration.php Co-authored-by: Vincent Langlet <[email protected]>

wbloszyk · 2020-06-19T11:00:20Z

@VincentLanglet Can we merge it? Ask becouse #6140 (comment)

VincentLanglet · 2020-06-19T12:41:17Z

@VincentLanglet Can we merge it? Ask becouse #6140 (comment)

I see no reason for not merging this. :)

phansys · 2020-06-19T12:36:52Z

src/DependencyInjection/Configuration.php

@@ -176,6 +170,7 @@ public function getConfigTreeBuilder()
                        // NEXT_MAJOR : remove this option
                        ->booleanNode('legacy_twig_text_extension')
                            ->info('Use text filters from "twig/extensions" instead of those provided by "twig/string-extra".')
+                            ->setDeprecated('The child node "%node%" at path "%path%" is deprecated since sonata-project/admin-bundle 3.x and will be remove in 4.0.')


Suggested change

->setDeprecated('The child node "%node%" at path "%path%" is deprecated since sonata-project/admin-bundle 3.x and will be remove in 4.0.')

->setDeprecated('The child node "%node%" at path "%path%" is deprecated since sonata-project/admin-bundle 3.x and will be removed in version 4.0.')

I think version is unnecessary here.

phansys · 2020-06-19T12:40:52Z

src/DependencyInjection/Configuration.php

+                                        'Using `true` as value for "sonata_admin.options.legacy_twig_text_extension" option is deprecated since sonata-project/admin-bundle 3.64 and will be remove in 4.0'.
+                                        'You should set it to `false` before upgrade process.'


Suggested change

'Using `true` as value for "sonata_admin.options.legacy_twig_text_extension" option is deprecated since sonata-project/admin-bundle 3.64 and will be remove in 4.0'.

'You should set it to `false` before upgrade process.'

'Using `true` as value for "sonata_admin.options.legacy_twig_text_extension" option is deprecated since sonata-project/admin-bundle 3.64 and will be removed in version 4.0.'.

' Remove it or set its value to `false`.'

At first it should be set to flase, then can be delete in upgrade process becouse by default is it true. Remove it is bad idea

phansys · 2020-06-19T12:41:48Z

src/Twig/Extension/DeprecatedTextExtension.php

+ */
+final class DeprecatedTextExtension extends AbstractExtension
+{
+    public function twigTruncateFilter(Environment $env, ?string $value, int $length = 30, bool $preserve = false, $separator = '...')


Suggested change

public function twigTruncateFilter(Environment $env, ?string $value, int $length = 30, bool $preserve = false, $separator = '...')

public function twigTruncateFilter(Environment $env, ?string $value, int $length = 30, bool $preserve = false, $separator = '...'): string

phansys · 2020-06-19T12:45:04Z

@wbloszyk, I've left some suggestions just after the PR was merged. Please, take a look.

wbloszyk · 2020-06-19T15:10:33Z

@phansys Thanks for suggestions. I think this should be change in some bigger pedantic PR.

greg0ire · 2020-06-19T19:19:14Z

There is a deprecation, this is minor.

wbloszyk force-pushed the fix_text_extension branch from 0fb5608 to 79d3cbd Compare June 16, 2020 15:05

VincentLanglet requested a review from a team June 16, 2020 18:26

wbloszyk force-pushed the fix_text_extension branch 2 times, most recently from bcd10b2 to 3c57f96 Compare June 17, 2020 08:41

wbloszyk changed the title ~~Fix missing TextExtension from twig-extension 1.x~~ Fix BC after drop twig/extensions Jun 17, 2020

wbloszyk force-pushed the fix_text_extension branch from 3c57f96 to 5587412 Compare June 17, 2020 08:59

wbloszyk force-pushed the fix_text_extension branch from 5587412 to bc82182 Compare June 18, 2020 09:23

jordisala1991 reviewed Jun 19, 2020

View reviewed changes

src/DependencyInjection/SonataAdminExtension.php Outdated Show resolved Hide resolved

composer.json Outdated Show resolved Hide resolved

jordisala1991 added the patch label Jun 19, 2020

wbloszyk force-pushed the fix_text_extension branch from bc82182 to 2161e4d Compare June 19, 2020 07:23

wbloszyk requested a review from jordisala1991 June 19, 2020 07:27

wbloszyk force-pushed the fix_text_extension branch from 2161e4d to 733c89a Compare June 19, 2020 09:42

VincentLanglet requested a review from a team June 19, 2020 09:51

VincentLanglet reviewed Jun 19, 2020

View reviewed changes

src/DependencyInjection/Configuration.php Outdated Show resolved Hide resolved

wbloszyk force-pushed the fix_text_extension branch from edca82a to eef1af1 Compare June 19, 2020 10:01

wbloszyk requested a review from VincentLanglet June 19, 2020 10:01

Fix BC after drop twig/extensions

a94e623

This commit replace using TextExtension from `twig/extensions` by copy truncate function to `DeprecatedTextExtension` method. Update src/DependencyInjection/Configuration.php Co-authored-by: Vincent Langlet <[email protected]>

wbloszyk force-pushed the fix_text_extension branch from eef1af1 to a94e623 Compare June 19, 2020 10:10

jordisala1991 approved these changes Jun 19, 2020

View reviewed changes

jordisala1991 requested a review from a team June 19, 2020 10:49

core23 approved these changes Jun 19, 2020

View reviewed changes

VincentLanglet approved these changes Jun 19, 2020

View reviewed changes

VincentLanglet merged commit be445e1 into sonata-project:3.x Jun 19, 2020

phansys requested changes Jun 19, 2020

View reviewed changes

wbloszyk deleted the fix_text_extension branch June 19, 2020 15:11

greg0ire added minor and removed patch labels Jun 19, 2020

wbloszyk mentioned this pull request Jun 20, 2020

AdminLTE 3 implementation #5989

Closed

7 tasks

franmomu mentioned this pull request Sep 2, 2020

Move internal configuration to PHP #6284

Merged

greg0ire mentioned this pull request Sep 13, 2020

Add functional tests sonata-project/SonataDoctrineMongoDBAdminBundle#412

Merged

1 task

franmomu mentioned this pull request Jan 5, 2021

Unfixable sonata.deprecated_text.twig.extension #6745

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix BC after drop twig/extensions #6145

Fix BC after drop twig/extensions #6145

wbloszyk commented Jun 16, 2020 •

edited

Loading

wbloszyk commented Jun 16, 2020

VincentLanglet commented Jun 16, 2020

wbloszyk commented Jun 16, 2020 •

edited

Loading

VincentLanglet commented Jun 16, 2020 •

edited

Loading

wbloszyk commented Jun 17, 2020 •

edited

Loading

wbloszyk commented Jun 19, 2020

VincentLanglet Jun 19, 2020

wbloszyk Jun 19, 2020 •

edited

Loading

wbloszyk commented Jun 19, 2020

VincentLanglet commented Jun 19, 2020

phansys Jun 19, 2020

wbloszyk Jun 19, 2020

phansys Jun 19, 2020

wbloszyk Jun 19, 2020

phansys Jun 19, 2020

wbloszyk Jun 19, 2020

phansys commented Jun 19, 2020

wbloszyk commented Jun 19, 2020 •

edited

Loading

greg0ire commented Jun 19, 2020

	->setDeprecated('The child node "%node%" at path "%path%" is deprecated since sonata-project/admin-bundle 3.x and will be remove in 4.0.')
	->setDeprecated('The child node "%node%" at path "%path%" is deprecated since sonata-project/admin-bundle 3.x and will be removed in version 4.0.')

		'Using `true` as value for "sonata_admin.options.legacy_twig_text_extension" option is deprecated since sonata-project/admin-bundle 3.64 and will be remove in 4.0'.
		'You should set it to `false` before upgrade process.'

	public function twigTruncateFilter(Environment $env, ?string $value, int $length = 30, bool $preserve = false, $separator = '...')
	public function twigTruncateFilter(Environment $env, ?string $value, int $length = 30, bool $preserve = false, $separator = '...'): string

Fix BC after drop twig/extensions #6145

Fix BC after drop twig/extensions #6145

Conversation

wbloszyk commented Jun 16, 2020 • edited Loading

Subject

Changelog

wbloszyk commented Jun 16, 2020

VincentLanglet commented Jun 16, 2020

wbloszyk commented Jun 16, 2020 • edited Loading

VincentLanglet commented Jun 16, 2020 • edited Loading

wbloszyk commented Jun 17, 2020 • edited Loading

wbloszyk commented Jun 19, 2020

VincentLanglet Jun 19, 2020

Choose a reason for hiding this comment

wbloszyk Jun 19, 2020 • edited Loading

Choose a reason for hiding this comment

wbloszyk commented Jun 19, 2020

VincentLanglet commented Jun 19, 2020

phansys Jun 19, 2020

Choose a reason for hiding this comment

wbloszyk Jun 19, 2020

Choose a reason for hiding this comment

phansys Jun 19, 2020

Choose a reason for hiding this comment

wbloszyk Jun 19, 2020

Choose a reason for hiding this comment

phansys Jun 19, 2020

Choose a reason for hiding this comment

wbloszyk Jun 19, 2020

Choose a reason for hiding this comment

phansys commented Jun 19, 2020

wbloszyk commented Jun 19, 2020 • edited Loading

greg0ire commented Jun 19, 2020

wbloszyk commented Jun 16, 2020 •

edited

Loading

wbloszyk commented Jun 16, 2020 •

edited

Loading

VincentLanglet commented Jun 16, 2020 •

edited

Loading

wbloszyk commented Jun 17, 2020 •

edited

Loading

wbloszyk Jun 19, 2020 •

edited

Loading

wbloszyk commented Jun 19, 2020 •

edited

Loading