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

debug inspect #6

Closed
rubydesign opened this issue Sep 15, 2015 · 12 comments
Closed

debug inspect #6

rubydesign opened this issue Sep 15, 2015 · 12 comments

Comments

@rubydesign
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor

mbj commented Sep 15, 2015

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

@whitequark
Copy link
Owner

What about @bbatsov?

@rubydesign
Copy link
Contributor Author

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
Copy link
Owner

It is definitely a past design failure in ast.

@bbatsov
Copy link

bbatsov commented Sep 15, 2015

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
#6 (comment).

Best Regards,
Bozhidar Batsov

http://www.batsov.com

@rubydesign
Copy link
Contributor Author

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
Copy link

bbatsov commented Sep 18, 2015

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
Copy link
Owner

OK. I will accept a PR implementing this as per #6 (comment).

@rubydesign
Copy link
Contributor Author

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
Copy link
Owner

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

@rubydesign
Copy link
Contributor Author

i guess this is clear

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

No branches or pull requests

4 participants