fix: config sorting shifts comments when no sorting occurs

lib/style/configs.ex

@@ -49,32 +49,26 @@ defmodule Styler.Style.Configs do
 
   def run({{:config, _, [_, _ | _]} = config, zm}, %{mix_config?: true} = ctx) do
     # all of these list are reversed due to the reduce
-    {configs, assignments, rest} =
-      Enum.reduce(zm.r, {[], [], []}, fn
-        {:config, _, [_, _ | _]} = config, {configs, assignments, []} -> {[config | configs], assignments, []}
-        {:=, _, [_lhs, _rhs]} = assignment, {configs, assignments, []} -> {configs, [assignment | assignments], []}
-        other, {configs, assignments, rest} -> {configs, assignments, [other | rest]}
-      end)
+    {configs, assignments, rest} = accumulate(zm.r, [], [])
+
+    left_siblings = assignments |> Enum.reverse() |> Style.reset_newlines() |> Enum.reverse(zm.l)
 
-    [config | configs] =
+    [config | left_siblings] =
       [config | configs]
       |> Enum.group_by(fn
         {:config, _, [{:__block__, _, [app]} | _]} -> app
         {:config, _, [arg | _]} -> Style.without_meta(arg)
       end)
-      |> Enum.sort(:desc)
+      |> Enum.sort()
       |> Enum.flat_map(fn {_app, configs} ->
         configs
-        |> Enum.sort_by(&Style.without_meta/1, :asc)
+        |> Enum.sort_by(&Style.without_meta/1)
         |> Style.reset_newlines()
-        |> Enum.reverse()
       end)
       |> Style.fix_line_numbers(List.first(rest))
+      |> Enum.reverse(left_siblings)
 
-    assignments = assignments |> Enum.reverse() |> Style.reset_newlines()
-
-    zm = %{zm | l: configs ++ Enum.reverse(assignments, zm.l), r: Enum.reverse(rest)}
-    {:skip, {config, zm}, ctx}
+    {:skip, {config, %{zm | l: left_siblings, r: rest}}, ctx}
   end
 
   def run(zipper, %{config?: true} = ctx) do
@@ -88,4 +82,8 @@ defmodule Styler.Style.Configs do
       {:halt, zipper, ctx}
     end
   end
+
+  defp accumulate([{:config, _, [_, _ | _]} = c | siblings], cs, as), do: accumulate(siblings, [c | cs], as)
+  defp accumulate([{:=, _, [_lhs, _rhs]} = a | siblings], cs, as), do: accumulate(siblings, cs, [a | as])
+  defp accumulate(rest, configs, assignments), do: {configs, assignments, rest}
 end

test/style/configs_test.exs

@@ -27,44 +27,53 @@ defmodule Styler.Style.ConfigsTest do
     end
   end
 
-  test "orders configs stanzas" do
-    # doesn't order when we haven't seen `import Config`, so this is something else that we don't understand
+  test "doesn't sort when no import config" do
     assert_style """
-    config :z, :x
-    config :a, :b
+    config :z, :x, :c
+    config :a, :b, :c
     """
+  end
+
+  test "simple case" do
+    assert_style(
+      """
+      import Config
+
+      config :z, :x, :c
+      config :a, :b, :c
+      config :y, :x, :z
+      config :a, :c, :d
+      """,
+      """
+      import Config
+
+      config :a, :b, :c
+      config :a, :c, :d
+
+      config :y, :x, :z
 
-    # 1. orders `config/2,3` relative to each other
-    # 2. lifts assignments above config blocks
-    # 3. non assignment/config separate "config" blocks
+      config :z, :x, :c
+      """
+    )
+  end
 
