Enhance difference highlighting for proper lists

lib/ex_unit/examples/difference.exs

@@ -38,7 +38,7 @@ defmodule Difference do
   end
 
   test "lists" do
-    list1 = ["One", :ok, make_ref(), {}]
+    list1 = ["Tvo", make_ref(), :ok, {}]
     list2 = ["Two", :ok, self(), {true}]
     assert list1 == list2
   end

lib/ex_unit/lib/ex_unit/diff.ex

@@ -40,13 +40,11 @@ defmodule ExUnit.Diff do
 
   # Char lists and lists
   def script(left, right) when is_list(left) and is_list(right) do
-    cond do
-      Inspect.List.printable?(left) and Inspect.List.printable?(right) ->
-        script_string(List.to_string(left), List.to_string(right), ?')
-      Inspect.List.keyword?(left) and Inspect.List.keyword?(right) ->
-        script_keyword(left, right)
-      true ->
-        script_list(left, right, [])
+    if Inspect.List.printable?(left) and Inspect.List.printable?(right) do
+      script_string(List.to_string(left), List.to_string(right), ?')
+    else
+      keywords? = Inspect.List.keyword?(left) and Inspect.List.keyword?(right)
+      script_list_new(left, right, keywords?)
     end
   end
 
@@ -81,6 +79,15 @@ defmodule ExUnit.Diff do
     String.myers_difference(string1, string2)
   end
 
+  defp check_if_proper_and_get_length([_ | rest], length),
+    do: check_if_proper_and_get_length(rest, length + 1)
+
+  defp check_if_proper_and_get_length([], length),
+    do: {true, length}
+
+  defp check_if_proper_and_get_length(_other, length),
+    do: {false, length + 1}
+
   # The algorithm is outlined in the
   # "String Matching with Metric Trees Using an Approximate Distance"
   # paper by Ilaria Bartolini, Paolo Ciaccia, and Marco Patella.
@@ -119,69 +126,79 @@ defmodule ExUnit.Diff do
     end)
   end
 
-  defp script_keyword(list1, list2) do
-    path = {0, 0, list1, list2, []}
-    result =
-      find_script(0, length(list1) + length(list2), [path])
-      |> format_each_fragment([])
-    [{:eq, "["}, result, {:eq, "]"}]
+  defp script_list_new(list1, list2, keywords?) do
+    {proper1?, length1} = check_if_proper_and_get_length(list1, 0)
+    {proper2?, length2} = check_if_proper_and_get_length(list2, 0)
+
+    if proper1? and proper2? do
+      initial_path = {0, 0, list1, list2, []}
+      result =
+        find_script(0, length1 + length2, [initial_path], keywords?)
+        |> format_each_fragment([], keywords?)
+      [{:eq, "["}, result, {:eq, "]"}]
+    else
+      script_list(list1, list2, [])
+    end
   end
 
-  defp format_each_fragment([{:diff, script}], []),
+  defp format_each_fragment([{:diff, script}], [], _keywords?),
     do: script
 
-  defp format_each_fragment([{kind, elems}], []),
-    do: [format_fragment(kind, elems)]
+  defp format_each_fragment([{kind, elems}], [], keywords?),
+    do: [format_fragment(kind, elems, keywords?)]
 
-  defp format_each_fragment([_, _] = fragments, acc) do
+  defp format_each_fragment([_, _] = fragments, acc, keywords?) do
     result =
       case fragments do
         [diff: script1, diff: script2] ->
           [script1, {:eq, ", "}, script2]
 
         [{:diff, script}, {kind, elems}] ->
-          [script, {kind, ", "}, format_fragment(kind, elems)]
+          [script, {kind, ", "}, format_fragment(kind, elems, keywords?)]
 
         [{kind, elems}, {:diff, script}] ->
-          [format_fragment(kind, elems), {kind, ", "}, script]
+          [format_fragment(kind, elems, keywords?), {kind, ", "}, script]
 
         [del: elems1, ins: elems2] ->
-          [format_fragment(:del, elems1), format_fragment(:ins, elems2)]
+          [format_fragment(:del, elems1, keywords?), format_fragment(:ins, elems2, keywords?)]
 
         [{:eq, elems1}, {kind, elems2}] ->
-          [format_fragment(:eq, elems1), {kind, ", "}, format_fragment(kind, elems2)]
+          [format_fragment(:eq, elems1, keywords?), {kind, ", "}, format_fragment(kind, elems2, keywords?)]
 
         [{kind, elems1}, {:eq, elems2}] ->
-          [format_fragment(kind, elems1), {kind, ", "}, format_fragment(:eq, elems2)]
+          [format_fragment(kind, elems1, keywords?), {kind, ", "}, format_fragment(:eq, elems2, keywords?)]
       end
     Enum.reverse(acc, result)
   end
 
-  defp format_each_fragment([{:diff, script} | rest], acc) do
-    format_each_fragment(rest, [{:eq, ", "}, script | acc])
+  defp format_each_fragment([{:diff, script} | rest], acc, keywords?) do
+    format_each_fragment(rest, [{:eq, ", "}, script | acc], keywords?)
   end
 
-  defp format_each_fragment([{kind, elems} | rest], acc) do
-    new_acc = [{kind, ", "}, format_fragment(kind, elems) | acc]
-    format_each_fragment(rest, new_acc)
+  defp format_each_fragment([{kind, elems} | rest], acc, keywords?) do
+    new_acc = [{kind, ", "}, format_fragment(kind, elems, keywords?) | acc]
+    format_each_fragment(rest, new_acc, keywords?)
   end
 
-  defp format_fragment(kind, elems) do
-    formatter = fn {key, val} ->
-      format_key_value(key, val, true)
+  defp format_fragment(kind, elems, keywords?) do
+    formatter = fn
+      {key, val} when keywords? ->
+        format_key_value(key, val, true)
+      elem ->
+        inspect(elem)
     end
     {kind, Enum.map_join(elems, ", ", formatter)}
   end
 
-  defp find_script(envelope, max, _paths) when envelope > max do
+  defp find_script(envelope, max, _paths, _keywords?) when envelope > max do
     nil
   end
 
-  defp find_script(envelope, max, paths) do
-    case each_diagonal(-envelope, envelope, paths, []) do
+  defp find_script(envelope, max, paths, keywords?) do
+    case each_diagonal(-envelope, envelope, paths, [], keywords?) do
       {:done, edits} ->
         compact_reverse(edits, [])
-      {:next, paths} -> find_script(envelope + 1, max, paths)
+      {:next, paths} -> find_script(envelope + 1, max, paths, keywords?)
     end
   end
 
@@ -197,70 +214,73 @@ defmodule ExUnit.Diff do
   defp compact_reverse([{kind, char} | rest], acc),
     do: compact_reverse(rest, [{kind, [char]} | acc])
 
-  defp each_diagonal(diag, limit, _paths, next_paths) when diag > limit do
+  defp each_diagonal(diag, limit, _paths, next_paths, _keywords?) when diag > limit do
     {:next, Enum.reverse(next_paths)}
   end
 
-  defp each_diagonal(diag, limit, paths, next_paths) do
-    {path, rest} = proceed_path(diag, limit, paths)
+  defp each_diagonal(diag, limit, paths, next_paths, keywords?) do
+    {path, rest} = proceed_path(diag, limit, paths, keywords?)
     with {:cont, path} <- follow_snake(path) do
-      each_diagonal(diag + 2, limit, rest, [path | next_paths])
+      each_diagonal(diag + 2, limit, rest, [path | next_paths], keywords?)
     end
   end
 
-  defp proceed_path(0, 0, [path]), do: {path, []}
+  defp proceed_path(0, 0, [path], _keywords?), do: {path, []}
 
-  defp proceed_path(diag, limit, [path | _] = paths) when diag == -limit do
-    {move_down(path), paths}
+  defp proceed_path(diag, limit, [path | _] = paths, keywords?) when diag == -limit do
+    {move_down(path, keywords?), paths}
   end
 
-  defp proceed_path(diag, limit, [path]) when diag == limit do
-    {move_right(path), []}
+  defp proceed_path(diag, limit, [path], keywords?) when diag == limit do
+    {move_right(path, keywords?), []}
   end
 
-  defp proceed_path(_diag, _limit, [path1, path2 | rest]) do
+  defp proceed_path(_diag, _limit, [path1, path2 | rest], keywords?) do
     if elem(path1, 1) > elem(path2, 1) do
-      {move_right(path1), [path2 | rest]}
+      {move_right(path1, keywords?), [path2 | rest]}
     else
-      {move_down(path2), [path2 | rest]}
+      {move_down(path2, keywords?), [path2 | rest]}
     end
   end
 
-  defp script_keyword_inner({key, val1}, {key, val2}),
+  defp script_keyword_inner({key, val1}, {key, val2}, true),
     do: [{:eq, format_key(key, true)}, script_inner(val1, val2)]
 
-  defp script_keyword_inner(_pair1, _pair2),
+  defp script_keyword_inner(_pair1, _pair2, true),
     do: nil
 
-  defp move_right({x, x, [elem1 | rest1] = list1, [elem2 | rest2], edits}) do
-    if result = script_keyword_inner(elem1, elem2) do
+  defp script_keyword_inner(elem1, elem2, false),
+    do: script(elem1, elem2)
+
+  defp move_right({x, x, [elem1 | rest1] = list1, [elem2 | rest2], edits}, keywords?) do
+    if result = script_keyword_inner(elem1, elem2, keywords?) do
       {x + 1, x + 1, rest1, rest2, [{:diff, result} | edits]}
     else
       {x + 1, x, list1, rest2, [{:ins, elem2} | edits]}
     end
   end
 
-  defp move_right({x, y, list1, [elem | rest], edits}) do
+  defp move_right({x, y, list1, [elem | rest], edits}, _keywords?) do
     {x + 1, y, list1, rest, [{:ins, elem} | edits]}
   end
 
-  defp move_right({x, y, list1, [], edits}) do
+  defp move_right({x, y, list1, [], edits}, _keywords?) do
     {x + 1, y, list1, [], edits}
   end
 
-  defp move_down({x, x, [elem1 | rest1], [elem2 | rest2] = list2, edits}) do
-    if result = script_keyword_inner(elem1, elem2) do
+  defp move_down({x, x, [elem1 | rest1], [elem2 | rest2] = list2, edits}, keywords?) do
+    if result = script_keyword_inner(elem1, elem2, keywords?) do
       {x + 1, x + 1, rest1, rest2, [{:diff, result} | edits]}
     else
       {x, x + 1, rest1, list2, [{:del, elem1} | edits]}
     end
   end
 
-  defp move_down({x, y, [elem | rest], list2, edits}) do
+  defp move_down({x, y, [elem | rest], list2, edits}, _keywords?) do
     {x, y + 1, rest, list2, [{:del, elem} | edits]}
   end
 
-  defp move_down({x, y, [], list2, edits}) do
+  defp move_down({x, y, [], list2, edits}, _keywords?) do
     {x, y + 1, [], list2, edits}
   end
 

lib/ex_unit/test/ex_unit/diff_test.exs

@@ -37,21 +37,63 @@ defmodule ExUnit.DiffTest do
   end
 
   test "lists" do
-    list1 = ["One", :ok, nil, {}, :ok]
-    list2 = ["Two", :ok, 0.0, {true}]
+    list1 = ["Tvo", nil, :ok, {}, :ok]
+    list2 = ["Two", :ok, self(), {true}]
+
     expected = [
       {:eq, "["},
       [
-        [del: "\"One\"", ins: "\"Two\""],
-        [eq: ", ", eq: ":ok"],
-        [eq: ", ", del: "nil", ins: "0.0"],
-        [{:eq, ", "}, {:eq, "{"}, [[ins: "true"]], {:eq, "}"}],
-        [del: ", ", del: ":ok"]
+        [{:eq, "\""}, [eq: "T", del: "v", ins: "w", eq: "o"], {:eq, "\""}], {:eq, ", "},
+        {:del, "nil"}, {:del, ", "},
+        {:eq, ":ok"}, {:eq, ", "},
+        {:ins, inspect(self())}, {:ins, ", "},
+        [{:eq, "{"}, [[ins: "true"]], {:eq, "}"}], {:del, ", "}, {:del, ":ok"}
       ],
       {:eq, "]"}
     ]
     assert script(list1, list2) == expected
 
+    list1 = [1, "2", 1]
+    list2 = [1, 1, 2]
+    expected = [
+      {:eq, "["},
+      [eq: "1", eq: ", ", del: "\"2\"", del: ", ", eq: "1", ins: ", ", ins: "2"],
+      {:eq, "]"}
+    ]
+    assert script(list1, list2) == expected
+
+    list1 = [1, 1, "1", 2]
+    list2 = [1, 1, 2]
+    expected = [
+      {:eq, "["},
+      [eq: "1, 1", eq: ", ", del: "\"1\"", del: ", ", eq: "2"],
+      {:eq, "]"}
+    ]
+    assert script(list1, list2) == expected
+
+    list1 = [1, 2]
+    list2 = [1, 1, 2]
+    expected = [
+      {:eq, "["},
+      [{:eq, "1"}, {:eq, ", "}, [del: "2", ins: "1"], {:ins, ", "}, {:ins, "2"}],
+      {:eq, "]"}
+    ]
+    assert script(list1, list2) == expected
+
+    list1 = []
+    list2 = [1, 2]
+    expected = [{:eq, "["}, [ins: "1, 2"], {:eq, "]"}]
+    assert script(list1, list2) == expected
+
+    list1 = [1, 2]
+    list2 = []
+    expected = [{:eq, "["}, [del: "1, 2"], {:eq, "]"}]
+    assert script(list1, list2) == expected
+
+    assert script([], []) == [eq: "[]"]
+  end
+
+  test "charlists" do
     charlist1 = 'fox hops over \'the dog'
     charlist2 = 'fox jumps over the lazy cat'
     expected = [
@@ -60,8 +102,6 @@ defmodule ExUnit.DiffTest do
       {:eq, "'"}
     ]
     assert script(charlist1, charlist2) == expected
-
-    assert script([], []) == [eq: "[]"]
   end
 
   test "keyword lists" do