laktek / punch

A fun and easy way to build modern websites
http://laktek.github.com/punch
MIT License
1.17k stars 107 forks source link

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

Open s-oram opened 9 years ago

s-oram commented 9 years ago

I was attempting to follow the Punch setup tutorial but partials weren't rendering. I found the fix for the problem here: https://github.com/laktek/punch/issues/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.

laktek commented 9 years ago

Check this PR from the past, which supposedly should fix path issues in Windows - https://github.com/laktek/punch/pull/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 commented 9 years ago

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 commented 9 years ago

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