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

Fixed Jinja on Windows (it choked on "\"-paths) #41

Merged
merged 1 commit into from
Aug 16, 2012

Conversation

edbrannin
Copy link
Contributor

Added strange_case.support.jinja.fix_path, which is used in
StrangeCaseEnvironment.init() and JinjaNode.render().

As it says in the docstring:

Jinja chokes on backslash-separated paths, and slash-separatd
paths work well enough in Windows anyway. See also Jinja2-99,
Jinja2-98.

Added strange_case.support.jinja.fix_path, which is used in
StrangeCaseEnvironment.__init__() and JinjaNode.render().

As it says in the docstring:

Jinja chokes on backslash-separated paths, and slash-separatd
paths work well enough in Windows anyway.  See also [Jinja2-99][],
[Jinja2-98][].

[Jinja2-98]: pallets/jinja#98
[Jinja2-99]: pallets/jinja#99
@colinta
Copy link
Owner

colinta commented Aug 16, 2012

God help the person who uses \ in a file name :-P (and actually, this would be fine on *nix, because os.path.sep == '/', so the \ would be ignored)

colinta added a commit that referenced this pull request Aug 16, 2012
Fixed Jinja on Windows (it choked on "\"-paths)
@colinta colinta merged commit 4931b51 into colinta:master Aug 16, 2012
@colinta
Copy link
Owner

colinta commented Aug 16, 2012

Thanks for all your help on the Windows compatibility!

@edbrannin
Copy link
Contributor Author

Thanks for making the SSG closest to what I'm looking for!

(I'd like to start using this at work for some internal documentation, and personally for template-extraction on some legacy websites. Most other SSGs are a bit over-engineered for that other use-case, and I really like Jinja compared to anything available for Ruby.)

The only Windows issue I'm aware of now is misaka not building, I can use markdown2 until that's fixed. Oh, and I don't think the code for #36 (4.5.17) ever made it into Master?

@colinta
Copy link
Owner

colinta commented Aug 16, 2012

We might have spoken too soon, i'm getting failures after merging this request...

4.5.17 is up, on pypi at least: http://pypi.python.org/pypi/StrangeCase (says "4.5.17")

@colinta
Copy link
Owner

colinta commented Aug 16, 2012

I can get the failures fixed by getting rid of relpath in the fix_path method:

diff --git a/strange_case/support/jinja.py b/strange_case/support/jinja.py
index 8c9434c..3f6e3b4 100644
--- a/strange_case/support/jinja.py
+++ b/strange_case/support/jinja.py
@@ -145,5 +146,4 @@ def fix_path(path):
     Note: This function will also chomp any in-filename backslashes.
     Hopefully you don't have any of those in the relative path to your template.
     """
-    return os.path.relpath(path).replace(os.path.sep, '/')
-
+    return path.replace(os.path.sep, '/')

@ghost ghost assigned colinta Aug 16, 2012
@colinta
Copy link
Owner

colinta commented Aug 16, 2012

I"ve pushed v4.5.18, which includes this fix, and passes the tests. Can you verify all-is-well on your end?

@edbrannin
Copy link
Contributor Author

Nope. I was using os.path.relpath because of this:

C:\tmp\scase>scase
Traceback (most recent call last):
  File "C:\Python27\Scripts\scase-script.py", line 9, in <module>
    load_entry_point('StrangeCase==v4.5.16', 'console_scripts', 'scase')()
  File "c:\dev\github\strangecase\strange_case\__main__.py", line 185, in run
    strange_case(CONFIG)
  File "c:\dev\github\strangecase\strange_case\__init__.py", line 154, in strange_case
    root_node.generate()
  File "c:\dev\github\strangecase\strange_case\nodes\root_folder.py", line 37, in generate
    child.generate(self)
  File "c:\dev\github\strangecase\strange_case\nodes\file.py", line 20, in generate
    self.generate_file(site, self.source_path, target_path)
  File "c:\dev\github\strangecase\strange_case\nodes\jinja.py", line 10, in generate_file
    content = self.render(site)
  File "c:\dev\github\strangecase\strange_case\nodes\jinja.py", line 21, in render
    template = Registry.get('jinja_environment').get_template(fix_path(self.source_path))
  File "C:\Python27\lib\site-packages\jinja2\environment.py", line 719, in get_template
    return self._load_template(name, self.make_globals(globals))
  File "C:\Python27\lib\site-packages\jinja2\environment.py", line 693, in _load_template
    template = self.loader.load(self, name, globals)
  File "c:\dev\github\strangecase\strange_case\support\jinja.py", line 107, in load
    source, filename, uptodate = self.get_source(environment, name)
  File "c:\dev\github\strangecase\strange_case\support\jinja.py", line 77, in get_source
    contents, filename, uptodate = super(YamlFrontMatterLoader, self).get_source(environment, template)
  File "C:\Python27\lib\site-packages\jinja2\loaders.py", line 165, in get_source
    f = open_if_exists(filename)
  File "C:\Python27\lib\site-packages\jinja2\utils.py", line 224, in open_if_exists
    return open(filename, mode)
IOError: [Errno 22] invalid mode ('rb') or filename: u'C:/tmp/scase\\C:tmp\\scase\\site\\test2.j2'

@colinta
Copy link
Owner

colinta commented Aug 16, 2012

What about using os.path.abspath instead? any help?

On Aug 16, 2012, at 12:09 PM, Ed Brannin wrote:

Nope. I was using os.path.relpath because of this:

C:\tmp\scase>scase
Traceback (most recent call last):
File "C:\Python27\Scripts\scase-script.py", line 9, in
load_entry_point('StrangeCase==v4.5.16', 'console_scripts', 'scase')()
File "c:\dev\github\strangecase\strange_case__main__.py", line 185, in run
strange_case(CONFIG)
File "c:\dev\github\strangecase\strange_case__init__.py", line 154, in strange_case
root_node.generate()
File "c:\dev\github\strangecase\strange_case\nodes\root_folder.py", line 37, in generate
child.generate(self)
File "c:\dev\github\strangecase\strange_case\nodes\file.py", line 20, in generate
self.generate_file(site, self.source_path, target_path)
File "c:\dev\github\strangecase\strange_case\nodes\jinja.py", line 10, in generate_file
content = self.render(site)
File "c:\dev\github\strangecase\strange_case\nodes\jinja.py", line 21, in render
template = Registry.get('jinja_environment').get_template(fix_path(self.source_path))
File "C:\Python27\lib\site-packages\jinja2\environment.py", line 719, in get_template
return self._load_template(name, self.make_globals(globals))
File "C:\Python27\lib\site-packages\jinja2\environment.py", line 693, in _load_template
template = self.loader.load(self, name, globals)
File "c:\dev\github\strangecase\strange_case\support\jinja.py", line 107, in load
source, filename, uptodate = self.get_source(environment, name)
File "c:\dev\github\strangecase\strange_case\support\jinja.py", line 77, in get_source
contents, filename, uptodate = super(YamlFrontMatterLoader, self).get_source(environment, template)
File "C:\Python27\lib\site-packages\jinja2\loaders.py", line 165, in get_source
f = open_if_exists(filename)
File "C:\Python27\lib\site-packages\jinja2\utils.py", line 224, in open_if_exists
return open(filename, mode)
IOError: [Errno 22] invalid mode ('rb') or filename: u'C:/tmp/scase\C:tmp\scase\site\test2.j2'

Reply to this email directly or view it on GitHub.

@edbrannin
Copy link
Contributor Author

Here are the variables at the top of JinjaNode.render():

C:\tmp\scase>scase
self.source_path is 'C:\tmp\scase\site\test2.j2', fixed to 'C:/tmp/scase/site/test2.j2'
Fixed Relative path is 'site/test2.j2'

It looks like the Jinja environment knows its PWD, and is "helpfully" prepending that to whatever path we give it. The error I pasted above shows an absolute path tacked onto the end of another absolute path.

What failure had you experienced? Maybe this will work if we only use os.path.relpath() in JinjaNode.render()?

@edbrannin
Copy link
Contributor Author

Note for posterity: this is continuing over at #44.

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