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 the verbose partial #559 #675

Merged
merged 4 commits into from
Sep 11, 2017
Merged

Fix the verbose partial #559 #675

merged 4 commits into from
Sep 11, 2017

Conversation

gergan
Copy link

@gergan gergan commented Aug 11, 2017

Addresses #559

Summary of changes:
We have the same problem as the reporter of #559 but using the handelbars implementation. I think the root of all problems is that almost everywhere the partial name currently is compared with subdir concatenated with '/' and filename.

subdir contains the OS native representation of the relative path, so it will work on linux but not on windows as it will contain backslashes and the rest will be generated with slashes.
I have found the property verbosePartial, which strangely enough is not used anywhere, but has the same value as the things against which pattern_assembler and pattern_registry are testing the partials.

So my solution was to 'normalize' the verbose partial with only forward slashes and use it consequently throughout the sources.

I will create a pull request for the Handelbars implementation (patternengine-node-handlebars), which than makes it work with the default include syntax.

The assumption for the two patches is that the templates are using only forward slashes for the inclusion, which is the only reasonable assumption in my opinion.

@bmuenzenmeyer
Copy link
Member

bmuenzenmeyer commented Aug 16, 2017

hello @gergan

thanks for diving into this issue and looking into a fix - I will admit the verbose include syntax is not something I use often.

I am going to point this PR at dev per the contributions guidelines

@bmuenzenmeyer bmuenzenmeyer changed the base branch from master to dev August 16, 2017 12:22
@gergan
Copy link
Author

gergan commented Aug 17, 2017

hello @bmuenzenmeyer

my bad have overlooked the contribution guidelines

@bmuenzenmeyer bmuenzenmeyer merged commit a52a346 into pattern-lab:dev Sep 11, 2017
@bmuenzenmeyer
Copy link
Member

💯This looks great - demonstrates a willingness to dive into the code and improve it.

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.

4 participants