fix(formatter): fix small issue where there was only one newline added

[pdgonzalez872]

to a def that is followed by a module attribute.

Closes #13866

---

I'm not sure this implementation is the best, but it seems to do the job. I wouldn't be surprised if there is a better way to solve it though. This was my naive approach.

The test cases show the current behavior (def + def works, but def + mod attr doesn't).

Feel free to adjust/modify/close/do whatever you'd like to it if you see it fit.

Thank you for all of your work as usual! ❤️

[josevalim]

We should not look at def, because we can have def, defp, defmacro, defmacrop... and projects can define defn, defnp, etc. We do have some code today that skips adding new lines if they are module attributes but perhaps the code should only avoid adding lines below, not lines up.

[pdgonzalez872]

Ah, that makes a lot of sense.

I'll try to play around with this in the next couple of days, but I won't be upset if you solve this in 2 minutes :)

No hard feelings :)

[pdgonzalez872]

@josevalim, seems closer now, but likely a bit far from the optimal solution.

I kinda wanted to offer a devil's advocate here, to the issue we put together 😂. After reading and working on the code a little bit, I came to appreciate the user's choice. I didn't think of this when seeing the issue for the first time mostly because I had not seen/worked with/knew how the formatter works.

I like how the formatter respect's the user's choice first. The crux to this devil's advocate argument is that the original code "wanted" to save space by doing the shorthard def.

If space was not an issue, the code would have used a multiline def, which the formatter already handles:

# pre-format
defmodule Example do
  @impl FooA
  def a() do
    :something
  end
  @impl FooB
  def b() do
    :something_else
  end
end

# post-format
defmodule Example do
  @impl FooA
  def a() do
    :something
  end

  @impl FooB
  def b() do
    :something_else
  end
end

Even after writing the above I'm more convinced that this is:

• not applicable
• likely a doc adjustment to document the user's choice rather than a behavior change

WDYT?

[sabiwara]

I agree, looking at all the formatting changes in this repo highlighted by the CI, this seems to have a much bigger impact than just the original example, and the user's choice perspective is a good point.

[josevalim]

Thank you @pdgonzalez872!

I agree with the overall conclusions and the diff from your PR was helpful in seeing that. I think this will impose too much and leaving control to the user is the best way to go here.