Automatically generate helpers #842

netpok · 2019-10-28T14:40:20Z

This pull request allows automatic helper generation. This is achieved by listening on the command finished event of migrate, migrate:rollback and package:discover commands.

I added this feature disabled by default, but it could be default on in the next major version.

Feel free to rename the config variable, this was my first idea.

src/Listeners/GenerateHelpers.php

netpok · 2019-10-29T12:33:38Z

I think this pull request is complete from my side. Of course suggestions are welcomed.

mfn · 2020-01-04T15:57:43Z

This competes with #797

netpok · 2020-01-25T21:22:37Z

After comparing the two solution my opinion is:

The other solution adds an extra event check to every single database query, it doesn't do anything as the recorder is not enabled but it's still an overhead (I know this is only a dev package). That issue can be simply rectified by registering the query listener only on the started event.
I'm not sure that it's necessary to only update the models that were affected by the migration, it is much easier to update all the models.
It only resolves the models under the app path so any external models like the cashier subscription model will be skipped.

Now the other side of the coin:

The config structure is probably better than mine.
The use of the MigrationsStarted event is a nice catch that I missed, since I made this pull request I noticed a problem where only a full match triggers the update, so for example migrate:roll will not trigger the model generation while it still rolls the migrations back.

The best of the two would be in my opinion:

Adopt the other config syntax
Update the models simply on the MigrationsEnded event (Probably just trigger a flag on that event and still handle it in the CommandFinished event where we can get a valid output interface for the artisan call)
Update the helpers when the package:discover command is finished since this command is usually executed by the composer there is a low chance of missing characters. Sadly there is no PackagesDiscovered event as far as I know

I considered dropping the helper update part in favor of the post update section but it looks quite hacky to me. If the script runs in a non dev environment it fails with an error code to hide the error massage and make composer not fail one needs to use the following:

@php artisan ide-helper:generate > /dev/null || true

While this solves the problem I'm not sure that this command does anything usable under windows.

@mfn and anyone else: Please share your thoughts about this.

mfn · 2020-01-25T22:04:21Z

I didn't share this information yet but when I found the two PRs doing the same, my initial gut feeling was (and still is) from a technical approach I prefer this PR.

But I also agree with your assessment regarding the config options 😄

However before putting more work into this, IMHO @barryvdh should voice his preference here (this PR, other PR or no PR)

mortenscheel · 2020-02-01T22:44:34Z

I'm the author of the other PR, and I agree with everything you've both said.

The reason I decided to only update the affected models was that I was having a weird issue with git at the time. After running ide-helper:models, every single model file (even unmodified ones) would show up as modified. It's not an issue any longer, and I suppose it was just a misconfiguration of git's core.autocrlf config.

So yeah, that complicated workaround wasn't necessary after all. Just update everything on MigrationsEnded.

mfn · 2020-02-02T11:26:20Z

@mortenscheel thanks for your feedback!

Would you then mind closing your PR in favour of this one?

jpickwell · 2020-02-26T16:25:27Z

src/Listeners/GenerateHelpers.php

+                $this->generate('meta', $event->output);
+                break;
+            case "migrate":
+            case "migrate:rollback":


Models should also be updated for "migrate:fresh," "migrate:refresh," and "migrate:reset" commands.

Suggested change

case "migrate:rollback":

case "migrate:fresh":

case "migrate:refresh":

case "migrate:reset":

case "migrate:rollback":

jpickwell · 2020-02-26T16:27:15Z

src/Listeners/GenerateHelpers.php

