-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add breakpoint support to IEx.pry #6307
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
Conversation
230680d to
7ad8d4f
Compare
7ad8d4f to
a19ac52
Compare
d6c46d6 to
cfede2f
Compare
|
|
|
I guess we are trying to be consistent with |
|
"whereami" is a common debugger instruction so I wouldn't add the underscores. I am ok with replacing it by something else entirely though. What other names do other debuggers use?
|
cfede2f to
b89ea99
Compare
|
GDB is using Python debugger is using And Perl debugger is using |
|
Thank you @slashmili! I think |
3bfd67a to
c9d2a23
Compare
lib/iex/lib/iex/pry.ex
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the tuple 3 elements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thank you!
ac22534 to
9e28e66
Compare
a1aca33 to
29c2503
Compare
29c2503 to
ca02358
Compare
lib/elixir/src/elixir_expand.erl
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is kind of hacky, isn't it? It's just to get around the foo warning. It also makes the regular shell and shell-in-pry behave differently for code like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation is sound but the idea is debatable. I have tried the following approaches:
- Autocompletion of nullary calls - it is a bad user experience because it works only some times (for example, it wouldn't work for
open/0andwhereami/0because we also have other arities) - Parens autocompletion of all calls - if we add only
(, we get incomplete input most of the times. If we add both, then we need to go back one position (currently it is not possible to add both and then put the cursor on the middle) - Accept
continuein the shell - the UX is great, the inconsistency is, well, inconsistent
We can bring the warning back once we figure out autocompletion but since this feature is about the user experience, I have put it first here.
lib/iex/lib/iex.ex
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given this section, should we talk somewhere about interrupt from the Ctrl-g menu? It's more powerful in a way it can solve this situation, but also infinite loops & waiting in a receive without a message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scrap that, I just read this bit further
ca02358 to
a85748a
Compare
lib/iex/lib/iex/pry.ex
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a hash of the file stored somewhere or something similar? My experience with that kind of source printing is that if the source changes, something completely irrelevant and extremely confusing can be printed. If we had a hash we could display some kind of warning that file changed on disc compared to what we have compiled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea. There is no hashing though. But something we could do for now is to print mod.fun/arity we are debugging. That will help with making it clearer in many cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do have the module version but that is built on the AST.
a85748a to
9ea814f
Compare
|
For what it's worth, in sdb, I went with |
88625ff to
b11df4d
Compare
| private functions of the module being pried. Module functions | ||
| still need to be accessed via `Mod.fun(args)`. | ||
| Alternatively, you can use `IEx.break!/4` to setup a breakpoint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to be more concise here and forward to the documentation of IEx.break!/4? So that we don't have to repeat the warning for OTP 20+ and all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just wanted to give a quick run down of the pros and cons so the user doesn't have to figure it out for themselves.
lib/iex/lib/iex.ex
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really an ArgumentError? It feels like most of the errors above are not, for example the :otp_20_is_required. What if we introduce a IEx.BreakError or something similar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I will go with runtime error.
lib/iex/lib/iex/config.ex
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpicky nitpick but I guess we tend to go with no [] on last argument kw list:
Agent.start_link(__MODULE__, :handle_init, [], name: @agent)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good call.
lib/iex/lib/iex/evaluator.ex
Outdated
| put_history(state) | ||
| handle_eval(Code.string_to_quoted(code, [line: line, file: "iex"]), code, line, iex_state, state) | ||
| put_whereami(state) | ||
| quoted = Code.string_to_quoted(code, [line: line, file: "iex"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would say same here for [] with options.
| @doc """ | ||
| Opens the current prying location. | ||
| This command only works inside a pry session started manually |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IEx.pry/0, otherwise no linking?
| # In both cases, we are safe to print and the request will | ||
| # suceed. | ||
| request = | ||
| case IEx.Server.evaluator do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IEx.Server.evaluator()?
lib/iex/lib/iex/pry.ex
Outdated
| end | ||
|
|
||
| def handle_call(:remove_breaks, _from, _counter) do | ||
| # Make sure to deinstrumented before clearing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick but "deinstrumented" -> "deinstrument".
| @table | ||
| |> :ets.match({:_, :"$1", :_, :_}) | ||
| |> List.flatten | ||
| |> Enum.uniq |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bunch of parens for this calls?
| {:dictionary, dictionary} = Process.info(pid, :dictionary) | ||
| dictionary[:evaluator] | ||
| {dictionary[:evaluator], pid} | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can maybe be refactored as
if pid = IEx.Server.local() do
...
else
nil
endbut I don't have preference, just wanted to mention it.
| self_leader = Process.group_leader | ||
| evaluator = opts[:evaluator] || | ||
| :proc_lib.start(IEx.Evaluator, :init, [:ack, self_pid, self_leader, opts]) | ||
| :proc_lib.start(IEx.Evaluator, :init, [:ack, self(), Process.group_leader, opts]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
() as well I guess 😄
3e89dd3 to
f3c826d
Compare


The main interface is a
IEx.break!/4function that sets up a breakpoint inmodule,functionandaritywith the given number ofstops.This function will instrument the given module and load a new version in memory with breakpoints at the given function and arity. If the module is recompiled, all breakpoints are lost.
When a breakpoint is reached, IEx will ask if you want to
prythe given function and arity. In other words, this works similar toIEx.pry/0as the running process becomes the evaluator of IEx commands and is temporarily changed to have a custom group leader. However, differently fromIEx.pry/0, aliases and imports from the source code won't be available in the shell.IEx helpers includes many conveniences related to breakpoints. Below they are listed with the full module, such as
IEx.Helpers.breaks/0, but remember it can be called directly asbreaks()inside IEx. They are:break!/2- sets up a breakpoint for a givenMod.fun/aritybreak!/4- sets up a breakpoint for the given module, function, aritybreaks/0- prints all breakpoints and their idscontinue/0- continues until the next breakpoint in the same shellopen/0- opens editor on the current breakpointremove_breaks/0- removes all breakpoints in all modulesremove_breaks/1- removes all breakpoints in a given modulereset_break/1- sets the number of stops on the given id to zeroreset_break/3- sets the number of stops on the given module, function, arity to zerorespawn/0- starts a new shell (breakpoints will ask for permission once more)whereami/1- shows the current locationBy default, the number of stops in a breakpoint is 1. Any follow-up call won't stop the code execution unless another breakpoint is set.
Alternatively, the number of be increased by passing the
stopsargument.IEx.Helpers.reset_break/1andIEx.Helpers.reset_break/3can be used to reset the number back to zero. Note the module remains "instrumented" even after all stops on all breakpoints are consumed. You can remove the instrumentation in a given module by callingIEx.Helpers.remove_breaks/1and on all modules by callingIEx.Helpers.remove_breaks/0.To exit a breakpoint, the developer can either invoke
continue(), which will block the shell until the next breakpoint is found or the process terminates, or invokerespawn(), which starts a new IEx shell, freeing up the pried one.This functionality only works on Elixir code and requires OTP 20+.
Examples
The following sets up a breakpoint on
URI.decode_query/2:The following call will setup a breakpoint that stops once.
To set a breakpoint that will stop 10 times:
IEx.break!/2is a convenience macro that allows breakpoints to be given in theMod.fun/arityformat:Or to set a breakpoint that will stop 10 times:
This function returns the breakpoint ID and will raise if there is an error setting up the breakpoint.
Breaks and mix test
To use
IEx.break!/4during tests, you need to run Mix insideiexand pass the--tracetomix testto avoid running into timeouts: