Skip to content
This repository has been archived by the owner on Mar 31, 2023. It is now read-only.

Improving theme cloning process in order to minimize manual updates #268

Merged
merged 5 commits into from
Jul 6, 2018
+8 −10
Merged

Improving theme cloning process in order to minimize manual updates #268

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
98bc2d1
Improving theme cloning process in order to minimize manual updates
shaal Jul 3, 2018
25998a8
fixing comment line
shaal Jul 3, 2018
1c572e3
remove a non-existing directory `components/css`
shaal Jul 5, 2018
4b7adf4
Support the `-slim` option
shaal Jul 5, 2018
06d72cb
Fixing the function _emulsify_alter_files (code by @ccjjmartin)
shaal Jul 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions components/_patterns/01-atoms/04-images/icons/_icon.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@
*/
#}
{% set icon_base_class = icon_base_class|default('icon') %}
{# For Drupal: Update this path to match your theme directory structure #}
{# For Pattern Lab: The icon_url is defined in _data/paths.json. No action is required #}
{% set icon_url = icon_url|default('/themes/custom/YOURTHEME/dist/img/sprite/') %}
{# Update `emulsify` here to your theme's machine name #}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shaal I am curious as to the broken behavior you experienced here. Did "emulsify" on this line get replaced? On the line right after this did it not get replaced? I am definitely pro simplifying install processes so I would love to hear more about the bug you encountered.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the goal here is to set this as emulsify by default so that it can be auto-replaced. In the "before" code, it says to manually enter your theme's machine name. Setting it automatically on cloned theme creation is preferred.

Copy link
Collaborator

@ccjjmartin ccjjmartin Jul 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I was focused on the comment containing the string emulsify but that actually isn't the main change here. I should have been looking at the YOURTHEME versus emulsify. I am good with this change because it makes the out of the box experience better in most scenarios. I will point out that this is not going to work for people who don't put their theme in the root of /themes/ versus /themes/custom/ But that is likely a bigger issue that what we are trying to address here. We should consider replacing this (in another PR not this one) with just a string called PATH and then it drops in the theme path.

Alterations are defined in the _emulsify_get_alterations function which is currently only doing these:

    'Emulsify' => $human_readable_name,
    'emulsify' => $machine_name,
    'Theme that uses Pattern Lab v2' => $description,
    'hidden: true' => '',

It would be pretty easy to add a new line:

    'PATH' => $path,

And add another parameter for to that same function to pass the path in. When I wrote this I wasn't thinking we would need the PATH within the twig files.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created this ticket for follow up on this: #271

{# For Drupal: The icon_url is defined in the following line. No action is required #}
{% set icon_url = icon_url|default('/themes/custom/emulsify/dist/img/sprite/') %}
{{ attach_library('emulsify/sprite') }}

<svg {{ bem(icon_base_class, (icon_modifiers), icon_blockname, (icon_js_class)) }}>
Expand Down
3 changes: 2 additions & 1 deletion emulsify.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,8 @@ function _emulsify_get_files_to_alter() {
}
else {
return array_merge($default_array, array(
'templates/navigation/menu--main.html.twig',
Copy link
Collaborator

@ccjjmartin ccjjmartin Jul 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shaal Can you remove these two lines completely and apply this patch to your branch:

diff --git a/emulsify.php b/emulsify.php
index a3bec1c..a889a5e 100644
--- a/emulsify.php
+++ b/emulsify.php
@@ -550,7 +550,7 @@ function _emulsify_get_files_to_rename() {
  * @return boolean
  *   A boolean representing the success or failure of the function.
  */
-function _emulsify_alter_files($theme_path, array $files_to_alter = array(), array $alterations = array(), $absolute = FALSE) {
+function _emulsify_alter_files($theme_path, array $files_to_alter = array(), array $alterations = array(), $absolute = FALSE, int $depth = 0) {
   if (empty($files_to_alter) || empty($alterations)) {
     return TRUE;
   }
@@ -568,11 +568,11 @@ function _emulsify_alter_files($theme_path, array $files_to_alter = array(), arr
       $files = scandir($file_path);
       $files = array_splice($files, 2);
       foreach ($files as $file) {
-        $processed_files[] = $file_path . $file;
-      }
-      $alter_status = _emulsify_alter_files($theme_path, $processed_files, $alterations, TRUE);
-      if ($alter_status === FALSE) {
-        return FALSE;
+        $processed_file = [$file_path . DIRECTORY_SEPARATOR . $file];
+        $alter_status = _emulsify_alter_files($theme_path, $processed_file, $alterations, TRUE, $depth + 1);
+        if ($alter_status === FALSE) {
+          return FALSE;
+        }
       }
     }
     elseif ($file_type === 'file') {

This was originally designed that you can just list the components directory and it recursively searches for files and replaces the text within them. It must have broken somewhere along the way. This fixes it though according to my tests, reproducing steps below:

  1. Run php emulsify.php "Alter Test"
  2. Verify that the _icon.twig file contains the name alter_test as the machine_name on the desired lines changed above.

NOTE: We may want to check to see if anything else unexpectedly broke since the replacements are now actually working throughout the components and templates directories.

'components/_patterns/01-atoms/04-images/icons/_icon.twig',
'components',
'templates',
'README.md',
Expand Down Expand Up @@ -492,7 +494,6 @@ function _emulsify_get_files_to_copy() {
'components/_macros',
'components/_meta',
'components/_twig-components',
'components/css',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, this needed to go.

'components/images',
'components/_patterns/style.scss',
'components/_patterns/00-base/global/01-colors',
Expand Down