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

Allow baking enums. #968

Merged
merged 9 commits into from Dec 27, 2023
Merged

Allow baking enums. #968

merged 9 commits into from Dec 27, 2023

Conversation

dereuromark
Copy link
Member

@dereuromark dereuromark commented Dec 22, 2023

bin/cake bake enum FooBar

or

bin/cake bake enum FooBar -i

Next step could be to use conventions to auto map/detect enums for table fields:

  • {EntityName}{FieldName} enum

E.g. Users table with field status: User entity and UserStatus enum would map and get added to forms etc.


the fails are unrelated and will be green once cakephp/cakephp#17482 is released with 5.0.4

tests/comparisons/Model/testBakeEnum.php Outdated Show resolved Hide resolved
tests/comparisons/Model/testBakeEnum.php Outdated Show resolved Hide resolved
tests/comparisons/Model/testBakeEnum.php Outdated Show resolved Hide resolved
templates/bake/Model/enum.twig Outdated Show resolved Hide resolved
@dereuromark
Copy link
Member Author

dereuromark commented Dec 23, 2023

Given the description I think --backed=string|int should suffice as option.

$ bin/cake bake enum FooBar -h
Bake (backed) enums for use in models.

Usage:
cake bake enum [options]

Options:

--backed, -b      If using backed enums. Set to `string` or `int`.
--connection, -c  The datasource connection to get data from.

@dereuromark
Copy link
Member Author

dereuromark commented Dec 23, 2023

In a 2nd step we could think of allowing argument/option to dictate the enum values, similar to bake migrations etc:

Backed int

0: inactive, 1:active, ...

Backed string

nope: inactive, 'yep yep':active, ...

Default

inactive, active, ...

In a 3rd step: If those would be part of the db field comment, it could be used to auto-generate the whole enum based on that, making it super RAD.

src/Command/EnumCommand.php Outdated Show resolved Hide resolved
templates/bake/Model/enum.twig Outdated Show resolved Hide resolved
src/Command/EnumCommand.php Outdated Show resolved Hide resolved
Co-authored-by: ADmad <ADmad@users.noreply.github.com>
@ADmad ADmad changed the base branch from 3.x to 3.next December 24, 2023 05:13
@dereuromark
Copy link
Member Author

Looks like we can bake enums now for xmas:)

@dereuromark
Copy link
Member Author

Given that we only now have backed, and string vs int
Couldnt we just add boolean --int flag? -b ... seems not necessary in that case.

@ADmad
Copy link
Member

ADmad commented Dec 24, 2023

--int has just 1 character less than -b int :)

I would prefer if we keep the more descriptive name.

@dereuromark
Copy link
Member Author

Should it be just type/t though? Since it is clear it is a backing type on backed enums :)

@ADmad
Copy link
Member

ADmad commented Dec 24, 2023

I can live with type if others are in favor.

@dereuromark
Copy link
Member Author

Never mind, t is theme :)

That said: Shouldnt it throw an error if you define a short twice?

--theme, -t       The theme to use when baking code. (choices:
                  Bake|BootstrapUI|Migrations|Queue|Search|Setup|StateMachine)
--type, -t        The return type for the backed enum class

No error here..

@ADmad
Copy link
Member

ADmad commented Dec 24, 2023

Shouldnt it throw an error if you define a short twice?

Guess we are missing duplicate naming checks.

…#971)

* Make backing type a bool, since it defaults anyway on 1 of 2 options.

* Use --int / -i as flag.
@dereuromark
Copy link
Member Author

@ADmad it is now simpler, only one char for int

So all good?

@dereuromark dereuromark merged commit 8fd6290 into 3.next Dec 27, 2023
7 of 8 checks passed
@dereuromark dereuromark deleted the model-enum branch December 27, 2023 17:47
@dereuromark dereuromark mentioned this pull request Dec 28, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants