Skip to content
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

FlagBagType support #183

Merged
merged 1 commit into from
Apr 19, 2022
Merged

FlagBagType support #183

merged 1 commit into from
Apr 19, 2022

Conversation

michnovka
Copy link
Contributor

This is initial commit for FlagBagType support.

Lets discuss in here

@michnovka
Copy link
Contributor Author

I have ported also BitmaskToBitFlagsValueTransformer, but I dont think this is going to be used now. In fact the whole as_value part makes little to no sense for me under the new implementation. One of the issues is that the new 2.x EnumType extends the Symfony's own EnumType and not the ChoiceType which was the case with 1.x version. So if we use DataTransformer that spits out int and not BackedEnum[], then the Symfony EnumType does not handle it and fails.

I have made BitmaskToBitFlagsValueTransformerTest so I think we can keep the DataTransformer in the package, maybe somebody will find it useful. But Id remove the as_value option from the FlagBagType and would only allow it to work with the FlagBagToCollectionTransformer (refactored version of SingleToCollectionFlagEnumTransformer)

WDYT @ogizanagi ?

@ogizanagi
Copy link
Member

ogizanagi commented Apr 18, 2022

I have ported also BitmaskToBitFlagsValueTransformer, but I dont think this is going to be used now. In fact the whole as_value part makes little to no sense for me under the new implementation.

I agree, let's remove this part until it is proven useful.
I don't want to strictly port every 1.x features as is, only what makes sense regarding the new PHP 8.1 native enums implementation. Also we don't need everything to land in 2.0.0, just what can impact the design and main interfaces.

@michnovka
Copy link
Contributor Author

Also we don't need everything to land in 2.0.0, just what can impact the design and main interfaces.

Yes, but I would like to include SQL enum, this PR and also FlagBag DBAL type for sure

@michnovka
Copy link
Contributor Author

This is ready for merge @ogizanagi

@michnovka
Copy link
Contributor Author

One important note: I have made the FlagBag::getBitmask() public. I think its a useful tool to be used with FlagBags and dont see any reason why it should stay protected.

src/FlagBag.php Outdated Show resolved Hide resolved
src/FlagBag.php Outdated Show resolved Hide resolved
src/FlagBag.php Outdated Show resolved Hide resolved
src/Bridge/Symfony/Form/Type/FlagBagType.php Outdated Show resolved Hide resolved
@michnovka
Copy link
Contributor Author

@ogizanagi changes committed, ready for merge

@ogizanagi ogizanagi mentioned this pull request Apr 19, 2022
24 tasks
@ogizanagi
Copy link
Member

Thanks @michnovka for working on this feature, this is much appreciated.

@ogizanagi ogizanagi merged commit 7600213 into Elao:2.x Apr 19, 2022
@michnovka
Copy link
Contributor Author

@ogizanagi np, I use this lib so I like to contribute. I will see if I can work on FlagBag DBAL type when #182 is merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants