[Console] Allow dynamic properties in ProgressBar #47898
Conversation
|
I think this cannot be a durable solution. But I'm ok with merging this in lower branches (as the use case at hand seems legit) only if we find a proper solution for the dev branch that doesn't imply dynamic properties. |
|
Fine with merging it only to 6.2 if your condition to merge it lower would be refactoring of a whole thing; that would be a lot of work. |
|
Would removing |
|
yes definitely |
|
You can remove usage of dynamic properties by resolving the values before setting the format, and update the format string when the state changes. Here is a suggested solution: ostrolucky/stdinho#5 |
|
Indeed, so there is a different way to do this, thanks @GromNaN! So me personally I don't need this change anymore, we can close. |
…nstance (GromNaN) This PR was merged into the 6.3 branch. Discussion ---------- [Console] Add placeholder formatters per ProgressBar instance | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #34929 #47898 | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> The `ProgressBar` class abuse of static properties to define formats and callables for placeholders. While it is possible to set the format pattern directly without using the statically defined formats, the placeholder formatters have to be defined globally which add limitations and produce unintended side effects. Example on this command class: https://github.com/ostrolucky/stdinho/blob/76a54db4ff379fe428acc12cbe66d319a289eb3b/src/ProgressBar.php Commits ------- c5fdfbc Add placeholder formatters per ProgressBar instance
I rely on dynamic props in ProgressBar in https://github.com/ostrolucky/stdinho/blob/43c89658ae9d991f0daf7978d5d2877c2816cc68/src/ProgressBar.php#L32. However that will stop working starting PHP 8.2 unless this attribute is added. I see this attribute quite necessary here unless component is refactored, because of combination of static functions and final keyword usage.
Before this workaround, there was an issue #31193 that I closed because I've solved it by using dynamic properties.
I'm also open to move this to lower branch. The way I see it, this can go as low as Symfony 4.4, as it's perfectly safe patch.