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

Improve Plug.Debugger #552

Closed
josevalim opened this Issue May 20, 2017 · 11 comments

Comments

Projects
None yet
3 participants
@josevalim
Member

josevalim commented May 20, 2017

As per the discussion here, we can:

  1. Show the documentation for a function whenever it is clicked on the stacktrace
  2. If a particular stacktrace entry has arguments, we should show them as well
  3. In case of a function clause error, we will have a particular session that shows all possible clauses for that particular function (with a limit)

@rstacruz, since you worked on the original design, could you please give us a hand here? At the HTML/JS side, what we need is:

  1. Support a section that will change whenever a different stacktrace is clicked and show its documentation. Documentation may not always be available.
  2. Support a section that will change whenever a different stacktrace is clicked and show the stacktrace arguments. This may not always be available.
  3. Support a special section that will be shown only for certain errors with the available function clauses

I will work on the Elixir side of things but your help will be really appreciated on the design side. Thank you! ❤️

@josevalim

This comment has been minimized.

Member

josevalim commented May 20, 2017

@rstacruz here is a screenshot of what I have so far:

screen shot 2017-05-20 at 20 11 53

On the left side, notice:

  1. Link to the docs
  2. The "Called with" pane
  3. The "Available clauses" pane

The work is on this branch.

I have included the app that reproduces this exact page in a gist. The last pane requires Elixir master and OTP 20-rc.1 so it may be tricky reproducing. For this reason the whole HTML page is in the gist too.

@hakunin

This comment has been minimized.

hakunin commented May 20, 2017

Looks awesome! This would probably be a future enhancement, but I wonder if it could also color code which parts of the clause matched / missed and sort them by most plausible at the top.

I don't know how easy/hard it would be to do, just throwing it out there.

@josevalim

This comment has been minimized.

Member

josevalim commented May 21, 2017

We can probably do the red/green but re-ordering would be really complicated.

@josevalim

This comment has been minimized.

Member

josevalim commented May 21, 2017

Red/green has been added, thanks for the suggestions @hakunin!

screen shot 2017-05-21 at 21 50 33

@hakunin

This comment has been minimized.

hakunin commented May 21, 2017

Wow now I see it, this brings a lot of clarity! Even if you could do it in your head, it just saves those precious mental cycles. Love it!

@rstacruz

This comment has been minimized.

Contributor

rstacruz commented May 23, 2017

Happy to help! I'll go have a look.

@rstacruz

This comment has been minimized.

Contributor

rstacruz commented May 23, 2017

By the way, I think the JS side is inherited from @charliesome's work on better_errors for Ruby. Been a while, but I don't remember refactoring his work on it; nevertheless I know my way around JS pretty well :)

@rstacruz

This comment has been minimized.

Contributor

rstacruz commented May 24, 2017

btw, do we also have Called with %i arguments listed even if the error isn't no function clause matching %s? Because that'd be awesome.

@josevalim

This comment has been minimized.

Member

josevalim commented May 24, 2017

@rstacruz if the information is available, we will show it, but it depends on the VM keeping this information in the stacktrace, so it is unfortunately out of our control.

@josevalim

This comment has been minimized.

Member

josevalim commented May 28, 2017

@rstacruz please let us know if you need any help to move this forward. We want to release a new Plug version and your approval/changes would mean a lot to the debugger changes. :)

@josevalim

This comment has been minimized.

Member

josevalim commented Jun 5, 2017

Since we are planning to release Elixir v1.5-rc this week, I went ahead and slightly improved my branch and merged it. @rstacruz, if you have any suggestions, we would still love to hear them!

Thanks @hakunin for the initial suggestions!

@josevalim josevalim closed this Jun 5, 2017

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