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 multiple dispatch, fire, notify and send statements #623

Merged
merged 1 commit into from
May 6, 2023
Merged

Allow multiple dispatch, fire, notify and send statements #623

merged 1 commit into from
May 6, 2023

Conversation

faustbrian
Copy link
Contributor

@faustbrian faustbrian commented May 5, 2023

Alternative to #612 for #611

This new approach preprocesses the YAML content and transforms duplicate keys into keys that are suffixed with their line number to avoid key collisions. This also reverts #619.

Input
https://github.com/laravel-shift/blueprint/pull/623/files#diff-a4f4e06b3fdb4a8424880f544fca3c75a0ecf2ebb5158feba4963f7066e2d265

Output
https://github.com/laravel-shift/blueprint/pull/623/files#diff-72d9e08bf23239b5487ea8767fe78e8f3e9be1b14e5bd64e1ae2c4848da167f1

@faustbrian faustbrian marked this pull request as draft May 5, 2023 14:24
@faustbrian
Copy link
Contributor Author

This requires quite some regex voodoo to get it working. It's almost working but there seems to be something somewhere that causes duplication, for example when you run vendor/bin/phpunit --filter="output_writes_events" you'll get duplicated events. Not sure yet what causes that but I assume the multiple calls to the controller lexer.

@jasonmccreary
Copy link
Collaborator

I can take a stab if you want to leave it for the weekend.

@faustbrian
Copy link
Contributor Author

I'll continue tomorrow on it but feel free to poke around, probably something obvious that causes the array items to get duplicated 😅

@jasonmccreary
Copy link
Collaborator

Cool. You on Twitter? I'll give you a shout out.

@jasonmccreary
Copy link
Collaborator

jasonmccreary commented May 5, 2023

I think parsing out the controller section via regex will prove challenging and likely have edge cases. If you wanted to produce the dashed array syntax, then you'd might be better off going line by line through the controller section.

However, thinking about it more, these statements may not be sequential. And moving them into a single array, may not generate the code the user intended.

For example:

      validate: title, content
      save: ticket
      send: ReviewNotification to:ticket.reviewer with:ticket
      dispatch: SyncMedia with:ticket
      send: TicketOpened to:ticket.author with:ticket

As such, I think simply finding these indented properties and giving them a numerical suffix is the way to go. Then update the lexer to handle numbered properties when creating the statement. If so, you could likely revert #619.

src/Blueprint.php Outdated Show resolved Hide resolved
@faustbrian
Copy link
Contributor Author

@jasonmccreary updated the OP with the (I think) final changes

@faustbrian faustbrian marked this pull request as ready for review May 6, 2023 01:57
@jasonmccreary
Copy link
Collaborator

I foresee a lot of bugs, but here we go...

@jasonmccreary jasonmccreary merged commit e6f9cb1 into laravel-shift:master May 6, 2023
7 checks passed
@faustbrian faustbrian deleted the feat/multi-statements branch May 6, 2023 11:22
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.

None yet

2 participants