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 error when template is not found #389

Merged
merged 1 commit into from
Mar 10, 2015
Merged

Fix error when template is not found #389

merged 1 commit into from
Mar 10, 2015

Conversation

SamyPesse
Copy link
Contributor

This is a fix for an error I introduced in my precedent PR (Sorry ...).

I'm currently running some tests for GitBook on Windows and Nunjucks is not working there (tests are all failing).
You should maybe consider using Appveyor (as simple as travis and free for open source projects) to test Nunjucks on Windows.
I'll take a deep look at why Nunjucks is not working (it's reason why GitBook is not working).

@jlongster
Copy link
Contributor

Interesting, I know there are other windows users that use nunjucks. Maybe something recent broke it. I'll take a look at Appveyor, thanks.

jlongster added a commit that referenced this pull request Mar 10, 2015
Fix error when template is not found
@jlongster jlongster merged commit 8ef8c47 into mozilla:master Mar 10, 2015
@SamyPesse SamyPesse deleted the patch-1 branch March 10, 2015 17:39
@SamyPesse
Copy link
Contributor Author

@jlongster it looks like the problem come from my PRs for relative paths, so it's probably my fault.

I'm looking at it, but it will be great if you can setup a project on Appveyor associated with nunjucks so that in the future it avoids breaking Windows compatibility.

@SamyPesse
Copy link
Contributor Author

I think the problem is coming from here: https://github.com/mozilla/nunjucks/blob/master/src/compiler.js#L127

Basically windows path with \ like C:\test\hello are not escaping so the paths become invalid.

I'm working on a fix and will propose a PR in the coming hours.

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