refactor dbg for if

[dkuku]

@josevalim This is what I came up.
It requires calling the macro module where we have defined the functions that print the data.
It looks a bit confusing calling Macro.write so I think it makes sense to move all the logic to macro/dbg.ex then the implementation would call Dbg.write or similar. In case it will show up in traces it will look less confusing.

[dkuku]

It uses the dbg function and prints the header now in the test output before it crashes, so it has to be muted.
Before the changes any crash was silent.

[dkuku]

It's not great.

[whatyouhide]

I read the discussion in the other issue, but chiming in only now. So, are we sure the best approach here is to print along the way just in case something raises? Right now the dbg "architecture" is this:

1. Rewrite AST to capture intermediate results (for if, |>, and so on)
2. Pass those captured results to a formatting function at the end, which prints them out

Maybe a good approach here would be to try/catch to capture the output, pass the caught error to the formatting function, and then re-raise it?

[josevalim]

We discussed try/catch it and I would like to avoid that if possible. My concern is about changing the execution semantics too much and then the debugger starts affecting the system itself.

[josevalim]

@dkuku I think there is probably a cleaner solution here which is to break if into two "debuggable ASTs", one for the header and another for the body. So the structure would be pretty close to what we have to day, it is just that if emits two of them. WDYT?

[dkuku]

👍🏻 ok, got it

[whatyouhide]

@josevalim I’m just concerned about turning this into a pure operation (AST → AST with output as well) into an impure one, where we write to stdout as we go. I’m a bit out of touch with this, we might already have something callback-based and whatnot, but just wanted to make sure to call it out.

[josevalim]

@whatyouhide my proposal is to keep it as is, we just offer a bit more of granularity. For example, if is broken into two debuggable ASTs instead of one. It should be pretty close.

[whatyouhide]

@josevalim yeah I guess that's a good and not-very-invasive option to go for for the time being. 🙃

[dkuku]

Looking at the actual implementation of the dbg macro how this could be split to handle multiple results?

    quote do
      to_debug = unquote(dbg_ast_to_debuggable(code, env))
      unquote(__MODULE__).__dbg__(unquote(header), to_debug, unquote(options))
    end

I used if as a simple example to demonstrate refactoring, but the real issue for me lies in pipe chains where a failure at any point prevents the result from being printed. This forces me to add additional dbg or inspect calls somewhere before the failure to see the intermediate values that lead to the error. Imagine such a pipe—let’s say I want to modify the Enhance.data() function. To do this, I add a |> dbg() at the end of the chain and run the tests. Everything is printed correctly. However, when I make changes to the module and it fails, there is no output at all and I have to switch back to the place where I added the dbg call and add some additional ones before.

data
|> Build.query()
|> GetData.from_db()
|> Transform.data()
|> Enhance.data()

[josevalim]

Make dbg_ast_to_debuggable(code, env) perhaps return a list of expressions which you then transform as above? So the code becomes:

Enum.reduce(dbg_ast_to_debuggable(code, env), :ok, fn to_debug, acc ->
  quote do
      unquote(acc)
      unquote(__MODULE__).__dbg__(unquote(header), unquote(to_debug), unquote(options))
    end
end)

Or similar. You may need a reverse somewhere though.