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

Add jade2php adapter #65

Closed
janwirth opened this issue Jan 7, 2015 · 19 comments
Closed

Add jade2php adapter #65

janwirth opened this issue Jan 7, 2015 · 19 comments
Labels

Comments

@janwirth
Copy link

janwirth commented Jan 7, 2015

The compiler interface should be similar, if not identical to jade's, so i would try this myself, but i have no idea how to set up an environmet to code and test accord adapters.
@Jenius could you help me out right there? that would be killer.

https://www.npmjs.com/package/jade-php

@janwirth janwirth closed this as completed Jan 7, 2015
@janwirth janwirth reopened this Jan 7, 2015
@janwirth
Copy link
Author

janwirth commented Jan 7, 2015

I guess an approach this as outlined by @slang800 would be the way to go.
correct me if I'm wrong.

@notslang
Copy link
Contributor

notslang commented Jan 7, 2015

Yeah, or you could just fork accord & implement it like any other adapter.

Also you might want to be careful with the way that jade-php mutates the jade instance. I've never used it before, but looking at the usage example, it seems like it duck-punches jade and might screw with the global jade instance. Not totally sure, but it looks hacky.

@janwirth
Copy link
Author

janwirth commented Jan 7, 2015

Compiling jade still works even after running jade-php on loads of files.
As i mentioned, i have no idea how to setup a dev environement for creating and testing accord adapters

@notslang
Copy link
Contributor

notslang commented Jan 7, 2015

I mean when jade-php is being run within the same process as regular jade compilations.

It's pretty simple to setup a dev env:

  • make sure node.js / npm, and git are installed
  • fork & clone the accord repo into your machine
  • cd into the accord directory you just made
  • run npm install to get all the deps
  • start coding & use npm test to run the test cases

@janwirth
Copy link
Author

janwirth commented Jan 8, 2015

Oh, of course. thanks alot. As I barely have used testing so far I am not aware of use cases like this.
I will file a pull request once I get something working.

@notslang
Copy link
Contributor

notslang commented Jan 8, 2015

Oh, our tests are mostly just comparing the result of compiling/rendering a bunch of files to known outputs. If you look around the tests directory you'll get the hang of it.

@jescalan
Copy link
Owner

jescalan commented Jan 8, 2015

Hey @FranzSkuffka -- thanks for pitching in! If you have never worked with this kind of environment it definitely could be some time and effort before you get used to it. If you are working on some code for this and need help, feel free to open up a "Work in progress" pull request (just put [WIP] in the title), and we will review it.

You should definitely get familiar with testing practices before submitting anything, this is pretty important for projects like accord. Check out the contributing file for some help.

@jescalan jescalan added the engine label Jan 8, 2015
@jescalan jescalan changed the title integrate jade-php Add jade-php adapter Jan 8, 2015
@janwirth janwirth changed the title Add jade-php adapter Add jade2php adapter Jan 8, 2015
@janwirth
Copy link
Author

janwirth commented Jan 8, 2015

@Jenius i have checked out the contribution tips yesterday already and invested some time in copying the jade adapter tests etc. to test jade php. I think I'm somewhat down with the test workflow already as it makes perfect sense.
@slang800 i got what you meant with jade-php duck-punching the jade instance - seems like huge bogus to me. fortunately i have found the fully tested and more advanced alternative called jade2php .
I will try to create a jade2php adapter instead.

@janwirth
Copy link
Author

jade2php, tho, is not production-ready either and needs quite some work.

@jescalan
Copy link
Owner

Ouch. Yeah this is something we sometimes discover in the process of trying to add adapters. This leads to either contributing back to the original library if we are particularly invested, or searching for a different solution, usually.

@janwirth
Copy link
Author

Jade2php is ready for a production-ready release soon.

I am glad you were able to make dogescript work!

@jescalan
Copy link
Owner

Ok cool! I guess we will wait until they are ready, then get merge in this pull request.

Oh yeah dogescript was the first priority. Much important. Very wow.

@janwirth
Copy link
Author

I will send another pull request once i've written a proper adapter. i even missed the @compile call this time..
regarding the @compile private method: isn't it the same with most of the adapters? Why isn't it part of the adapter class as it does not access any local properties or functions?

@jescalan
Copy link
Owner

Yeah, this is a good point. It's because it's the same in most adapters, but different in some. It just provides a way to catch any stray error that might be thrown from a compile method, really. Since it's different in a number of adapters, it is harder to pull out.

You don't have to use it by any means, it's just usually useful for catching errors. If the compiler throws errors in a different way, for example, you don't need it. But by passing invalid code, I was able to get most adapters to throw an error, which can only be caught and passed on using a try/catch. You can see error tests for a number of the adapters, try to generate an error for yours and make sure it's caught, then you are in the clear!

@janwirth
Copy link
Author

Aye aye captain.
I think was about to write so many words, but, you know, YAGNI. lol. 😄

@jescalan
Copy link
Owner

Closing as we've given up on this one for now

@janwirth
Copy link
Author

Hey there,
I'm currently implementing the adapter, forked from the jade adapter. It does compile as I saw when printing the res variable. But I am missing something here: The object that the compile function returns still contains the result, unlike that of the other adapters. When running the tests i get the following error:
3) jade2php should compile a string:
Uncaught TypeError: Property 'result' of object # is not a function

Source:
https://github.com/FranzSkuffka/accord/blob/master/lib/adapters/jade2php.coffee

Could you help me out on this one?

@jescalan
Copy link
Owner

I'm sorry, I don't have the bandwidth at the moment to be able to help. Hopefully someone else will be able to!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants
@jescalan @notslang @janwirth and others