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

Adds support for coffee with 'multi' parser #67

Closed

Conversation

lteacher
Copy link
Contributor

@lteacher lteacher commented Aug 3, 2017

Hello,

Thanks for the package as it is very helpful. Someone had previously hacked up the Protractor in a fork to provide this functionality which is obviously not ideal and since we need to upgrade it would be splendid to use this.

However, we have quite a bit of coffeescript still and so it would be great if coffee files could be found which is not the case right now in the multi parser.

The change seems fair to me so just opened this PR, if not accepted we can just use custom parser. Thanks.

PR Changes

  • Changes multi.js spec regex to include .coffee files

@NickTomlin
Copy link
Owner

Thanks for this! I think it's definitely reasonable. Could you rewrite the commit message to conform to the angular.js commit conventions. E.g. all we'd need is to change it from Adds support for coffee with 'multi' parser to feat: Adds support for coffee with 'multi' parser

This way semantic release will pick it up and release it automatically.

Thanks!

@lteacher lteacher force-pushed the patch-support-coffee-files branch from b220329 to 45413a6 Compare August 3, 2017 22:31
- Changes multi.js spec regex to include .coffee files

New feature
@lteacher lteacher force-pushed the patch-support-coffee-files branch from 45413a6 to 0dcfe7a Compare August 3, 2017 22:40
@lteacher
Copy link
Contributor Author

lteacher commented Aug 3, 2017

@NickTomlin no problem. I updated the message. Couldn't seem to get rid out the footer warning and seems the build fails linting some other commit?

@wswebcreation
Copy link
Collaborator

Really nice addition!

@NickTomlin , this PR adds the filetype to it, but could it also be something to make this an argument for protractor-flake?
Reason why I'm popping this up is that I'm not sure if this will work with for example jasmine and typescript when you do the compiling on the fly in the onPrepare. I think you would get file-type *.ts in the log and that will result in nog finding the specific file.

@NickTomlin
Copy link
Owner

@lteacher ah that new failure is actually my fault for updating the readme without following my own conventions! Feel free to ignore it, I can manually merge this in.

@wswebcreation I hadn't thought of that. I think it's a good suggestion! Perhaps we can do something like .(js|coffee|ts) by default and then add support for configuring additional extensions via the config? I'm fine with merging this in as-is and then doing that work later (if you wan to submit a PR feel free!)

@NickTomlin NickTomlin closed this in a466f34 Aug 4, 2017
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