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

Interpreter: allow some options, and colorize whereami #12198

Merged
merged 3 commits into from
Jul 6, 2022

Conversation

asterite
Copy link
Member

@asterite asterite commented Jul 4, 2022

This PR allows passing the following options to the interpreter:

  • -D FLAG
  • --error-trace
  • -h
  • --no-color

In addition to that, if color is desired, the output of whereami and also the output when you run into debugger is colored.

Before this PR:

image

With this PR:

image

With this PR and --no-color (notice that the line numbers are no longer colored):

image

Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

@HertzDevil
Copy link
Contributor

Could you also add --prelude and --link-flags?

@asterite
Copy link
Member Author

asterite commented Jul 4, 2022

I initially added prelude, but the interpreter depends on exit and nothing really works without the prelude. For example inspecting values won't work.

Link flags... I think that's a new feature. When will they show up?

I actually intended to add coloring support in this PR. I added some other options that were trivial to implement. Could we maybe leave the other, more colplex ones, for later PRs?

@HertzDevil
Copy link
Contributor

Never mind about --link-flags, I was thinking about Windows but -D is probably enough.

What do you mean by exit?

@asterite
Copy link
Member Author

asterite commented Jul 4, 2022

When you do crystal i and then you quit, the interpreter calls exit manually by creating a Call and then interpreting that:

private def interpret_exit
interpret(Call.new(nil, "exit", global: true))
end

Without the default prelude, that doesn't work. Well, maybe it will work with some other prelude, but not with the empty one. I guess we could still add the option. After all it's a "use this if you want to, but you probably shouldn't".

Here's how it works right now with --prelude=empty:

➜  crystal git:(feature/interpreter-options-and-colorize) ✗ bin/crystal i --prelude=empty
icr:1:0> 1
=> Error while calling inspect on value: undefined method 'inspect' for Int32
1
icr:2:0> a = 1
=> Error while calling inspect on value: undefined method 'inspect' for Int32
1
icr:3:0> Showing last frame. Use --error-trace for full trace.

Error: undefined local variable or method 'exit' for top-level

Not a very pleasant experience :-)

Here's interpreting a file that only does a = 1:

Showing last frame. Use --error-trace for full trace.

Error: undefined local variable or method 'exit' for top-level

Again, not very useful.

But I guess it doesn't hurt either to add that option to be able to experiment with other preludes.

@straight-shoota
Copy link
Member

it doesn't hurt either to add that option to be able to experiment with other preludes.

Yes, I agree on that. Choosing a non-standard prelude is an option which is only useful when you know what you're doing. Even with compiled crystal. I think it makes sense to allow it on the interpreter as well.

We could easily improve the error messages or fall back to not calling methods that don't exist. (that's low priority though)

@asterite
Copy link
Member Author

asterite commented Jul 5, 2022

Will this make it to 1.5.0?

@straight-shoota straight-shoota added this to the 1.5.0 milestone Jul 5, 2022
@straight-shoota
Copy link
Member

Sure. It only affects the interpreter.

@straight-shoota straight-shoota merged commit 8a38594 into master Jul 6, 2022
@straight-shoota straight-shoota deleted the feature/interpreter-options-and-colorize branch July 6, 2022 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants