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 📌 Promote symfony/finder from dev-dependency to full dependency Fixed and 📌 Promote symfony/filesystem from dev-dependency to full dependency Fixed

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));

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.

The core committers seem willing to introduce the Symfony dependencies. Is there a reason (either way) to introduce/not introduce them?

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

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

Thanks for working on this btw!

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.

@mglaman - Did you save your progress on this? if so can you post a patch? I have someone that is willing to pick up and run with it.

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 📌 [PP-1] Allow starterkit theme generator tool to clone Olivero Postponed , 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.

Thanks for working on this! I tested this out and ran into a number of issues:

1. I'm unable to delete entire directories.

I declared starterkit: true in Olivero and tried to delete the tests directory by adding

delete:
  - 'tests'

delete:
  - 'tests/'

delete:
  - '/tests'

Or other permutations, and wasn't able to get it to succeed.

2. If nothing found under no_edit or no_rename, no error is thrown

I expect an error to be returned if the process doesn't find any files that it can operate against

3. Wildcards / globs don't work

I tried passing in

no_edit:
  - '**/js/*.js'

and it still edits the files.

3. No longer any need for starterkit: true in the YML file

The presence of a themeName.starterkit.yml is indication enough

4. Nice to have: debug mode

It'd be super nice if this process could have a debug mode to help out theme developers.

Can confirm #28.1.

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

second best camp

NO CREDIT FOR THIS MAN!!!!

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.

In the process of doing an in-depth review.

Setting to NW now, because I found a blocking bug.

I copied and pasted the starterkit.yml from starterkit_theme into Olivero just to give it a run

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

The theme generated fine. When going to admin/appearance, there was an error:

Drupal\Core\Extension\InfoParserException: Missing required key ("base theme") in themes/my_new_theme/my_new_theme.info.yml, see https://www.drupal.org/node/3066038 in Drupal\Core\Extension\ThemeExtensionList->createExtensionInfo() (line 269 of core/lib/Drupal/Core/Extension/ThemeExtensionList.php).

Sure enough, Olivero has base theme: false (on line 15) in its info.yml file. The new theme did not have this, which causes the error. If I go into Olivero and set the base theme to something else, it copies over. So I'm assuming it's not copying because it's set to false

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

Another bug:

The starterkit_theme.starterkit.yml contains a delete key. It looks like this has been replaced by ignore

Overall this is working great! Not finding any more ways to break it (so far!).

• Ignoring works
• Globs and wildcards work
• no_edit works
• When I try to no_rename a file that doesn't exist I get a [ERROR] Paths were defined `no_rename` but no files found. (same behavior for no_edit)

A couple questions:

• We get appropriate error messages when applying against a non-existent file with no_rename and no_edit, however the ignore key just skips over it and doesn't have the same behavior. Is it possible to make the behavior consistent? This shouldn't be a blocker for this issue.
• We also talked about a verbose mode. I don't see anything about this

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.

This is amazing!

• I verified that base theme: false now copies over as expected.
• Verfied starterkit_theme.starterkit.yml is now correct
• Verified that if no items within the ignore key, an error is thrown.

Question:
Currently if at least one file exists under ignore (or no_edit or no_rename) the script will not error out. However this is confusing, because there might be a mix of existing and non-existent files. Is it possible to error if any of the listed files are not found? This isn't a blocker, but setting to NW to see if this can be done.

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

Makes sense. That's mostly just a nice-to-have. Setting to RTCB

Updating IS

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 →

Tests are passing. Back to RTBC.

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.

Pulled down latest changes and went through some general testing and everything still works. Feedback appears to have been addressed 🙌

Adjusting credits (please feel free to tweak).

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 →

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

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.