Add more info for :with expressions in dbg/1

[sabiwara]

Take 2 for #12477, without using the process dict, as a draft for discussion.
I also added tests for guards and WithClauseError which needed to be taken into account.

[josevalim]

Can we include the format we were expecting as well? So the whole {:ok, width} <- Map.fetch...? And then the outcome in the next line:

Suggested change

	Map.fetch(opts, :width) #=> {:ok, 10}
	{:ok, width} <- Map.fetch(opts, :width)
	#=> {:ok, 10}

Although I also worry that this will be too verbose for actual long values. Imagine long clauses with each of them returning a struct as a value (or an ecto schema).

So I do wonder if the best way to go is indeed to say which clause failed, allowing the user to further debug themselves.

[sabiwara]

Can we include the format we were expecting as well? So the whole {:ok, width} <- Map.fetch...?

My thinking here is that we print the whole with just below, so just showing the expressions felt enough to debug the pipeline? As you said, I was fearing it could be too verbose otherwise.

allowing the user to further debug themselves.

I agree this might be a reasonable approach if really there is an issue with showing all steps, but given this is what we do for pipelines I don't think it should be an issue?

Most of the complexity comes from handling guards & raise, showing all steps is not the hard part I think.

[josevalim]

My thinking here is that we print the whole with just below, so just showing the expressions felt enough to debug the pipeline?

I see, sounds good.

I agree this might be a reasonable approach if really there is an issue with showing all steps, but given this is what we do for pipelines I don't think it should be an issue?

But can we argue any other behaviour for pipelines? :)

[sabiwara]

But can we argue any other behaviour for pipelines? :)

Actually... :) One could argue that one might just add |> dbg() after every step they care about, and we just show the last one, in order to avoid noise. Allowing the user to further debug themselves.
I very rarely felt this way, but it did happen to me very anecdotally to fallback to |> IO.inspect after a pipeline for this reason.

[josevalim]

That's a good point. And it is actually easier to dbg individual parts in a pipeline then in a with.

I guess another way to explore this is: if we had an actual debugger, would we like to go clause by clause and else by else? The answer would be yes, so we should probably follow that principle. So this looks good to me.

[dkuku]

This is a good example that shows that we should add a bit more info why this failed - it looks ok at the beginning but it's the problematic line that did fail and you have to jump back and forth to compare.
Often you're debugging code written by someone else and there may be more lines inbetween.
It can even use elixir syntax:

match?({:ok, height} when is_integer(height), {:ok, 0.0}) #=> false
or 
{:ok, height} when is_integer(height) <- {:ok, 0.0}

[dkuku]

Another example is pattern match on map key but the map is not including the key. This is one of the hardest to spot for me.

[dkuku]

What about showing it this way?

             {:ok, width} <- {:ok, 10}
             {:ok, height} <- :error

[sabiwara]

It's a trade-off, cf discussion above.
The risk is to get unwieldy and cluttered logs for bigger patterns, especially since the useful info is the value itself and the pattern is redundant since it's being logged just after:

This approach is also consistent with what we do for case: we show the value being matched followed by the whole expression, without highlighting each pattern that matched or didn't:

So, I think it's a reasonable trade-off, even if I understand for this small example that it might look nice.

Another possibility to address the "clutter" concern could be to inspect the pattern algebra with a limit, but I don't think that's possible today.

However, if I'm overthinking the verbosity concern and everybody is convinced of the benefit, I could easily change the code to replace with the whole arrow node.