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

Fix uniqueness of macroable objects on grammars #26461

Closed
wants to merge 1 commit into from
Closed

Fix uniqueness of macroable objects on grammars #26461

wants to merge 1 commit into from

Conversation

morloderex
Copy link
Contributor

@morloderex morloderex commented Nov 9, 2018

In my pr #24513 i added the macroable trait to the grammars outermost parent class.

But what i didn't realize is that if one where to try to register the same macro name into two different grammars only the last call to the macro method will be used sense the macro property is static and lives in the outmost parent.

So every call to macro would override another one.

@staudenmeir
Copy link
Contributor

Isn't this a breaking change for people who are using Grammar::macro()?

@morloderex
Copy link
Contributor Author

Yes, that is correct.

@morloderex
Copy link
Contributor Author

morloderex commented Nov 9, 2018

But i'm not sure how would you deal with class identities in enheritance?

@staudenmeir
Copy link
Contributor

For breaking changes, you should target the master branch (Laravel 5.8).

@taylorotwell
Copy link
Member

See above.

@saibotk
Copy link

saibotk commented Aug 9, 2022

@morloderex This seems to be a reasonable feature, is there any interest to pick this up again?

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.

4 participants