+     */
+    public function handle(CommandFinished $event)
+    {
+        switch ($event->command) {


If a command fails, or has any of the --help, --version, or --pretend flags, then "ide-helper" commands should not be run.

Suggested change

switch ($event->command) {

if ($this->eventIsInvalid($event)) {

return;

}

switch ($event->command) {

See following comment for implementation of eventIsInvalid method.

jpickwell · 2020-02-26T16:28:08Z

src/Listeners/GenerateHelpers.php

+                break;
+        }
+    }
+


Suggested change

/**

* @param \Illuminate\Console\Events\CommandFinished $event

*

* @return bool

*/

protected function eventIsInvalid(CommandFinished $event): bool

{

/*

* The event is invalid if any of the following are true:

* 1. exitCode is not zero (0); i.e., the command failed

* 2. input does not have a "command" argument; this happens when the

* -V/--version flag is used

* 3. input has "help" option (-h/--help)

* 4. input has "pretend" option (--pretend)

*/

return $event->exitCode !== 0 ||

!$event->input->hasArgument("command") ||

$this->hasOption($event, "help") ||

$this->hasOption($event, "pretend");

}

You have a point, this should be mostly solved by using MigrationsEnded event as a trigger as mentioned above, nut it can be definitely used on the package discover command, because as far as I know there is no PackageDiscovered event. Thanks for the suggestions.

mortenscheel · 2020-04-07T12:34:01Z

Looking forward to having this feature added. Any progress?

netpok · 2020-04-07T13:25:30Z

I'm still waiting response from the project maintainers about this pull request.

mfn · 2020-04-07T18:06:11Z

:/

ping @barryvdh 🤗

netpok · 2020-05-16T12:19:55Z

I will try to make a pull request to the framework with a PackagesDiscovered event so we could use that.

mfn

@netpok be aware that this library supports 5.5 and your new event would only be in L7+ I guess

I'm also not 110% sure it's really worth to have this but

@barryvdh what's your take? See #842 (comment)

mfn · 2020-06-28T14:13:21Z

composer.json

@@ -26,6 +26,9 @@
        "scrutinizer/ocular": "~1.1",
        "squizlabs/php_codesniffer": "^3"
    },
+    "suggest": {
+        "illuminate/events": "Required for automatic helper generation (^5.5|^6)."


This should also have |^7 in the meantime

Time has passed:

^5.5 can be removed

^7 and ^8 added 😄

mfn · 2020-06-28T14:14:58Z

src/IdeHelperServiceProvider.php

@@ -37,6 +39,10 @@ class IdeHelperServiceProvider extends ServiceProvider
     */
    public function boot()
    {
+        if ($this->app['config']->get('ide-helper.listen_on_commands')) {


the default for get() (2nd parameter) should be false for clarity sake

(it will work anyway because it's null, but…)

barryvdh · 2020-07-24T07:36:46Z

I like the idea, but not sure if this is the best way. For Composer commands you can just add it to composer.json, but for migrations I see the benefit. I think it would be enough to just have a config option 'post-migrate' => ['ide-helper:generate -RW'] or something?

mfn · 2020-07-31T14:04:31Z

I side with @barryvdh last comment, this would combine the "established way" (composer) but allows a way into the migration (with the flexibility of providing how)

netpok · 2020-07-31T14:43:33Z

I will update the PR for the changes, I found out that:
"test $COMPOSER_DEV_MODE -eq 0 || $PHP_BINARY artisan ide-helper:generate"
can be used in the post dump section without displaying or generating errors in the production environment

netpok · 2020-08-31T09:39:45Z

Sorry, I forgot this, I will have some time this week to update this

MatanYadaev · 2020-11-04T10:55:38Z

Is this PR dead?

netpok · 2021-02-27T10:34:58Z

Sorry for not handling this, I'm closing it in favor of #1163 which is an updated version for this.

netpok added 4 commits October 28, 2019 15:28

Add config option to enable helper file generation

46ee6d1

Add listener to generate helpers

f61f068

Register listener if enabled

df105fc

Add illuminate/events as suggested dependency

97895a0

mfn reviewed Oct 28, 2019

View reviewed changes

src/Listeners/GenerateHelpers.php Outdated Show resolved Hide resolved

netpok added 9 commits October 28, 2019 16:48

Make command paramers configurable

f530f3a

Add docBlocks

64ffc2d

Use array() instead of []

9856756

Fix config name

4ad4186

Split handle method

5313c74

Fix config

8a41b9a

Simplify listener

330e185

Allow disabling a command

8f330dd

Add metadata generation

803892b

mfn mentioned this pull request Jan 4, 2020

Update Model DocBlocks automatically after migrations #797

Closed

jpickwell reviewed Feb 26, 2020

View reviewed changes

mfn reviewed Jun 28, 2020

View reviewed changes

mortenscheel mentioned this pull request Aug 13, 2020

Laravel 7 support mortenscheel/laravel-ide-helper-plus#4

Open

netpok mentioned this pull request Feb 27, 2021

Auto generate models #1163

Merged

netpok closed this Feb 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatically generate helpers #842

Automatically generate helpers #842

netpok commented Oct 28, 2019 •

edited

Loading

netpok commented Oct 29, 2019

mfn commented Jan 4, 2020

netpok commented Jan 25, 2020 •

edited

Loading

mfn commented Jan 25, 2020

mortenscheel commented Feb 1, 2020

mfn commented Feb 2, 2020

jpickwell Feb 26, 2020 •

edited

Loading

jpickwell Feb 26, 2020 •

edited

Loading

jpickwell Feb 26, 2020 •

edited

Loading

netpok Apr 7, 2020

mortenscheel commented Apr 7, 2020

netpok commented Apr 7, 2020

mfn commented Apr 7, 2020

netpok commented May 16, 2020

mfn left a comment

mfn Jun 28, 2020

mfn Aug 31, 2020

mfn Jun 28, 2020

barryvdh commented Jul 24, 2020

mfn commented Jul 31, 2020

netpok commented Jul 31, 2020

netpok commented Aug 31, 2020

MatanYadaev commented Nov 4, 2020

netpok commented Feb 27, 2021

-            case "migrate:rollback":
+            case "migrate:fresh":
+            case "migrate:refresh":
+            case "migrate:reset":
+            case "migrate:rollback":

+    /**
+     * @param \Illuminate\Console\Events\CommandFinished $event
+     *
+     * @return bool
+     */
+    protected function eventIsInvalid(CommandFinished $event): bool
+    {
+        /*
+         * The event is invalid if any of the following are true:
+         * 1. exitCode is not zero (0); i.e., the command failed
+         * 2. input does not have a "command" argument; this happens when the
+         *    -V/--version flag is used
+         * 3. input has "help" option (-h/--help)
+         * 4. input has "pretend" option (--pretend)
+         */
+        return $event->exitCode !== 0 ||
+            !$event->input->hasArgument("command") ||
+            $this->hasOption($event, "help") ||
+            $this->hasOption($event, "pretend");
+    }

Automatically generate helpers #842

Automatically generate helpers #842

Conversation

netpok commented Oct 28, 2019 • edited Loading

netpok commented Oct 29, 2019

mfn commented Jan 4, 2020

netpok commented Jan 25, 2020 • edited Loading

mfn commented Jan 25, 2020

mortenscheel commented Feb 1, 2020

mfn commented Feb 2, 2020

jpickwell Feb 26, 2020 • edited Loading

Choose a reason for hiding this comment

jpickwell Feb 26, 2020 • edited Loading

Choose a reason for hiding this comment

jpickwell Feb 26, 2020 • edited Loading

Choose a reason for hiding this comment

netpok Apr 7, 2020

Choose a reason for hiding this comment

mortenscheel commented Apr 7, 2020

netpok commented Apr 7, 2020

mfn commented Apr 7, 2020

netpok commented May 16, 2020

mfn left a comment

Choose a reason for hiding this comment

mfn Jun 28, 2020

Choose a reason for hiding this comment

mfn Aug 31, 2020

Choose a reason for hiding this comment

mfn Jun 28, 2020

Choose a reason for hiding this comment

barryvdh commented Jul 24, 2020

mfn commented Jul 31, 2020

netpok commented Jul 31, 2020

netpok commented Aug 31, 2020

MatanYadaev commented Nov 4, 2020

netpok commented Feb 27, 2021

netpok commented Oct 28, 2019 •

edited

Loading

netpok commented Jan 25, 2020 •

edited

Loading

jpickwell Feb 26, 2020 •

edited

Loading

jpickwell Feb 26, 2020 •

edited

Loading

jpickwell Feb 26, 2020 •

edited

Loading