search

HaxeCheckstyle / haxeparser

A Haxe parser for Haxe
61 stars 23 forks source link

In is now a binop #40

Closed ibilon closed 7 years ago

ibilon commented 7 years ago

Works for all but one test (including a std parse)

* Test::testIn()
ERR: Test.hx:185(Test.testIn) - expected 'a in b' but was 'a  in  b'

which is eeq("a in b"); because new haxe.macro.Printer("").printBinop(OpIn) gives in (there is one space before and after in).

No idea how to fix that.

Simn commented 7 years ago

I think this requires some conditional compilation so it still works with Haxe 3...

ibilon commented 7 years ago

Right forgot about that, fixed and modified travis to test both.

ibilon commented 7 years ago

Travis is finally fixed.

So do you think haxe's printBinop should be changed? Or would that be an issue since in does require spaces around it?

Simn commented 7 years ago

Good question about printing this... let's get @nadako's opinion.

nadako commented 7 years ago

That depends on what's the actual purpose of printBinop... I assume it's for outputting a certain token for given binop, not depending on a context. With that in mind it shouldn't add spaces to in, because spaces around it only make sense in context of printing a EBinop expression where it already has spaces IIRC.

So yeah, printBinop should be fixed to just return "in".

Simn commented 7 years ago

Sounds good to me!

Simn commented 7 years ago

Any idea how long it takes before it uses the updated Haxe?

ibilon commented 7 years ago

As soon as it is uploaded, which is after the third task of https://travis-ci.org/HaxeFoundation/haxe/builds/265974571 finishes.

ibilon commented 7 years ago

Looks like it's up now.

Simn commented 7 years ago

Thanks!