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 path normalize on Windows, Alternative file extension, can.view.attr #32

Merged
merged 3 commits into from
Feb 18, 2015

Conversation

srasp
Copy link

@srasp srasp commented Feb 17, 2015

Added extension mapping, if you use another view extension as ".mustache", eg. ".mst". Just added "extension" parameter to the options.
If can.view.attr is used, it wasn´t take into account. Added an options parameter to maintain used view attrs.

Signed-off-by: srasp sebastian.rasp@promerit.com

Added extension mapping, if you use another view extension as ".mustache", eg. ".mst". Just added "extension" parameter to the options.
If can.view.attr is used, it wasn´t take into account. Added an options parameter to maintain used view attrs.

Signed-off-by: srasp <sebastian.rasp@promerit.com>
@daffl
Copy link
Contributor

daffl commented Feb 17, 2015

Awesome thank you! Two things: The build failed with a JSHint error (the variable i is defined twice in https://github.com/srasp/can-compile/blob/master/lib/compile.js#L51 and https://github.com/srasp/can-compile/blob/master/lib/compile.js#L57).

Could you also add the documentation for the extension parameter (probably somewhere around https://github.com/daffl/can-compile#programmatically) and add it as an option in the command line at https://github.com/daffl/can-compile/blob/master/bin/can-compile#L12).

Documentation updated
@srasp
Copy link
Author

srasp commented Feb 18, 2015

As you recommended I changed the variable in compile.js and added the parameters to the documentation as well as in the command line at can-compile.

@marshallswain
Copy link
Member

@srasp It looks like the automated tests have failed again. You can click the details link to the right of the message that says "The Travis CI build failed" to see what's going on. You can actually run these tests yourself before pushing your code, as well. Have you tried running the tests on your own machine? If you're not familiar with the process, this is how to get it working:

  • Make sure you have Grunt installed globally.
  • Open the root folder of this project in a terminal window and do npm install
  • then run grunt test. Actually, you can just run grunt, since the default grunt task is set to test, in Gruntfile.js. It looks like it will be running JSHint on your code, too, based on the next line of code.

If the tests run without any errors and you're cleaning up anything JSHint says to fix, the Travis CI builder won't balk at your awesome commits.

@daffl
Copy link
Contributor

daffl commented Feb 18, 2015

That's a weird error though. I'll check what's going on.

Updated test, because normalizer method did not work on Windows,
Added test for alternative file extension (.mst)
@srasp
Copy link
Author

srasp commented Feb 18, 2015

Sorry guys, now it should work. I had to check if the new option extensions is defined.
I´ve updated the test, because on Windows the normilzer method didn´t work. Additonally, I´ve added a test case, to test the alternative file extension.

@marshallswain
Copy link
Member

No need to apologize. Thank you for your work.

@daffl
Copy link
Contributor

daffl commented Feb 18, 2015

Awesome, thank you!

daffl added a commit that referenced this pull request Feb 18, 2015
Fixed path normalize on Windows, Alternative file extension, can.view.attr
@daffl daffl merged commit 6d751e8 into canjs:master Feb 18, 2015
@daffl
Copy link
Contributor

daffl commented Feb 18, 2015

Released as version 0.8.0. Thanks again!

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.

3 participants