Skip to content

Fix for Broken Lineage Functionality in PL When Using Twig templates w/ Path Namespaces (ie. Drupal 8-friendly Paths) #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 20, 2017

Conversation

sghoweri
Copy link
Collaborator

Addresses this open issue and corresponds with the 2nd half of work mentioned here.

Note: the regex updates added to the Pattern Lab Twig Engine fork (PR from above) are required in order for this update to work as expected.

Lemme know if there's anything I can help clarify on this!

Author's note(s):

    1. Given that I'm really not a PHP / Drupal dev, I'm commenting the crap out of what I came up with to hopefully make a bit more sense. I'm sure there's a better way of handling with but at least this rough around the edges approach seems to work...
    1. Can we please move over to using spaces instead of tabs at some point? grumble grumble
    1. Kinda related to number 2, could we also fix these forked repos to use the standard git flow "develop" branch convention? Any particular reason why we should stick to using "dev"?

… comments to explain my PHP chicken scratch.

Addresses existing open issue #3 and corresponds with the 2nd half of work mentioned in drupal-pattern-lab/patternengine-php-twig#1
Copy link

@aleksip aleksip left a comment

Choose a reason for hiding this comment

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

Tested this and it seems to work fine. Great work!

I have just submitted PR #9 and if that is accepted then a $patternName = str_replace('.', '-', $patternName); should be added here too.

We could also consider adding a common getPatternName utility function, as right now it is a protected function in the Rule class.

@sghoweri
Copy link
Collaborator Author

Excellent! Thanks @aleksip - really appreciate it!

In terms of order of operations, what would you recommend? Merge this in first, then apply your other PR on top? Would that work?

@aleksip
Copy link

aleksip commented Jun 20, 2017

Actually, might be best if your PR was merged first, as it is now, and I could then add the str_replace to LineageHelper in my PR.

@EvanLovely
Copy link

I've already merged (and release) PR #9 from @aleksip

Great annotation approach @sghoweri !

@EvanLovely
Copy link

EvanLovely commented Jun 20, 2017

@evanmwillhite wanna toss your review in before we merge and release?

@evanmwillhite
Copy link

Nah, don't wait on me. Go for it!

@EvanLovely EvanLovely merged commit 407f7bb into dev Jun 20, 2017
@EvanLovely EvanLovely deleted the feature/fix-pattern-lab-lineages branch June 20, 2017 17:37
@EvanLovely
Copy link

Merged into master as well and released as v2.7.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants