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

Basically, the Path module's join assumes you're only working with paths... #57

Merged
merged 2 commits into from
Dec 1, 2013

Conversation

OlenDavis
Copy link
Contributor

..., and as url's are not merely paths when they specify a scheme (https://, http://, etc), this normalization breaks some builds. Allows developers to specify strings that incorporate two forward slashes intentionally as part of the scheme of their template urls (for instance, for hosting those files from a CDN when not pre-cached - and note that the Path module's normalization doesn't take this case into account) by checking whether the involved parts of the url include "://" and if so, simply concatenating them, and using Path's join (which normalizes the resulting url) otherwise.

…ths, and as url's are not merely paths when they specify a scheme (https://, http://, etc), this normalization breaks some builds. Allows developers to specify strings that incorporate two forward slashes intentionally as part of the scheme of their template urls (for instance, for hosting those files from a CDN when not pre-cached - and note that the Path module's normalization doesn't take this case into account) by checking whether the involved parts of the url include "://" and if so, simply concatenating them, and using Path's join (which normalizes the resulting url) otherwise.
@ericclemmons
Copy link
Owner

Someone else brought this up too, and just recommended I use url. Thoughts?

@OlenDavis
Copy link
Contributor Author

How about prefix + url.parse( url )? That way, you enforce the implication that the url is a url, while treating the prefix very simply as a prefix? And any developer that was depending on the nearly undetermined previous behavior will simply be coerced into clarifying their own internal logic.

@OlenDavis
Copy link
Contributor Author

Sorry; that'd be prefix + url.format( url.parse( urlString ) ) where urlString is the result of the url option string/function.

… the url and prefix options by using Url rather than Path.
@OlenDavis
Copy link
Contributor Author

How's it look Eric?

@ericclemmons
Copy link
Owner

@OlenDavis Trying to resolve the tests now.

Can you do me a favor & post how you're using this exactly? I'm assuming you have a prefix of http://cdn.mysite.com/assets which is then joined with app/templates/home.html or similar, right?

@OlenDavis
Copy link
Contributor Author

Like so:

    ngtemplates:
        ngApp:
            cwd    : '<%= pkg.buildDir %>/<%= pkg.builtDir %>/template'
            src    : '**/*.html'
            dest   : builtTemplatesJs

            options:
                module : pkg.ngApp
                url    : ( url ) -> "#{ staticFilePath }template/#{ url }#{ preprocessContext.APPLICATION_CONFIG_staticFileSuffix }"
                htmlmin:
                    collapseBooleanAttributes: no
                    collapseWhitespace       : yes
                    removeAttributeQuotes    : yes
                    removeComments           : yes
                    removeEmptyAttributes    : yes
                    removeRedundantAttributes: yes

@ericclemmons ericclemmons merged commit da40411 into ericclemmons:master Dec 1, 2013
@ericclemmons
Copy link
Owner

Let me know how v0.4.9 works out for ya!

@OlenDavis
Copy link
Contributor Author

Will do! And thank you kindly!

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