+  test "more complicated" do
     assert_style(
       """
       import Config
       dog_sound = :woof
-      # z is best when configged w/ dog sounds
-      # dog sounds ftw
       config :z, :x, dog_sound
 
-      # this is my big c
-      # comment i'd like to leave c
-      # about c
       c = :c
       config :a, :b, c
       config :a, :c, :d
       config :a,
         a_longer_name: :a_longer_value,
         multiple_things: :that_could_all_fit_on_one_line_though
 
-      # this is my big my_app
-      # comment i'd like to leave my_app
-      # about my_app
       my_app =
         :"dont_write_configs_like_this_yall_:("
 
-      # this is my big your_app
-      # comment i'd like to leave your_app
-      # about your_app
       your_app = :not_again!
       config your_app, :dont_use_varrrrrrrrs
       config my_app, :nooooooooo
@@ -79,23 +88,11 @@ defmodule Styler.Style.ConfigsTest do
       import Config
 
       dog_sound = :woof
-      # z is best when configged w/ dog sounds
-      # dog sounds ftw
-
-      # this is my big c
-      # comment i'd like to leave c
-      # about c
       c = :c
 
-      # this is my big my_app
-      # comment i'd like to leave my_app
-      # about my_app
       my_app =
         :"dont_write_configs_like_this_yall_:("
 
-      # this is my big your_app
-      # comment i'd like to leave your_app
-      # about your_app
       your_app = :not_again!
 
       config :a, :b, c
@@ -133,4 +130,109 @@ defmodule Styler.Style.ConfigsTest do
     config :c, :d
     """
   end
+
+  describe "playing nice with comments" do
+    test "lets you leave comments in large stanzas" do
+      assert_style """
+      import Config
+
+      config :a, B, :c
+
+      config :a,
+        b: :c,
+        # d is here
+        d: :e
+      """
+    end
+
+    test "complicated comments" do
+      assert_style(
+        """
+        import Config
+        dog_sound = :woof
+        # z is best when configged w/ dog sounds
+        # dog sounds ftw
+        config :z, :x, dog_sound
+
+        # this is my big c
+        # comment i'd like to leave c
+        # about c
+        c = :c
+        config :a, :b, c
+        config :a, :c, :d
+        config :a,
+          a_longer_name: :a_longer_value,
+          # Multiline comment
+          # comment in a block
+          multiple_things: :that_could_all_fit_on_one_line_though
+
+        # this is my big my_app
+        # comment i'd like to leave my_app
+        # about my_app
+        my_app =
+          :"dont_write_configs_like_this_yall_:("
+
+        # this is my big your_app
+        # comment i'd like to leave your_app
+        # about your_app
+        your_app = :not_again!
+        config your_app, :dont_use_varrrrrrrrs
+        config my_app, :nooooooooo
+        import_config "my_config"
+
+        cat_sound = :meow
+        config :z, a: :meow
+        a_sad_overwrite_that_will_be_hard_to_notice = :x
+        config :a, :b, a_sad_overwrite_that_will_be_hard_to_notice
+        """,
+        """
+        import Config
+
+        dog_sound = :woof
+        # z is best when configged w/ dog sounds
+        # dog sounds ftw
+
+        # this is my big c
+        # comment i'd like to leave c
+        # about c
+        c = :c
+        # Multiline comment
+        # comment in a block
+
+        # this is my big my_app
+        # comment i'd like to leave my_app
+        # about my_app
+        my_app =
+          :"dont_write_configs_like_this_yall_:("
+
+        # this is my big your_app
+        # comment i'd like to leave your_app
+        # about your_app
+        your_app = :not_again!
+
+        config :a, :b, c
+        config :a, :c, :d
+
+        config :a,
+          a_longer_name: :a_longer_value,
+          multiple_things: :that_could_all_fit_on_one_line_though
+
+        config :z, :x, dog_sound
+
+        config my_app, :nooooooooo
+
+        config your_app, :dont_use_varrrrrrrrs
+
+        import_config "my_config"
+
+        cat_sound = :meow
+        a_sad_overwrite_that_will_be_hard_to_notice = :x
+
+        config :a, :b, a_sad_overwrite_that_will_be_hard_to_notice
+
+        config :z, a: :meow
+        """
+      )
+    end
+  end
 end

test/support/style_case.ex

@@ -38,13 +38,11 @@ defmodule Styler.StyleCase do
         {before_ast, before_comments} = Styler.string_to_quoted_with_comments(before)
         dbg(before_ast)
         dbg(before_comments)
-        IO.puts("======Expected==========\n")
-        IO.puts(expected)
+        IO.puts("======Expected AST==========\n")
         {expected_ast, expected_comments} = Styler.string_to_quoted_with_comments(expected)
         dbg(expected_ast)
         dbg(expected_comments)
-        IO.puts("======Got===============\n")
-        IO.puts(styled)
+        IO.puts("======Got AST===============\n")
         dbg(styled_ast)
         dbg(styled_comments)
         IO.puts("========================\n")
@@ -111,7 +109,7 @@ defmodule Styler.StyleCase do
         assert true
       else
         flunk(
-          format_diff(restyled, styled, "expected styling to be idempotent, but a second pass resulted in more changes.")
+          format_diff(styled, restyled, "expected styling to be idempotent, but a second pass resulted in more changes.")
         )
       end
     end
@@ -131,32 +129,49 @@ defmodule Styler.StyleCase do
     end
   end
 
-  def format_diff(left, right, prelude \\ "Styling produced unexpected results") do
+  def format_diff(expected, styled, prelude \\ "Styling produced unexpected results") do
     # reaching into private ExUnit stuff, uh oh!
     # this gets us the nice diffing from ExUnit while allowing us to print our code blocks as strings rather than inspected strings
-    {%{left: left, right: right}, _} = ExUnit.Diff.compute(left, right, :==)
-    left = for {diff?, content} <- left.contents, do: if(diff?, do: [:red, content, :reset], else: content)
-    right = for {diff?, content} <- right.contents, do: if(diff?, do: [:green, content, :reset], else: content)
+    {%{left: expected, right: styled}, _} = ExUnit.Diff.compute(expected, styled, :==)
+
+    expected =
+      for {diff?, content} <- expected.contents do
+        cond do
+          diff? and String.trim_leading(Macro.unescape_string(content)) == "" -> [:red_background, content, :reset]
+          diff? -> [:red, content, :reset]
+          true -> content
+        end
+      end
+
+    styled =
+      for {diff?, content} <- styled.contents do
+        cond do
+          diff? and String.trim_leading(Macro.unescape_string(content)) == "" -> [:green_background, content, :reset]
+          diff? -> [:green, content, :reset]
+          true -> content
+        end
+      end
+
     header = IO.ANSI.format([:red, prelude, :reset])
 
-    left =
-      [[:cyan, "left:\n", :reset] | left]
+    expected =
+      [[:cyan, "expected:\n", :reset] | expected]
       |> IO.ANSI.format()
       |> to_string()
       |> Macro.unescape_string()
       |> String.replace("\n", "\n  ")
 
-    right =
-      [[:cyan, "right:\n", :reset] | right]
+    styled =
+      [[:cyan, "styled:\n", :reset] | styled]
       |> IO.ANSI.format()
       |> to_string()
       |> Macro.unescape_string()
       |> String.replace("\n", "\n  ")
 
     """
     #{header}
-    #{left}
-    #{right}
+    #{expected}
+    #{styled}
     """
   end
 end