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 so that steal("./resources/underscore.js") and steal("//app/resources #27

Closed
wants to merge 1 commit into from

Conversation

bman654
Copy link

@bman654 bman654 commented Jun 20, 2011

I was noticing that I was sometimes getting files stolen more than once. I tracked it down to one location where I had:

steal("//bpa/resources/underscore.js")

and in some other dependency that resides in the "resources" folder:

steal("./underscore.js");

Was due to the absolute path for the second case resolving to

//bpa/resources/./underscore.js

…rces/underscore.js") do not load the file twice (as //app/./resources/underscore.js and //app/resources/underscore.js)
@jupiterjs
Copy link

./ isn't supported. Though it might be in the next release.

I have to think about adding a few bytes for something that isn't supported.

@jupiterjs
Copy link

But thanks for submitting a patch!

@bman654
Copy link
Author

bman654 commented Jun 20, 2011

I've been using "./" based upon your steal preview post which suggested that very usage in the new steal() when you need to access a relative path.

@jupiterjs
Copy link

Ah ... that's not in the current version of steal. And we are only 75% sure it's going to be in the next version.

If we use ./ in the next version, I'll pull this in.

@bman654
Copy link
Author

bman654 commented Jun 20, 2011

I was just being proactive in my code since I'm pretty sure I'll switch to the new version when it is ready and wanted to minimize the conversion.

Thanks.

@moschel
Copy link

moschel commented Jul 18, 2011

The ./ syntax is supported in the new version of steal that is now on master, so this patch is no longer necessary. The test case you provided should be working now. If you still see a problem, please let us know.

@moschel moschel closed this Jul 18, 2011
moschel added a commit that referenced this pull request Dec 21, 2011
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.

None yet

3 participants