search

edvinasbartkus / grails-coffeescript-resources

Another CoffeeScript compiler for Grails
Other
20 stars 10 forks source link

Move away from jCoffeeScript library #7

Closed bobbywarner closed 12 years ago

bobbywarner commented 12 years ago

This is just a suggestion after I reviewed the jCoffeeScript project code (https://github.com/yeungda/jcoffeescript).

I suggest we just pull in the code necessary to compile CoffeeScript directly into the plugin and not have a dependency on the jCoffeeScript library. Here's my reasons:

First, the code is so minimal (it's essentially just one class, JCoffeeScriptCompiler) that having a dependency on another library doesn't seem worth it.

Second, it doesn't look like jCoffeeScript is keeping up with the latest release of CoffeeScript (currently 1.2) and it would be easier for this Grails plugin to remain current with the latest release by not having this additional dependency.

Please let me know what you think. As I mentioned above, it's just a suggestion. Thanks for the great work on the plugin!!

Thanks, Bobby

edvinasbartkus commented 12 years ago

The reason why I have used jCoffeeScript was that I wanted to make project independent from any native commands. For Windows users it has been big problem to install CoffeeScript for a long time. With java compiler they could easily use this plugin.

I believe the best approach would be to try to find native coffeescript compiler on the system and compile the coffeescript files. If that is not possible then jCoffeeScript could be used with warnning.

For this issue I think I will do the following steps:

  1. Add native (system) coffeescript compile support
  2. Use coffeescript native by default
  3. Add an option to choose which compiler to use
  4. Option to specify coffeescript compile script path

What do you think? :) I have managed to integrate the plugin with resources plugin. I will write tests and if everything runs ok next week 0.3 milestone will be done.

bobbywarner commented 12 years ago

No, I think you misunderstood what I was suggesting or I didn't state it correctly. Don't use native or look for system CoffeeScript binaries. It's perfect just the way you have it as no native binaries are needed and everything works on the JVM.

I was just suggesting to copy the code in JCoffeeScriptCompiler.java into a class in the plugin so that we can change it (upgrade to new CoffeeScript releases) more rapidly as needed without having to wait for jCoffeeScript to be updated (it was last updated in May). The plugin would still only use the JVM, just a plugin class to compile the CoffeeScript instead of a class in jCoffeeScript. Make sense? I can work on a pull request for this if you'd like. Please let me know.

Glad to hear you have it integrated with Resources. Awesome work! Thanks!

Thanks, Bobby

edvinasbartkus commented 12 years ago

Yes, I have misunderstood you :-)

But, wouldn't it be better to make pull requests to jCoffeeScript project? That way there can be more people with benefit. Plugin could only use the latest jCoffeeScript from Maven repo. It might be difficult to maintain CoffeeScript compiler on our own. However, haven't checked how jCoffeeScript is built so don't know what is the level of difficulty :-)

If you have time and will to make it I would accept pull request :)

bobbywarner commented 12 years ago

Yes, I absolutely agree that updating jCoffeeScript would be better so more people could benefit, but it looks like it's just not that active. For instance, there was a pull request opened back in September to update to CoffeeScript 1.1.2 and it was never committed (https://github.com/yeungda/jcoffeescript/pull/14).

I see this grails-coffeescript-resources plugin becoming very popular with the framework very quickly (based on the responses I've received from my screencasts) and just want to make sure we can keep current with the latest version of CoffeeScript.

Sounds good! I'll work on a pull request! Thanks again!! :)

jfaissolle commented 12 years ago

Moreover, the jCoffeeScript jar contains the Rhino engine instead of pulling it as a dependency. This gives me conflict headaches. So thumbs up for this one !

halfbaked commented 12 years ago

Only seeing this now, but it seems to be related to my pull request. https://github.com/edvinasbartkus/grails-coffeescript-resources/pull/11

Any comments welcome...

bobbywarner commented 12 years ago

@halfbaked Yea, this looks exactly like what I was going to do (just hadn't had time to put together the pull request yet!). Once this pull request get committed, I'll update my fork and see what else I can add. Good stuff!!

Thanks, Bobby