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

Added the ability to pass a templatePath variable into ng-constants to a... #19

Closed
wants to merge 1 commit into from

Conversation

dropshape
Copy link
Contributor

...llow custom templates

@mlegenhausen
Copy link
Owner

Thanks for your push request. But isn't the wrap option enough? Can you define a use case where this is helpful?

@dropshape
Copy link
Contributor Author

Hi mlegenhausen,

The reason i added the template functionality wad due to the fact that your built in template broke my jshint rules. I tried using the wrap functionality but decided against it as it meant adding a lot of code to my grunt file which I prefer not to do. For example I had to add this

wrap: '/*jshint -W109*/\ndefine(function(require) { \n    \'use strict\';' +
                '\n    var angular = require(\'angular\');' +
                '\n\n    return <%= __ngModule %> \n\n});',

But that also generated an output file that was a bit messy. The solution I found was to add my own custom template that would output the constants just how I like them. This change is backwards compatible so it should not break the wrap functionality.

Let me know what you think.

@mlegenhausen
Copy link
Owner

My generell recommendation is not to include the generated code in your jshint task. For me generated code is always valid cause machines are much better in code generation than humans ;) (I need to add this to the README)

But I like your approach cause wrap is only a special case of the templatePath parameter. I will add this!

For now if you want to use the wrap option you can use an external file and load your template via wrap: grunt.file.read('my-custom-tpl.html).

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

2 participants