Allow themes to use a starterkit.yml file so it is easier to use the theme generator

I plan to work on this at DrupalCon contrib and welcome others to share thoughts for how to make this simpler.

MR #4121 is currently a work in progress, but attempts to simplify the starterkit adoption by other themes by introducing support for a new yml file, *.starterkit.yml. This file in the starterkit_theme looks like the below:

delete:
  - '/src/StarterKit.php'
  - '/starterkit_theme.starterkit.yml'
no_edit: null
no_rename: null
info:
  hidden: null
  starterkit: null
  version: 1.0.0

The delete key specifies a list of files, directories, or globs to delete from the temporary working directory of the process. In the above example, the files related to making the theme a starterkit are removed

The no_edit key specifies a list of files, directories, or globs to avoid changing the internal contents of.
The no_rename key is similar, but prevents files from being renamed.

The info key specifies keys that should be changed from the current theme's info files. Keys can be added or changed in this file simply by setting their values, and they can be removed by setting their values to null.

For context, I'm +1 for this. The starterkit was built the way it is because it was built for a single purpose and we realized later that there could be benefit in allowing other themes to use it. I think makes sense to build an API like this to simplify the experience.

I am also +1 on this, we experienced a large amount of "manual work required afterwards" with a custom base theme we provide in a distribution install profile for D9/10 and I think this would help alleviate it.

Test failures are saying it can't find the Symfony Filesystem component. Is this because that's only a dev dependency?

https://git.drupalcode.org/project/drupal/-/blob/11.x/composer.json#L37-38

That is correct. On line 38 of GenerateThemeTest it does a composer install with --no-dev.

Also running the test locally without the --no-dev I still get a failure

Failed asserting that two strings are equal.
Expected :'starterkit_theme:9.4.0'
Actual   :'test_custom_theme:1.0.0'

Blocked by #3369120: Promote symfony/finder from dev-dependency to full dependency and #3369121: Promote symfony/filesystem from dev-dependency to full dependency

To fix, run:

    COMPOSER_ROOT_VERSION=11.x-dev composer update --lock

