Update Generators to DCG 3

Pierre Paul Lefebvre → made their first commit to this issue’s fork.

Do we need to support Drush 11?

I left a comment in the PR, should have probably left it here instead :
This PR targets the 2.x branch, but you might want to create a new 3.x branch... Sorry for the big changes 😄

Drush 12 requires PHP8.1, so I think it would be wise to have a different branch for this version.

I think a 3.x version is in order, yes.

I would like to keep promoting adding a thumbnail by default. Any chance you can look into that? I will ping you once I have the 3.x branch created.

Alright, 3.x dev released.

I would be open to moving these generators into Drush core. Lets merge this and then if interested, please open an issue in Drush queue to discuss.

Oh! that is great news @moshe weitzman!

@e0ipso Awesome news! Thanks!
I will work on making the thumbnail work and update the target branch :)

This is now ready for review.
I have added support for thumbnail. I really wish I could come up with something cleaner, but that's what I was able to get.

For some background,
* in 2.x we used to create a File() manually and add it to the `assets` array directly. The `assets` array is now private, so it doesn't work anymore.
* When using addFile (and Im guessing it was the same issue in 2.x, which would explain point 1), the file gets parsed by the TwigRenderer and make everything crash.
* Listening to the post assets creation event could work, but we would not have the thumbnail in the list of the generated files
* The rendering process happens in the File::render function. I tried to create a new Binary file type which would not use the TwigRenderer, but then I had the same issue as point 1. `addFile` expects a string instead of a FileInterface or something.
* I tried to use the dumper set instead of calling copy directly, but the dumper expects an AssetCollection, which in turn is limited to File object, which are parsed by the TwigRenderer when loaded.

It would probably cleaner to make a PR in the drupal-code-generator project to add supports for binary files, but this works.
Im open to any changes if you have a better idea :)

The `assets` array is now private, so it doesn't work anymore.

The asset collection implements array access interface. So you still can add arbitrary files to it.

    $assets[] = new File('example.txt');

Wow now I feel really bad for missing that. Thanks!
It's a lot more clean now.

@Chi - would you be interested in a PR that moves this generator into DCG? If not, I'll take it into Drush.

Re #17. I think it makes sense since SDC is now part of Drupal core.

I think this looks good to me.i was not able to test manually yet.

@e0ipso do you need any help to close this? :)

Yes. As I was hinding in #19 I need someone to test this manually, since we don't have automated tests. Can you ensure everything works properly in both drush 11 and 12?

Ha sorry! Will do today for sure. Thanks

Everything works fine for drush 12, using a base project generated with ddev+composer.

For drush 11, it doesn't work at all, but it's also why we have this specific branch (3.x).
Branch 3.x requires drush 12, which in turn requires chi-teck/drupal-code-generator 3.x

chi-teck/drupal-code-generator:3.x is not backward compatible with version 2 (which is used by drush 11)

I was trying to test this and the command has changed.

```drush generate sdc:theme```

ChrisScrumping yes sadly, the way annotation works now, we cannot have a single command to work with theme and modules.
So the new commands are:

drush generate sdc:theme
drush generate sdc:module

@Chi, @moshe weitzman are you still willing to merge upstream, given that SDC is now in Drupal core?

I am thinking about releases and such. I don't want to confuse people into thinking they need a module if they don't in the end.

we cannot have a single command to work with both theme and modules

Technically it's possible. You can specify the generator type as GeneratorType::OTHER. However, in that case the generator will need to implement custom autocomplete for machine name that includes both modules and themes machine names. Also it'll need to override getDestination() method to determine where to dump the generated code.

By the way, is sdc:module generator really needed? If most of SDC plugins will be generated for themes I would consider dropping sdc:module generator and renaming sdc:module generator to sdc.

are you still willing to merge upstream

I think it makes sense to have it in DCG.

By the way, is sdc:module generator really needed?

I was trying to fix the issue without losing features, but I'm open to anything. drush generate sdc does seems a lot cleaner to me. I can see why the sdc:module would be important. Maybe I can default the sdc to sdc:theme.

Also it'll need to override getDestination() method to determine where to dump the generated code.

That's kind of what I was trying not to do :).

I think it makes sense to have it in DCG.

Im kind of new to contributing. Should I fork DCG, move my changes into the fork, make a merge request, add a comment here about the new PR and close the issue?
How do you (Chi/e0ipso) want to make it work?
I have a bit of time on my hands, I'm up for anything.

Im kind of new to contributing. Should I fork DCG, move my changes into the fork, make a merge request, add a comment here about the new PR and close the issue?

Yes, that's a correct procedure.

Development to this issue has been moved to : https://github.com/Chi-teck/drupal-code-generator/pull/109

The PR has been merged on the Chi-teck repository :).

Wonder if there is a timetable for this one?

hello @smustgrave ! This has been merged into drupal-code-generator, which means it's now available in drush core.

If you have an updated drush, you should be able to do :

drush generate sdc

If you don't have an updated drush and you're using composer, you should be able to do :

composer update drush -W #the -W flag might required, to force the update on the drupal-code-generator dependency.

Have fun!

There is nothing left to review.

Will test again but I had to downgrade drush to 11 to download this module.

Though confused the MR for this has not been merged so not sure how it’s fixed? Unless the solution is to use a different module, then this should be marked obsolete or description update. Also in the sdc documentation pages

@smustgrave, if you are using Drush 12, you don't need this module at all.

Drush 11 is scheduled for EOL in November 2023.
I suppose this module can be deprecated at the same time.

The project page should be updated to explain that this module is not needed anymore.

Automatically closed - issue fixed for 2 weeks with no activity.