requirejs support #9

merged 3 commits into from Nov 10, 2011


None yet
2 participants

ddlsmurf commented Nov 9, 2011

Reimported code (by @omarkhan and @frostedcheerios) for requirejs on top of master, #4 .

Looks fine here anyway =) Thanks!


omarkhan commented Nov 9, 2011

Can you include a few requirejs-based source files to test this against? Thanks

@omarkhan omarkhan commented on the diff Nov 9, 2011

@@ -27,3 +27,25 @@ exports.getFullName = (variable) ->
name += '.' + ( for p in'.')
return name
+exports.getAttr = (node, path) ->
+ ###
+ Safe function for looking up paths in the AST. If an attribute is
+ undefined at any part of the path, an object with is returned with the
+ type attribute set to null
+ ###
+ path = path.split('.')
+ nullObj = {type: null}
+ for attr in path
+ index = null
+ if '[' in attr
+ [attr, index] = attr.split('[')
+ index = index[..-2]

omarkhan Nov 9, 2011


This all looks a bit hacky. Wouldn't it be easier to use coffeescript's ?. to soak up null references rather than string parsing?


ddlsmurf Nov 9, 2011


I agree, this does handle indices, and I'm a coffeescript beginner so I'm not sure how it stacks up agains the generated code... I just copied it over from the existing work without thinking about it much, it seems to work fine with my requirejs work, and not interfere with normal behaviour.
Sorry, for much the same reasons I don't really have any sources to contribute. There is a ton of unit tests that looks easy enough to import, but I can't look into it any time soon. If you don't consider this prioritary, I will look into it eventually though.


ddlsmurf commented Nov 10, 2011

You scared me so I got the unit tests working again, run with jasmine-node --coffee spec/

omarkhan merged commit e10935c into omarkhan:master Nov 10, 2011


omarkhan commented Nov 10, 2011

Great, thanks. I'll refactor this when I have some time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment