Skip to content
This repository has been archived by the owner on Nov 27, 2018. It is now read-only.

bug fix: partials do not render on windows 7. #119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

bug fix: partials do not render on windows 7. #119

wants to merge 1 commit into from

Conversation

s-oram
Copy link

@s-oram s-oram commented Jan 22, 2015

I was attempting to follow the Punch setup tutorial but partials weren't rendering. I found the fix for the
problem here:
#102

My system is Windows 7 64bit.

Partials are now rendering after applying the fix. I have no idea what effects the changes will have when run on OSX and other operating systems. Sorry.

@@ -19,7 +19,7 @@ module.exports = {
return false;
}

var path_portions = template_path.split(Path.sep || "/");
var path_portions = template_path.split("/");
Copy link
Owner

Choose a reason for hiding this comment

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

hmm..weird. What's returned in windows for Path.sep?

Copy link
Author

Choose a reason for hiding this comment

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

Path.sep returns \

Copy link
Author

Choose a reason for hiding this comment

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

so in template_handler.js isSection: function(template_path){}
the template_path variable contains paths formatted like /images\banner.

@laktek
Copy link
Owner

laktek commented Jan 25, 2015

Check this PR from the past, which supposedly should fix path issues in Windows - #82.

Can you figure out why removing Path.sep fixes partial rendering? You can add couple of tests (similar to above PR) to verify that your fix would work on Windows and other OSs. relevant test file - https://github.com/laktek/punch/blob/master/spec/template_handler.spec.js

@s-oram
Copy link
Author

s-oram commented Feb 4, 2015

Spent a bit more time looking at this issue today.

Can you figure out why removing Path.sep fixes partial rendering?

On windows Path.sep returns \. I assume Path.sep returns / on OSX and Linux.
From what I can see, it looks like Punch is assuming the path separator is always \ in some places and using the system defined path separator in other places.

For example page_renderer.js line 208

request_path = request_path || "/";

template_handler.js line 22

 var path_portions = template_path.split(Path.sep || "/");

Removing the Path.sep usage as in the original pull request hides the problem but I'm assuming it's not a proper fix and the issue will show again with different input. I don't understand the whole system well enough to know.

To fix the problem I guess Punch needs to consistently use the system defined path separator or reformat paths to use / throughout the project. What do you think Laktek?

Anyway, I'm happy to keep working on this. Just not sure which way to go.

@jezza323
Copy link

The obvious solution is to use Path.sep everywhere, id go down that route if I were you

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

Successfully merging this pull request may close these issues.

None yet

3 participants