whitequark / ast

A library for working with Abstract Syntax Trees.
MIT License
195 stars 25 forks source link

debug inspect #6

Closed rubydesign closed 9 years ago

rubydesign commented 9 years ago

great stuff , thanks.

This just reduced my 10 (hand coded ast-) classes to nothing. Great :-)

In doing that i was obviously using the handy s() function. But also i wrote a modified Node.inspect that gets included with the s() and prints out the node in the exact same way as the code needs it.

The effect being that one can copy paste the failed test output as an expected result (it's ruby code, yeah)

I found that very handy and could make a pull request. Code is here https://github.com/dancinglightning/ast/commit/fdb4d8cb7a36601d76750dab658893e30e85c8c9

whitequark commented 9 years ago

Ideally, to_s would return the current representation and inspect would return yours; however, I don't know how many people rely on the return values being what they are, and it's probably not worth a major version bump.

mbj commented 9 years ago

Data point: I do not rely on AST::Node#to_s or #inspect format in my projects.

whitequark commented 9 years ago

What about @bbatsov?

rubydesign commented 9 years ago

my idea was that you only get it when you want it. One could decouple it from including the Sexp and make different module if there are concerns.

whitequark commented 9 years ago

It is definitely a past design failure in ast.

bbatsov commented 9 years ago

I'm on the road right now. Will be back home tomorrow and I'll take a look then.

On Tuesday, September 15, 2015, whitequark notifications@github.com wrote:

It is definitely a past design failure in ast.

— Reply to this email directly or view it on GitHub https://github.com/whitequark/ast/issues/6#issuecomment-140460679.

Best Regards, Bozhidar Batsov

http://www.batsov.com

rubydesign commented 9 years ago

After having used this a bit more, i actually reverted to the multiline version. (wasn't before) It now outputs exactly what is written in the module comment. Even in ruby code, that can still be pasted, as the comma at the end continues the expression. (though i personally changed to text files, less clutter) https://github.com/dancinglightning/ast/commit/846eb00aeb6d227ffbae7a3f5c3b286f60d11667

bbatsov commented 9 years ago

Ideally, to_s would return the current representation and inspect would return yours; however, I don't know how many people rely on the return values being what they are, and it's probably not worth a major version bump.

I'm not using this is as well, so I think making this change is unlikely to affect many (any) users.

whitequark commented 9 years ago

OK. I will accept a PR implementing this as per https://github.com/whitequark/ast/issues/6#issuecomment-140368790.

rubydesign commented 9 years ago

ok, do you want it in the sexp module (as my code is now), or should i change the node.to_sexp to output ruby code ?

whitequark commented 9 years ago

No, no modules. Just inspect (outputting the code) and to_s (aliased to to_sexp).

rubydesign commented 9 years ago

i guess this is clear