Skip to content
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

Fix hidden pattern references #509

Merged
merged 12 commits into from
Oct 11, 2016
Merged

Conversation

geoffp
Copy link
Contributor

@geoffp geoffp commented Oct 6, 2016

Addresses #469

Summary of changes:
This is frustratingly simple for how much time it took. Two main changes to the non-test code:

  1. Make the pattern prefix matching regex aware of leading underscores in pattern file names.
  2. Factor it out and write it only once.
  3. Also, pre-calculate the verbose partial name in the Pattern object. I thought I could use it, but I couldn't (handlebars doesn't tolerate slashes in partial syntax)

And lots of unit tests to prove it. I created a place for shared code in the tests.

@bmuenzenmeyer
Copy link
Member

fixed a merge conflict - going to test this manually too next free time i get - kids up now - dad time

@geoffp
Copy link
Contributor Author

geoffp commented Oct 10, 2016

Groovy. Do that dad time!

@geoffp
Copy link
Contributor Author

geoffp commented Oct 10, 2016

And, thanks for doing the merge. I picked exactly the wrong time to do stuff to unit tests.

@bmuenzenmeyer
Copy link
Member

@geoffp does using lineage to go to the hidden pattern work for you?

image

It does not for me using this quick test above

@bmuenzenmeyer
Copy link
Member

I'm okay making this a separate issue too if need be. Just curious what is happening on your end.

@bmuenzenmeyer bmuenzenmeyer merged commit 777dd1c into dev Oct 11, 2016
@bmuenzenmeyer bmuenzenmeyer deleted the fix-hidden-pattern-references branch October 11, 2016 13:01
@geoffp
Copy link
Contributor Author

geoffp commented Oct 11, 2016

@bmuenzenmeyer Just tested -- no, it's not working for me. Separate ticket?

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.

2 participants