Ran this as recommended by the test results, but it failed with this:

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires drupal/core == 10.1.9999999.9999999-dev, it is satisfiable by drupal/core[10.1.x-dev] from composer repo (https://repo.packagist.org) but drupal/core[dev-3364885-making-a-theme, 11.x-dev] from path repo (repos/drupal/core) has higher repository priority. The packages from the higher priority repository do not match your constraint and are therefore not installable. That repository is canonical so the lower priority repo's packages are not installable. See https://getcomposer.org/repoprio for details and assistance.
  Problem 2
    - Root composer.json requires drupal/core-dev == 10.1.9999999.9999999-dev, it is satisfiable by drupal/core-dev[10.1.x-dev] from composer repo (https://repo.packagist.org) but drupal/core-dev[dev-3364885-making-a-theme, 11.x-dev] from path repo (repos/drupal/composer/Metapackage/DevDependencies) has higher repository priority. The packages from the higher priority repository do not match your constraint and are therefore not installable. That repository is canonical so the lower priority repo's packages are not installable. See https://getcomposer.org/repoprio for details and assistance.
  Problem 3
    - Root composer.json requires drupal/core-project-message == 10.1.9999999.9999999-dev, it is satisfiable by drupal/core-project-message[10.1.x-dev] from composer repo (https://repo.packagist.org) but drupal/core-project-message[dev-3364885-making-a-theme, 11.x-dev] from path repo (repos/drupal/composer/Plugin/ProjectMessage) has higher repository priority. The packages from the higher priority repository do not match your constraint and are therefore not installable. That repository is canonical so the lower priority repo's packages are not installable. See https://getcomposer.org/repoprio for details and assistance.
  Problem 4
    - Root composer.json requires drupal/core-recommended == 10.1.9999999.9999999-dev, it is satisfiable by drupal/core-recommended[10.1.x-dev] from composer repo (https://repo.packagist.org) but drupal/core-recommended[dev-3364885-making-a-theme, 11.x-dev] from path repo (repos/drupal/composer/Metapackage/CoreRecommended) has higher repository priority. The packages from the higher priority repository do not match your constraint and are therefore not installable. That repository is canonical so the lower priority repo's packages are not installable. See https://getcomposer.org/repoprio for details and assistance.
  Problem 5
    - Root composer.json requires drupal/core-vendor-hardening == 10.1.9999999.9999999-dev, it is satisfiable by drupal/core-vendor-hardening[10.1.x-dev] from composer repo (https://repo.packagist.org) but drupal/core-vendor-hardening[dev-3364885-making-a-theme, 11.x-dev] from path repo (repos/drupal/composer/Plugin/VendorHardening) has higher repository priority. The packages from the higher priority repository do not match your constraint and are therefore not installable. That repository is canonical so the lower priority repo's packages are not installable. See https://getcomposer.org/repoprio for details and assistance.
  Problem 6
    - Root composer.json requires drupal/drupal == 10.1.9999999.9999999-dev, found drupal/drupal[dev-3364885-making-a-theme, 11.x-dev] but these do not match your constraint and are therefore not installable. Make sure you fix the constraint as packages installed from symlinked path repos are updated even in partial updates and the one from the lock file can thus not be used.
  Problem 7
    - Root composer.json requires drupal/devel == 5.1.1.0 -> satisfiable by drupal/devel[5.1.1].
    - drupal/devel 5.1.1 requires drupal/core ^9 || ^10 -> satisfiable by drupal/core[9.0.0-alpha1, ..., 9.5.x-dev, 10.0.0-alpha1, ..., 10.1.x-dev] from composer repo (https://repo.packagist.org) but drupal/core[dev-3364885-making-a-theme, 11.x-dev] from path repo (repos/drupal/core) has higher repository priority. The packages from the higher priority repository do not match your constraint and are therefore not installable. That repository is canonical so the lower priority repo's packages are not installable. See https://getcomposer.org/repoprio for details and assistance.

I think the starterkit YAML file is a great addition, will it be worth to also provide a simple way to pass options? For dynamic changes the theme may need, for example, it could allow.Olivero to set the default color of the generated theme, or allow themes to provide basic options for customization and avoid manual changes.

It could be as simple as passing a --options to the generate theme command and pass this along to the starterkit postprocess.

If, as a developer, wants to run unit tests, you need to require, as a dev dependency, the package drupal/core-dev.

The package drupal/core-dev brings symfony/finder and symfony/filesystem ^6.3: https://github.com/drupal/core-dev/blob/11.x/composer.json#L32-L33

As this funcionality seems to be for developers, could make sense to require, as a dev dependency, the drupal/core-dev package?

Discussed with @andy-blum and I think we can refactor this to not require the Symfony dependencies. This script is a development tool. But it is executable without the dependency existing whereas things like PHPUnit tests are not. I think it'd be easier to land without using the Symfony components and avoid debating this situation.

I'm going to give it a shot.

I ran out of time today. The hardest part is recreating this with glob or the proper iterators:

      $finder = new Finder();
      $files = $finder
        ->in($this->tmp_dir)
        ->files()
        ->contains("/$old_str/")
        ->filter(fn ($file) => !in_array($file->getRelativePathname(), $this->paths_to_skip_edit));

Is there a reason (either way) to introduce/not introduce them?

The reason for trying to avoid the new dependencies was to speed this along.

If the code is really complicated, does this help make the case for including the dependencies?

I would argue yes.

My 50 cents regarding theme generator.

1. Theme generator should scan all folders and files inside of starterkit theme (excluding of course some trash like node_modules, etc)
2. Theme generator should be sensitive to the text cases. Somehow it should understand "my_theme", "myTheme", "my-theme", etc text cases
3. Theme generator should understand that sometimes it shouldn't change name. Ex. - sometimes in `package.json` in theme we can have specific npm packages which are partially contains sub-name of the starterkit name -> obviously name of npm package should not be changed (but for example "name" property in package.json should be changed).
4. Back to first point - starterkit theme can contain js files with drupal behaviors. And behavior name can include name of the starterkit theme -> `MyStarterkitThemeMySuperBehavior` <--- here theme generator script should change part of behavior name to the generated theme name.

Now.. how we can deal with all these?

We are using hygen node script to handle generation process from the starterkit theme. It allows us to put conditional comments everywhere where we need regarding what to change, where, which text case, etc etc etc.

So this process is automated through the "hygenic" comments.

This is blocking #3301173: Allow starterkit theme generator tool to clone Olivero, so matching issue metadata.

Oh wow, there's two blockers?! 😳

My 2cents: while working on a new piece of code to add into the Starterkit theme, I needed to replace mentions of starterkit_theme_ in more places:

• in /js/*.js files (see this line)
• inside hook function's code in *.module file (see this line)

I cross-link these changes here before I revert them as I was told better not to touch the Theme Generator because of the ongoing work on this present issue.

Symfony's finder & filesystem components are now full runtime dependencies [CR]

Test failures are now related to the generate theme command. Abridged failures below:

---- Drupal\Tests\Core\Command\GenerateThemeTest ----
    
Testing Drupal\Tests\Core\Command\GenerateThemeTest
FFFFFFF..                                                           9 / 9
(100%)

Time: 00:56.152, Memory: 12.00 MB

There were 7 failures:

1) Drupal\Tests\Core\Command\GenerateThemeTest::test
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'starterkit_theme:9.4.0'
+'test_custom_theme:1.0.0'


2)
Drupal\Tests\Core\Command\GenerateThemeTest::testGeneratingFromAnotherTheme
In InfoParserDynamic.php line 67:

  The 'core_version_requirement' key must be present in themes/test_custom_theme/test_custom_theme.info.yml


generate-theme [--name [NAME]] [--description [DESCRIPTION]] [--path
[PATH]] [--starterkit [STARTERKIT]] [--] 


Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'Theme generated successfully to themes/generated_from_another_theme'
+''


3) Drupal\Tests\Core\Command\GenerateThemeTest::testDevSnapshot
Failed asserting that 'test_custom_theme:1.0.0' matches PCRE pattern
"/^starterkit_theme\:9.4.0-dev#[0-9a-f]+$/".


4) Drupal\Tests\Core\Command\GenerateThemeTest::testContribStarterkit
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'starterkit_theme:1.20'
+'test_custom_theme:1.0.0'


5) Drupal\Tests\Core\Command\GenerateThemeTest::testContribStarterkitDevSnapshot
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'The source theme starterkit_theme has a development version number
(7.x-dev). Because it is not a git checkout, a specific commit could not be
identified. This makes tracking changes in the source theme difficult. Are
you sure you want to continue? (yes/no) [yes]:\n
- > Theme generated successfully to themes/test_custom_theme'
+'Theme generated successfully to themes/test_custom_theme'

6) Drupal\Tests\Core\Command\GenerateThemeTest::testContribStarterkitDevSnapshotWithGitNotInstalled
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'[ERROR] The source theme starterkit_theme has a development version
number     \n
-         (7.x-dev). Determining a specific commit is not possible because
git is\n
-         not installed. Either install git or use a tagged release to
generate a\n
-         theme.'
+'Theme generated successfully to themes/test_custom_theme'

7) Drupal\Tests\Core\Command\GenerateThemeTest::testCustomStarterkit
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'The source theme starterkit_theme does not have a version specified. This
makes tracking changes in the source theme difficult. Are you sure you want
to continue? (yes/no) [yes]:\n
- > Theme generated successfully to themes/test_custom_theme'
+'Theme generated successfully to themes/test_custom_theme'

FAILURES!
Tests: 9, Assertions: 32, Failures: 7.

The problem is that the new private method overrideThemeInfo is returning a 1, which means an error happened, but this is not being captured at all in this line (https://git.drupalcode.org/issue/drupal-3364885/-/blob/3364885-making-a-...), therefore it continues and runs all the way to line 225.

So it seems like the new refactoring should read the error and bail out in that case.

Can confirm #28.1. the error message is:

In RecursiveDirectoryIterator.php line 110:

  RecursiveDirectoryIterator::__construct(/var/folders/y2/7c4nwg392t7b74mx3zw4ns9r0000gn/T/drupal-starterkit-theme-ab4c58516c3a17efd48fe0e7db5bea236557181f465717.64005187/tests)
  : Failed to open directory: No such file or directory

I also noticed that the no_edit and no_rename are not working as expected:

# parent.starterkit.yml
no_edit:
  - '/js/parent-init.js'
no_rename:
  - '/js/parent-init.js'

Running:

php web/core/scripts/drupal generate-theme --name "Sub Theme" --path themes/custom --starterkit parent sub_theme

I expected to see file: themes/custom/sub_theme/js/parent-init.js but instead saw themes/custom/sub_theme/js/sub_theme-init.js and I expected to see the file copied in place as-is:

// themes/custom/parent/js/parent-init.js
(function (Drupal) {
  Drupal.behaviors.parentInit = {
    attach(context) {
      Parent.init();
    },
  };
})(Drupal);

But instead saw:

// themes/custom/sub_theme/js/sub_theme-init.js
(function (Drupal) {
  Drupal.behaviors.sub_themeInit = {
    attach(context) {
      SubTheme.init();
    },
  };
})(Drupal);

I've been working on this @ FLDC. Taking a break but keeping it assigned. Most bugs addressed, and tests added, but needs some TLC refactoring

I helped with Matt's globbing at Drupal's second best camp

I wasn't able to finish before leaving FLDC but I'd say it's 80%. There a few things I want to take care of to tighten up the code and there is one of two missing test cases.

It is manually testable though. I'd hold on code reviews until I can finish my changes. I'll work on them in the next few days

Unassigning for now. I'm not moving to NR yet because the tests will fail, as I left an incomplete test

  public function testNoEdit(): void {
    $this->markTestIncomplete('needs to be written');
  }

But it is ready for general review and testing.

Ready for review! I wrote the last test and did my final bits of refactoring.

Ah, I know why https://git.drupalcode.org/project/drupal/-/merge_requests/4121#note_275423 good find

Addressing feedback. And adding verbose output. So if someone passed -v it displays verbose messaging.

Addressed findings and added a test for false being removed. Added extra messages which are displayed when -vvv is passed for debug output.

That'd be really hard to figure out, especially if they're using wildcards/globs. We could do the check if no wildcard was found in the string though

Blah, I broke tests with the debug output.

Pushed a commit to fix the tests 🤞

Took a stab at the CR https://www.drupal.org/node/3425844

Trying for a more informative title and commit message.

@mglaman - It might be worth fleshing out the bullet point about the info.yml file. Specifically, that you can override existing values and *remove* values with `null`.

@andy-blum updated CR

Just some small things on the MR. Looks really good.

Addressed feedback. Left one comment.

Remaining item addressed. Failure is on Drupal.Tests.Composer.Plugin.Scaffold.Functional.ManageGitIgnoreTest:: testUnmanagedGitIgnoreWhenGitNotAvailable which is unrelated.

Committed and pushed 6c2ed72990 to 11.x and d82e52c0cf to 10.3.x. Thanks!

This is great! Please create a change record that will help folks update docs and tutorials.

One was created and published when the MR was merged: https://www.drupal.org/node/3425844

This is awesome. Created a starterkit theme. Only issue is that any "dot" file was not copied over— .gitignore, .nvmrc —and these are pretty useful files for the new themes. Is this intentional or an oversight? Is there a way to override the behavior?

@mlncn can you open a new issue? "Dotfiles ignored when copied"

-  public function __construct(string $name = NULL) {
+  public function __construct(string $name = NULL, ?string $root = NULL) {

This parameter gets added with no documentation.

AND it's a parameter that's only used in tests??? That's not a good pattern.

Having the same issue on Radix with the Dotfiles not getting copied over.

Is there a way to exclude some strings to be replaced.

For example you have:

lollapalooza-cms and the initial theme was lollapalooza.

If I generate a new theme named new-theme

I notice lollapalooza-cms is replaced with new-theme-cms but I would prefer if the occurences of lollapalooza-cms is not touched anywhere.

I haven't found a way to exclude strings to be replaced by default.

So I now use a quick and dirty workaround, in the post process function I have an array of "protected words". They get switched back after they were replaced.

The post process function runs after the replacement. It would be nice to have a way to fix this before any replacement.