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

Fixing #2109 (bug in get_asset_path). #2110

Merged
merged 2 commits into from Sep 23, 2015

Conversation

Projects
None yet
3 participants
@felixfontein
Copy link
Contributor

felixfontein commented Sep 22, 2015

Fixes #2109. Also fixes a bug in the docstring, and adds one more example output to make clear what it really does.

None
>>> print(get_asset_path('nikola/nikola.py', ['bootstrap3', 'base'], {'nikola': 'nikola'}))
/.../nikola/nikola.py

This comment has been minimized.

@ralsina

ralsina Sep 22, 2015

Member

/.../ ???

This comment has been minimized.

@felixfontein

felixfontein Sep 22, 2015

Author Contributor

/.../ stands for "some random path on my hdd which I don't want to paste here", like in the already existing examples.

This comment has been minimized.

@Kwpolska

Kwpolska Sep 22, 2015

Member

that’s a doctest ellipsis, not a real path

This comment has been minimized.

@ralsina

ralsina Sep 22, 2015

Member

The others are actually doctests.

This comment has been minimized.

@felixfontein

felixfontein Sep 22, 2015

Author Contributor

Isn't the new one a doctest, too? Or is some special magic needed to make it a doctest as well?

This comment has been minimized.

@Kwpolska

Kwpolska Sep 22, 2015

Member

This is a doctest, as it begins with >>> — py.test should pick it up (whether or not doctests are good is another thing)

This comment has been minimized.

@felixfontein

felixfontein Sep 22, 2015

Author Contributor

And it passes; if I modify the path, doit doctest fails.

This comment has been minimized.

@ralsina

ralsina Sep 22, 2015

Member

Awsome then :-)

if os.path.isfile(candidate):
return candidate
relpath = os.path.relpath(path, rel_dst)
if not relpath.startswith('..' + os.path.sep):

This comment has been minimized.

@ralsina

ralsina Sep 22, 2015

Member

If you are going to check this, you may also want to add a normpath call there.

This comment has been minimized.

@felixfontein

felixfontein Sep 22, 2015

Author Contributor

Good point.

@ralsina

This comment has been minimized.

Copy link
Member

ralsina commented Sep 23, 2015

Thanks for fixing this Felix!

ralsina added a commit that referenced this pull request Sep 23, 2015

Merge pull request #2110 from getnikola/fix-get-asset-path
Fixing #2109 (bug in get_asset_path).

@ralsina ralsina merged commit 9d5093c into master Sep 23, 2015

3 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ralsina ralsina deleted the fix-get-asset-path branch Sep 23, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.