Output diffs on mix format --check-formatted

[NickNeck]

A suggestion for #12100 . The output would look like:

In case a single line was changed the changed spaces get a background colour. For multiple deleted or inserted lines just the text gets a colour.

I have written this for another project but maybe it fits in here.

[josevalim]

Suggested change

	old = String.split(old, "\n")
	new = String.split(new, "\n")
	old = String.split(old, ["\r\n", "\n"])
	new = String.split(new, ["\r\n", "\n"])

[josevalim]

But then what happens when two files are equal except for \r\n -> \n? So maybe it is better to keep it by your current split?

[NickNeck]

Yes, splitting with \n should be better. The last commit makes also changed \r visible.

iex(1)> Mix.TextDiff.format("a\r\nb\r\n", "a\nb\nc\n") |> IO.iodata_to_binary() |> IO.puts()
1   - |a↵
2   - |b↵
  1 + |a
  2 + |b
  3 + |c

:ok
iex(2)> Mix.TextDiff.format("a\r\n", "a\n") |> IO.iodata_to_binary() |> IO.puts()
1   - |a↵
  1 + |a

:ok
iex(3)> Mix.TextDiff.format("a\n", "a\r\n") |> IO.iodata_to_binary() |> IO.puts()
1   - |a
  1 + |a↵

:ok

[josevalim]

@NickNeck holy moly, this is awesome! ❤️

I think we want to keep the Mix.TextDiff module private though, unless @whatyouhide @sabiwara @ericmj @fertapric have a good suggestion of where it should be placed in the Elixir API.

[josevalim]

@NickNeck let's make Mix.TextDiff a function with @doc false at the bottom of Mix.Tasks.Format for now, so we can unit test it, and later on we will figure out a public API for it.

[sabiwara]

@NickNeck holy moly, this is awesome! ❤️

I think we want to keep the Mix.TextDiff module private though, unless @whatyouhide @sabiwara @ericmj @fertapric have a good suggestion of where it should be placed in the Elixir API.

I'm not sure, but maybe in the future it might be interesting to have a public Diff / Inspect.Diff / Debug.Diff module in Elixir exposing what is currently done by ExUnit.Diff and this new text diff functionality? But it would be good to have more use case examples pointing to the fact these could be reused out of their original context (ExUnit, Mix).

[sabiwara]

I'm probably overthinking this, but just wondering: maybe we don't need to cast it as a binary here and we can build an IO list?

• removing calls to IO.iodata_to_binary/1
• replacing map_join/3 by map_intersperse/3

[josevalim]

@NickNeck looks great! We just need to move TextDiff around and we can ship it! 👍

[davydog187]

I think you're missing an "r" on these ☺️

[NickNeck]

@NickNeck let's make Mix.TextDiff a function with @doc false at the bottom of Mix.Tasks.Format for now, so we can unit test it, and later on we will figure out a public API for it.

@josevalim I have moved Mix.TextDiff to Mix.Tasks.Format.text_diff_format/3.

[josevalim]

@NickNeck looks great! Can you please look at the failing tests?

Actually, I would suggest calling IO.iodata_to_binary/1 on the output of the function, as that would make those tests cleaner and resilient to refactoring. :)