From bad391adfeb18ce25ac49ba68628e5e502b50813 Mon Sep 17 00:00:00 2001 From: dl-alexandre <166029845+dl-alexandre@users.noreply.github.com> Date: Fri, 8 May 2026 05:42:07 -0700 Subject: [PATCH] Support ignoring whitespace in diffs --- assets/css/app.css | 10 ++ lib/diff/hex/behaviour.ex | 2 + lib/diff/hex/hex.ex | 35 +++--- lib/diff/storage/gcs.ex | 32 +++--- lib/diff/storage/local.ex | 32 +++--- lib/diff/storage/storage.ex | 46 ++++++-- lib/diff_web/live/diff_live_view.ex | 55 +++++---- lib/diff_web/templates/live/diff.html.leex | 3 + lib/diff_web/views/live_view.ex | 13 ++- test/diff/storage_test.exs | 15 +++ test/diff_web/integration_test.exs | 12 +- test/diff_web/live/diff_live_view_test.exs | 123 +++++++++++++++++---- 12 files changed, 276 insertions(+), 102 deletions(-) create mode 100644 test/diff/storage_test.exs diff --git a/assets/css/app.css b/assets/css/app.css index c322644..e4f7c23 100644 --- a/assets/css/app.css +++ b/assets/css/app.css @@ -395,6 +395,16 @@ table.package-list .button { font-weight: 600; } +.diff-stats-header .whitespace-toggle { + color: #ddd; + font-family: 'SFMono-Regular', 'Consolas', 'Liberation Mono', 'Menlo', monospace; + font-size: 14px; +} + +.diff-stats-header .whitespace-toggle:hover { + color: white; +} + .loading-spinner { height: 60px; width: 100%; diff --git a/lib/diff/hex/behaviour.ex b/lib/diff/hex/behaviour.ex index 824bcc6..650d559 100644 --- a/lib/diff/hex/behaviour.ex +++ b/lib/diff/hex/behaviour.ex @@ -1,4 +1,6 @@ defmodule Diff.Hex.Behaviour do @callback diff(package :: String.t(), from :: String.t(), to :: String.t()) :: {:ok, Enumerable.t()} | :error + @callback diff(package :: String.t(), from :: String.t(), to :: String.t(), Keyword.t()) :: + {:ok, Enumerable.t()} | :error end diff --git a/lib/diff/hex/hex.ex b/lib/diff/hex/hex.ex index 2ec5dea..c793a6d 100644 --- a/lib/diff/hex/hex.ex +++ b/lib/diff/hex/hex.ex @@ -102,7 +102,9 @@ defmodule Diff.Hex do end end - def diff(package, from, to) do + def diff(package, from, to), do: diff(package, from, to, []) + + def diff(package, from, to, opts) do with {:ok, tarball_from} <- get_tarball(package, from), path_from = Diff.TmpDir.tmp_dir("package-#{package}-#{from}"), :ok <- unpack_tarball(tarball_from, path_from), @@ -131,7 +133,7 @@ defmodule Diff.Hex do with {_, true} <- {:file_size_old, file_size_check?(path_old)}, {_, true} <- {:file_size_new, file_size_check?(path_new)}, - {_, {:ok, output}} <- {:git_diff, git_diff(path_old, path_new)} do + {_, {:ok, output}} <- {:git_diff, git_diff(path_old, path_new, opts)} do if output do [{:ok, {output, path_from, path_to}}] else @@ -157,24 +159,29 @@ defmodule Diff.Hex do end end - defp git_diff(path_from, path_to) do - case System.cmd("git", [ - "-c", - "core.quotepath=false", - "-c", - "diff.algorithm=histogram", - "diff", - "--no-index", - "--no-color", - path_from, - path_to - ]) do + defp git_diff(path_from, path_to, opts) do + args = + [ + "-c", + "core.quotepath=false", + "-c", + "diff.algorithm=histogram", + "diff", + "--no-index", + "--no-color" + ] ++ ignore_whitespace_args(opts) ++ [path_from, path_to] + + case System.cmd("git", args) do {"", 0} -> {:ok, nil} {output, 1} -> {:ok, output} other -> {:error, other} end end + defp ignore_whitespace_args(opts) do + if Keyword.get(opts, :ignore_whitespace, false), do: ["-w"], else: [] + end + defp file_size_check?(path) do File.stat!(path).size <= @max_file_size end diff --git a/lib/diff/storage/gcs.ex b/lib/diff/storage/gcs.ex index ccb1d0e..ad654ce 100644 --- a/lib/diff/storage/gcs.ex +++ b/lib/diff/storage/gcs.ex @@ -5,8 +5,8 @@ defmodule Diff.Storage.GCS do @gs_xml_url "https://storage.googleapis.com" - def get_diff(package, from_version, to_version, diff_id) do - with {:ok, hash} <- combined_checksum(package, from_version, to_version), + def get_diff(package, from_version, to_version, diff_id, opts \\ []) do + with {:ok, hash} <- combined_checksum(package, from_version, to_version, opts), url = url(diff_key(package, from_version, to_version, hash, diff_id)), {:ok, 200, _headers, body} <- Diff.HTTP.retry("gs", fn -> Diff.HTTP.get(url, headers()) end) do @@ -25,8 +25,8 @@ defmodule Diff.Storage.GCS do end end - def put_diff(package, from_version, to_version, diff_id, diff_data) do - with {:ok, hash} <- combined_checksum(package, from_version, to_version), + def put_diff(package, from_version, to_version, diff_id, diff_data, opts \\ []) do + with {:ok, hash} <- combined_checksum(package, from_version, to_version, opts), url = url(diff_key(package, from_version, to_version, hash, diff_id)), {:ok, 200, _headers, _body} <- Diff.HTTP.retry("gs", fn -> Diff.HTTP.put(url, headers(), diff_data) end) do @@ -42,10 +42,10 @@ defmodule Diff.Storage.GCS do end end - def list_diffs(package, from_version, to_version) do - case get_metadata(package, from_version, to_version) do + def list_diffs(package, from_version, to_version, opts \\ []) do + case get_metadata(package, from_version, to_version, opts) do {:ok, %{total_diffs: total_diffs}} -> - diff_ids = 0..(total_diffs - 1) |> Enum.map(&"diff-#{&1}") + diff_ids = diff_ids(total_diffs) {:ok, diff_ids} {:error, :not_found} -> @@ -56,8 +56,8 @@ defmodule Diff.Storage.GCS do end end - def get_metadata(package, from_version, to_version) do - with {:ok, hash} <- combined_checksum(package, from_version, to_version), + def get_metadata(package, from_version, to_version, opts \\ []) do + with {:ok, hash} <- combined_checksum(package, from_version, to_version, opts), url = url(metadata_key(package, from_version, to_version, hash)), {:ok, 200, _headers, body} <- Diff.HTTP.retry("gs", fn -> Diff.HTTP.get(url, headers()) end) do @@ -79,8 +79,8 @@ defmodule Diff.Storage.GCS do end end - def put_metadata(package, from_version, to_version, metadata) do - with {:ok, hash} <- combined_checksum(package, from_version, to_version), + def put_metadata(package, from_version, to_version, metadata, opts \\ []) do + with {:ok, hash} <- combined_checksum(package, from_version, to_version, opts), url = url(metadata_key(package, from_version, to_version, hash)), {:ok, json} <- Jason.encode(metadata), {:ok, 200, _headers, _body} <- @@ -106,12 +106,18 @@ defmodule Diff.Storage.GCS do [{"authorization", "#{token.type} #{token.token}"}] end - def combined_checksum(package, from, to) do + def combined_checksum(package, from, to, opts \\ []) do with {:ok, checksums} <- Diff.Hex.get_checksums(package, [from, to]) do - {:ok, :erlang.phash2({Application.get_env(:diff, :cache_version), checksums})} + {:ok, :erlang.phash2(Diff.Storage.cache_key(checksums, opts))} end end + defp diff_ids(0), do: [] + + defp diff_ids(total_diffs) do + 0..(total_diffs - 1) |> Enum.map(&"diff-#{&1}") + end + defp diff_key(package, from_version, to_version, hash, diff_id) do "diffs/#{package}-#{from_version}-#{to_version}-#{hash}-#{diff_id}.json" end diff --git a/lib/diff/storage/local.ex b/lib/diff/storage/local.ex index 700f56c..2c20a61 100644 --- a/lib/diff/storage/local.ex +++ b/lib/diff/storage/local.ex @@ -3,8 +3,8 @@ defmodule Diff.Storage.Local do @behaviour Diff.Storage - def get_diff(package, from_version, to_version, diff_id) do - case combined_checksum(package, from_version, to_version) do + def get_diff(package, from_version, to_version, diff_id, opts \\ []) do + case combined_checksum(package, from_version, to_version, opts) do {:ok, hash} -> filename = diff_key(package, from_version, to_version, hash, diff_id) path = Path.join([dir(), package, filename]) @@ -20,8 +20,8 @@ defmodule Diff.Storage.Local do end end - def put_diff(package, from_version, to_version, diff_id, diff_data) do - with {:ok, hash} <- combined_checksum(package, from_version, to_version), + def put_diff(package, from_version, to_version, diff_id, diff_data, opts \\ []) do + with {:ok, hash} <- combined_checksum(package, from_version, to_version, opts), filename = diff_key(package, from_version, to_version, hash, diff_id), path = Path.join([dir(), package, filename]), :ok <- File.mkdir_p(Path.dirname(path)) do @@ -34,10 +34,10 @@ defmodule Diff.Storage.Local do end end - def list_diffs(package, from_version, to_version) do - case get_metadata(package, from_version, to_version) do + def list_diffs(package, from_version, to_version, opts \\ []) do + case get_metadata(package, from_version, to_version, opts) do {:ok, %{total_diffs: total_diffs}} -> - diff_ids = 0..(total_diffs - 1) |> Enum.map(&"diff-#{&1}") + diff_ids = diff_ids(total_diffs) {:ok, diff_ids} {:error, :not_found} -> @@ -48,8 +48,8 @@ defmodule Diff.Storage.Local do end end - def get_metadata(package, from_version, to_version) do - case combined_checksum(package, from_version, to_version) do + def get_metadata(package, from_version, to_version, opts \\ []) do + case combined_checksum(package, from_version, to_version, opts) do {:ok, hash} -> filename = metadata_key(package, from_version, to_version, hash) path = Path.join([dir(), package, filename]) @@ -74,8 +74,8 @@ defmodule Diff.Storage.Local do end end - def put_metadata(package, from_version, to_version, metadata) do - with {:ok, hash} <- combined_checksum(package, from_version, to_version), + def put_metadata(package, from_version, to_version, metadata, opts \\ []) do + with {:ok, hash} <- combined_checksum(package, from_version, to_version, opts), filename = metadata_key(package, from_version, to_version, hash), path = Path.join([dir(), package, filename]), :ok <- File.mkdir_p(Path.dirname(path)), @@ -89,12 +89,18 @@ defmodule Diff.Storage.Local do end end - def combined_checksum(package, from, to) do + def combined_checksum(package, from, to, opts \\ []) do with {:ok, checksums} <- Diff.Hex.get_checksums(package, [from, to]) do - {:ok, :erlang.phash2({Application.get_env(:diff, :cache_version), checksums})} + {:ok, :erlang.phash2(Diff.Storage.cache_key(checksums, opts))} end end + defp diff_ids(0), do: [] + + defp diff_ids(total_diffs) do + 0..(total_diffs - 1) |> Enum.map(&"diff-#{&1}") + end + defp diff_key(package, from_version, to_version, hash, diff_id) do "diffs/#{package}-#{from_version}-#{to_version}-#{hash}-#{diff_id}.json" end diff --git a/lib/diff/storage/storage.ex b/lib/diff/storage/storage.ex index 5915992..b1b0589 100644 --- a/lib/diff/storage/storage.ex +++ b/lib/diff/storage/storage.ex @@ -4,6 +4,7 @@ defmodule Diff.Storage do @type to_version :: String.t() @type diff_id :: String.t() @type diff_html :: String.t() + @type diff_options :: Keyword.t() @type diff_metadata :: %{ total_diffs: non_neg_integer(), total_additions: non_neg_integer(), @@ -14,34 +15,59 @@ defmodule Diff.Storage do # New diff-level storage callbacks @callback get_diff(package, from_version, to_version, diff_id) :: {:ok, diff_html} | {:error, term} + @callback get_diff(package, from_version, to_version, diff_id, diff_options) :: + {:ok, diff_html} | {:error, term} @callback put_diff(package, from_version, to_version, diff_id, diff_html) :: :ok | {:error, term} + @callback put_diff(package, from_version, to_version, diff_id, diff_html, diff_options) :: + :ok | {:error, term} @callback list_diffs(package, from_version, to_version) :: {:ok, [diff_id]} | {:error, term} + @callback list_diffs(package, from_version, to_version, diff_options) :: + {:ok, [diff_id]} | {:error, term} # Metadata storage callbacks @callback get_metadata(package, from_version, to_version) :: {:ok, diff_metadata} | {:error, term} + @callback get_metadata(package, from_version, to_version, diff_options) :: + {:ok, diff_metadata} | {:error, term} @callback put_metadata(package, from_version, to_version, diff_metadata) :: :ok | {:error, term} + @callback put_metadata(package, from_version, to_version, diff_metadata, diff_options) :: + :ok | {:error, term} defp impl(), do: Application.get_env(:diff, :storage_impl) - def get_diff(package, from_version, to_version, diff_id) do - impl().get_diff(package, from_version, to_version, diff_id) + def get_diff(package, from_version, to_version, diff_id, opts \\ []) do + impl().get_diff(package, from_version, to_version, diff_id, opts) end - def put_diff(package, from_version, to_version, diff_id, diff_html) do - impl().put_diff(package, from_version, to_version, diff_id, diff_html) + def put_diff(package, from_version, to_version, diff_id, diff_html, opts \\ []) do + impl().put_diff(package, from_version, to_version, diff_id, diff_html, opts) end - def list_diffs(package, from_version, to_version) do - impl().list_diffs(package, from_version, to_version) + def list_diffs(package, from_version, to_version, opts \\ []) do + impl().list_diffs(package, from_version, to_version, opts) end - def get_metadata(package, from_version, to_version) do - impl().get_metadata(package, from_version, to_version) + def get_metadata(package, from_version, to_version, opts \\ []) do + impl().get_metadata(package, from_version, to_version, opts) + end + + def put_metadata(package, from_version, to_version, metadata, opts \\ []) do + impl().put_metadata(package, from_version, to_version, metadata, opts) + end + + def cache_key(checksums, opts) do + base_key = {Application.get_env(:diff, :cache_version), checksums} + + case cache_options(opts) do + [] -> base_key + options -> {base_key, options} + end end - def put_metadata(package, from_version, to_version, metadata) do - impl().put_metadata(package, from_version, to_version, metadata) + defp cache_options(opts) do + opts + |> Keyword.take([:ignore_whitespace]) + |> Enum.reject(fn {_key, value} -> value in [false, nil] end) end end diff --git a/lib/diff_web/live/diff_live_view.ex b/lib/diff_web/live/diff_live_view.ex index 9ffebe9..936783e 100644 --- a/lib/diff_web/live/diff_live_view.ex +++ b/lib/diff_web/live/diff_live_view.ex @@ -8,12 +8,14 @@ defmodule DiffWeb.DiffLiveView do end # Mount for single diff view - def mount(%{"package" => package, "versions" => versions}, _session, socket) do + def mount(%{"package" => package, "versions" => versions} = params, _session, socket) do + diff_options = diff_options(params) + case parse_versions(versions) do {:ok, from, to} -> case resolve_latest_version(package, from, to) do {:ok, resolved_from, resolved_to} -> - mount_single_diff(socket, package, resolved_from, resolved_to) + mount_single_diff(socket, package, resolved_from, resolved_to, diff_options) {:error, reason} -> {:ok, assign(socket, error: latest_version_error(package, reason))} @@ -53,13 +55,13 @@ defmodule DiffWeb.DiffLiveView do end end - defp mount_single_diff(socket, package, from, to) do - case Diff.Storage.get_metadata(package, from, to) do + defp mount_single_diff(socket, package, from, to, diff_options) do + case Diff.Storage.get_metadata(package, from, to, diff_options) do {:ok, metadata} -> - load_existing_diff(socket, package, from, to, metadata) + load_existing_diff(socket, package, from, to, metadata, diff_options) {:error, :not_found} -> - generate_new_diff(socket, package, from, to) + generate_new_diff(socket, package, from, to, diff_options) {:error, reason} -> Logger.error("Failed to load diff metadata: #{inspect(reason)}") @@ -67,8 +69,8 @@ defmodule DiffWeb.DiffLiveView do end end - defp load_existing_diff(socket, package, from, to, metadata) do - {:ok, diff_ids} = Diff.Storage.list_diffs(package, from, to) + defp load_existing_diff(socket, package, from, to, metadata, diff_options) do + {:ok, diff_ids} = Diff.Storage.list_diffs(package, from, to, diff_options) initial_batch_size = 5 {initial_diffs, _remaining} = Enum.split(diff_ids, initial_batch_size) @@ -80,6 +82,8 @@ defmodule DiffWeb.DiffLiveView do from: from, to: to, metadata: metadata, + diff_options: diff_options, + ignore_whitespace: ignore_whitespace?(diff_options), all_diff_ids: diff_ids, loaded_diffs: [], loaded_diff_content: %{}, @@ -93,7 +97,7 @@ defmodule DiffWeb.DiffLiveView do {:ok, socket} end - defp generate_new_diff(socket, package, from, to) do + defp generate_new_diff(socket, package, from, to, diff_options) do socket = assign(socket, view_mode: :single_diff, @@ -101,6 +105,8 @@ defmodule DiffWeb.DiffLiveView do from: from, to: to, metadata: %{files_changed: 0, total_additions: 0, total_deletions: 0}, + diff_options: diff_options, + ignore_whitespace: ignore_whitespace?(diff_options), all_diff_ids: [], loaded_diffs: [], loaded_diff_content: %{}, @@ -109,7 +115,7 @@ defmodule DiffWeb.DiffLiveView do has_more_diffs: false ) - send(self(), {:generate_diff, package, from, to}) + send(self(), {:generate_diff, package, from, to, diff_options}) {:ok, socket} end @@ -159,14 +165,14 @@ defmodule DiffWeb.DiffLiveView do {:noreply, socket} end - def handle_info({:generate_diff, package, from, to}, socket) do + def handle_info({:generate_diff, package, from, to, diff_options}, socket) do hex_impl = Application.get_env(:diff, :hex_impl, Diff.Hex) task = Task.Supervisor.async(Diff.Tasks, fn -> - case hex_impl.diff(package, from, to) do + case hex_impl.diff(package, from, to, diff_options) do {:ok, stream} -> - process_stream_to_diffs(package, from, to, stream) + process_stream_to_diffs(package, from, to, stream, diff_options) :error -> :error @@ -220,6 +226,7 @@ defmodule DiffWeb.DiffLiveView do socket.assigns.package, socket.assigns.from, socket.assigns.to, + socket.assigns.diff_options, diff_ids ) @@ -252,6 +259,7 @@ defmodule DiffWeb.DiffLiveView do socket.assigns.package, socket.assigns.from, socket.assigns.to, + socket.assigns.diff_options, diff_ids ) @@ -273,7 +281,7 @@ defmodule DiffWeb.DiffLiveView do {:noreply, socket} end - defp process_stream_to_diffs(package, from, to, stream) do + defp process_stream_to_diffs(package, from, to, stream, diff_options) do initial_metadata = %{ total_diffs: 0, total_additions: 0, @@ -289,7 +297,7 @@ defmodule DiffWeb.DiffLiveView do Diff.Tasks, indexed_stream, fn {element, index} -> - process_stream_element(package, from, to, element, index) + process_stream_element(package, from, to, element, index, diff_options) end, max_concurrency: 10, timeout: 30_000, @@ -309,7 +317,7 @@ defmodule DiffWeb.DiffLiveView do case results do {final_metadata, diff_ids} -> - case Diff.Storage.put_metadata(package, from, to, final_metadata) do + case Diff.Storage.put_metadata(package, from, to, final_metadata, diff_options) do :ok -> {:ok, final_metadata, diff_ids} @@ -323,7 +331,7 @@ defmodule DiffWeb.DiffLiveView do {:error, :invalid_diff} end - defp process_stream_element(package, from, to, element, index) do + defp process_stream_element(package, from, to, element, index, diff_options) do case element do {:ok, {raw_diff, path_from, path_to}} -> diff_id = "diff-#{index}" @@ -335,7 +343,7 @@ defmodule DiffWeb.DiffLiveView do "path_to" => path_to }) - case Diff.Storage.put_diff(package, from, to, diff_id, diff_data) do + case Diff.Storage.put_diff(package, from, to, diff_id, diff_data, diff_options) do :ok -> # Count additions and deletions from raw diff (exclude +++ and --- headers) lines = String.split(raw_diff, "\n") @@ -368,7 +376,7 @@ defmodule DiffWeb.DiffLiveView do diff_id = "diff-#{index}" too_large_data = Jason.encode!(%{type: "too_large", file: file_path}) - case Diff.Storage.put_diff(package, from, to, diff_id, too_large_data) do + case Diff.Storage.put_diff(package, from, to, diff_id, too_large_data, diff_options) do :ok -> metadata_update = %{ total_diffs: 1, @@ -436,6 +444,11 @@ defmodule DiffWeb.DiffLiveView do defp parse_version(""), do: {:ok, :latest} defp parse_version(input), do: Version.parse(input) + defp diff_options(%{"w" => "1"}), do: [ignore_whitespace: true] + defp diff_options(_params), do: [] + + defp ignore_whitespace?(diff_options), do: Keyword.get(diff_options, :ignore_whitespace, false) + defp parse_diff(diff) do case String.split(diff, ":", trim: true) do [app, from, to] -> {app, from, to, build_url(app, from, to)} @@ -445,12 +458,12 @@ defmodule DiffWeb.DiffLiveView do defp build_url(app, from, to), do: "/diff/#{app}/#{from}..#{to}" - defp load_diffs_in_parallel(package, from, to, diff_ids) do + defp load_diffs_in_parallel(package, from, to, diff_options, diff_ids) do Task.Supervisor.async_stream( Diff.Tasks, diff_ids, fn diff_id -> - {diff_id, DiffWeb.LiveView.load_diff_content(package, from, to, diff_id)} + {diff_id, DiffWeb.LiveView.load_diff_content(package, from, to, diff_id, diff_options)} end, max_concurrency: 10, timeout: 30_000 diff --git a/lib/diff_web/templates/live/diff.html.leex b/lib/diff_web/templates/live/diff.html.leex index 6dfd12c..0ed1c92 100644 --- a/lib/diff_web/templates/live/diff.html.leex +++ b/lib/diff_web/templates/live/diff.html.leex @@ -24,6 +24,9 @@ <%= @metadata.files_changed %> files changed +<%= @metadata.total_additions %> -<%= @metadata.total_deletions %> + + <%= whitespace_toggle_label(@ignore_whitespace) %> + <% end %> diff --git a/lib/diff_web/views/live_view.ex b/lib/diff_web/views/live_view.ex index df86d8e..9cb0460 100644 --- a/lib/diff_web/views/live_view.ex +++ b/lib/diff_web/views/live_view.ex @@ -2,8 +2,8 @@ defmodule DiffWeb.LiveView do use DiffWeb, :view require Logger - def load_diff_content(package, from, to, diff_id) do - case Diff.Storage.get_diff(package, from, to, diff_id) do + def load_diff_content(package, from, to, diff_id, opts \\ []) do + case Diff.Storage.get_diff(package, from, to, diff_id, opts) do {:ok, raw_content} -> # Parse the stored content based on format case Jason.decode(raw_content) do @@ -49,4 +49,13 @@ defmodule DiffWeb.LiveView do end) |> Enum.join("") end + + def whitespace_toggle_label(true), do: "Show whitespace changes" + def whitespace_toggle_label(false), do: "Hide whitespace changes" + + def whitespace_toggle_url(package, from, to, true), do: diff_url(package, from, to) + + def whitespace_toggle_url(package, from, to, false), do: diff_url(package, from, to) <> "?w=1" + + defp diff_url(package, from, to), do: "/diff/#{package}/#{from}..#{to}" end diff --git a/test/diff/storage_test.exs b/test/diff/storage_test.exs new file mode 100644 index 0000000..9c50cf9 --- /dev/null +++ b/test/diff/storage_test.exs @@ -0,0 +1,15 @@ +defmodule Diff.StorageTest do + use ExUnit.Case + + test "cache key keeps default format and separates ignore whitespace diffs" do + original_cache_version = Application.get_env(:diff, :cache_version) + Application.put_env(:diff, :cache_version, 2) + + on_exit(fn -> Application.put_env(:diff, :cache_version, original_cache_version) end) + + checksums = ["from-checksum", "to-checksum"] + + assert Diff.Storage.cache_key(checksums, []) == {2, checksums} + assert Diff.Storage.cache_key(checksums, ignore_whitespace: true) != {2, checksums} + end +end diff --git a/test/diff_web/integration_test.exs b/test/diff_web/integration_test.exs index 0bd265d..7cfe69d 100644 --- a/test/diff_web/integration_test.exs +++ b/test/diff_web/integration_test.exs @@ -27,12 +27,12 @@ defmodule DiffWeb.IntegrationTest do |> stub(:get_versions, fn "phoenix" -> {:ok, versions} end) Diff.StorageMock - |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9" -> + |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9", [] -> {:error, :not_found} end) Diff.HexMock - |> stub(:diff, fn "phoenix", "1.4.5", "1.4.9" -> :error end) + |> stub(:diff, fn "phoenix", "1.4.5", "1.4.9", [] -> :error end) {:ok, _view, html} = live(conn, "/diff/phoenix/1.4.5..") @@ -47,12 +47,12 @@ defmodule DiffWeb.IntegrationTest do end) Diff.StorageMock - |> stub(:get_metadata, fn "prerelease_only", "0.1.0", "1.0.0-rc.2" -> + |> stub(:get_metadata, fn "prerelease_only", "0.1.0", "1.0.0-rc.2", [] -> {:error, :not_found} end) Diff.HexMock - |> stub(:diff, fn "prerelease_only", "0.1.0", "1.0.0-rc.2" -> :error end) + |> stub(:diff, fn "prerelease_only", "0.1.0", "1.0.0-rc.2", [] -> :error end) {:ok, _view, html} = live(conn, "/diff/prerelease_only/0.1.0..") @@ -73,12 +73,12 @@ defmodule DiffWeb.IntegrationTest do |> stub(:get_versions, fn "nonexistent" -> {:error, :not_found} end) Diff.StorageMock - |> stub(:get_metadata, fn "nonexistent", "1.0.0", "2.0.0" -> + |> stub(:get_metadata, fn "nonexistent", "1.0.0", "2.0.0", [] -> {:error, :not_found} end) Diff.HexMock - |> stub(:diff, fn "nonexistent", "1.0.0", "2.0.0" -> :error end) + |> stub(:diff, fn "nonexistent", "1.0.0", "2.0.0", [] -> :error end) {:ok, _view, html} = live(conn, "/diff/nonexistent/1.0.0..2.0.0") diff --git a/test/diff_web/live/diff_live_view_test.exs b/test/diff_web/live/diff_live_view_test.exs index 546a079..fb7837a 100644 --- a/test/diff_web/live/diff_live_view_test.exs +++ b/test/diff_web/live/diff_live_view_test.exs @@ -27,13 +27,13 @@ defmodule DiffWeb.DiffLiveViewTest do }) Diff.StorageMock - |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9" -> + |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9", [] -> {:ok, metadata} end) - |> stub(:list_diffs, fn "phoenix", "1.4.5", "1.4.9" -> + |> stub(:list_diffs, fn "phoenix", "1.4.5", "1.4.9", [] -> {:ok, diffs_ids} end) - |> stub(:get_diff, fn "phoenix", "1.4.5", "1.4.9", _diff_id -> + |> stub(:get_diff, fn "phoenix", "1.4.5", "1.4.9", _diff_id, [] -> {:ok, diff_content} end) @@ -46,16 +46,41 @@ defmodule DiffWeb.DiffLiveViewTest do assert html =~ "8" assert html =~ "+45" assert html =~ "-12" + assert html =~ "Hide whitespace changes" + assert html =~ ~s(href="/diff/phoenix/1.4.5..1.4.9?w=1") + end + + test "mounts with ignore whitespace query option", %{conn: conn} do + metadata = %{ + total_diffs: 0, + total_additions: 0, + total_deletions: 0, + files_changed: 0 + } + + Diff.StorageMock + |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9", ignore_whitespace: true -> + {:ok, metadata} + end) + |> stub(:list_diffs, fn "phoenix", "1.4.5", "1.4.9", ignore_whitespace: true -> + {:ok, []} + end) + + {:ok, _view, html} = live(conn, "/diff/phoenix/1.4.5..1.4.9?w=1") + + assert html =~ "0 files changed" + assert html =~ "Show whitespace changes" + assert html =~ ~s(href="/diff/phoenix/1.4.5..1.4.9") end test "shows generating state when metadata not found", %{conn: conn} do Diff.StorageMock - |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9" -> + |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9", [] -> {:error, :not_found} end) Diff.HexMock - |> stub(:diff, fn "phoenix", "1.4.5", "1.4.9" -> :error end) + |> stub(:diff, fn "phoenix", "1.4.5", "1.4.9", [] -> :error end) {:ok, _view, html} = live(conn, "/diff/phoenix/1.4.5..1.4.9") @@ -84,23 +109,23 @@ defmodule DiffWeb.DiffLiveViewTest do # Mock Diff.Hex to return our test stream Diff.HexMock - |> expect(:diff, fn "phoenix", "1.4.5", "1.4.9" -> + |> expect(:diff, fn "phoenix", "1.4.5", "1.4.9", [] -> {:ok, mock_stream} end) # Mock storage operations for parallel processing Diff.StorageMock - |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9" -> + |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9", [] -> {:error, :not_found} end) - |> expect(:put_diff, 3, fn "phoenix", "1.4.5", "1.4.9", diff_id, data -> + |> expect(:put_diff, 3, fn "phoenix", "1.4.5", "1.4.9", diff_id, data, [] -> # Verify diff data structure assert diff_id =~ ~r/diff-\d+/ decoded = Jason.decode!(data) assert is_map(decoded) :ok end) - |> expect(:put_metadata, fn "phoenix", "1.4.5", "1.4.9", metadata -> + |> expect(:put_metadata, fn "phoenix", "1.4.5", "1.4.9", metadata, [] -> # Verify aggregated metadata from parallel processing assert metadata.total_diffs == 3 assert metadata.files_changed == 3 @@ -108,10 +133,10 @@ defmodule DiffWeb.DiffLiveViewTest do assert metadata.total_deletions > 0 :ok end) - |> stub(:list_diffs, fn "phoenix", "1.4.5", "1.4.9" -> + |> stub(:list_diffs, fn "phoenix", "1.4.5", "1.4.9", [] -> {:ok, ["diff-0", "diff-1", "diff-2"]} end) - |> stub(:get_diff, fn "phoenix", "1.4.5", "1.4.9", _diff_id -> + |> stub(:get_diff, fn "phoenix", "1.4.5", "1.4.9", _diff_id, [] -> {:ok, Jason.encode!(%{ "diff" => "test diff", @@ -145,29 +170,29 @@ defmodule DiffWeb.DiffLiveViewTest do ] Diff.HexMock - |> expect(:diff, fn "phoenix", "1.4.5", "1.4.9" -> + |> expect(:diff, fn "phoenix", "1.4.5", "1.4.9", [] -> {:ok, mock_stream} end) # Mock storage - only successful elements should be stored Diff.StorageMock - |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9" -> + |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9", [] -> {:error, :not_found} end) - |> expect(:put_diff, 2, fn "phoenix", "1.4.5", "1.4.9", _diff_id, _data -> + |> expect(:put_diff, 2, fn "phoenix", "1.4.5", "1.4.9", _diff_id, _data, [] -> :ok end) - |> expect(:put_metadata, fn "phoenix", "1.4.5", "1.4.9", metadata -> + |> expect(:put_metadata, fn "phoenix", "1.4.5", "1.4.9", metadata, [] -> # Only 2 diffs should be stored (error one skipped) assert metadata.total_diffs == 2 assert metadata.files_changed == 2 :ok end) - |> stub(:list_diffs, fn "phoenix", "1.4.5", "1.4.9" -> + |> stub(:list_diffs, fn "phoenix", "1.4.5", "1.4.9", [] -> # Skip error element {:ok, ["diff-0", "diff-2"]} end) - |> stub(:get_diff, fn "phoenix", "1.4.5", "1.4.9", _diff_id -> + |> stub(:get_diff, fn "phoenix", "1.4.5", "1.4.9", _diff_id, [] -> {:ok, Jason.encode!(%{ "diff" => "test diff", @@ -190,12 +215,12 @@ defmodule DiffWeb.DiffLiveViewTest do test "handles hex diff failure", %{conn: conn} do Diff.HexMock - |> expect(:diff, fn "phoenix", "1.4.5", "1.4.9" -> + |> expect(:diff, fn "phoenix", "1.4.5", "1.4.9", [] -> :error end) Diff.StorageMock - |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9" -> + |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9", [] -> {:error, :not_found} end) @@ -213,18 +238,18 @@ defmodule DiffWeb.DiffLiveViewTest do ] Diff.HexMock - |> expect(:diff, fn "phoenix", "1.4.5", "1.4.9" -> + |> expect(:diff, fn "phoenix", "1.4.5", "1.4.9", [] -> {:ok, mock_stream} end) Diff.StorageMock - |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9" -> + |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9", [] -> {:error, :not_found} end) - |> expect(:put_diff, fn "phoenix", "1.4.5", "1.4.9", _diff_id, _data -> + |> expect(:put_diff, fn "phoenix", "1.4.5", "1.4.9", _diff_id, _data, [] -> {:error, :storage_failed} end) - |> expect(:put_metadata, fn "phoenix", "1.4.5", "1.4.9", metadata -> + |> expect(:put_metadata, fn "phoenix", "1.4.5", "1.4.9", metadata, [] -> # Should still try to store metadata even with failed individual diffs # No successful diffs assert metadata.total_diffs == 0 @@ -241,6 +266,58 @@ defmodule DiffWeb.DiffLiveViewTest do refute final_html =~ "Failed to generate diff" end) end + + test "passes ignore whitespace option while generating diffs", %{conn: conn} do + mock_stream = [ + {:ok, + {"diff --git a/lib/app.ex b/lib/app.ex\n--- a/lib/app.ex\n+++ b/lib/app.ex\n@@ -1 +1 @@\n-old\n+new", + "/tmp/from", "/tmp/to"}} + ] + + Diff.HexMock + |> expect(:diff, fn "phoenix", "1.4.5", "1.4.9", ignore_whitespace: true -> + {:ok, mock_stream} + end) + + Diff.StorageMock + |> stub(:get_metadata, fn "phoenix", "1.4.5", "1.4.9", ignore_whitespace: true -> + {:error, :not_found} + end) + |> expect(:put_diff, fn "phoenix", + "1.4.5", + "1.4.9", + "diff-0", + _data, + ignore_whitespace: true -> + :ok + end) + |> expect(:put_metadata, fn "phoenix", + "1.4.5", + "1.4.9", + metadata, + ignore_whitespace: true -> + assert metadata.total_diffs == 1 + :ok + end) + |> stub(:get_diff, fn "phoenix", "1.4.5", "1.4.9", "diff-0", ignore_whitespace: true -> + {:ok, + Jason.encode!(%{ + "diff" => "test diff", + "path_from" => "/tmp/from", + "path_to" => "/tmp/to" + })} + end) + + capture_log(fn -> + {:ok, view, _html} = live(conn, "/diff/phoenix/1.4.5..1.4.9?w=1") + + :timer.sleep(100) + final_html = render(view) + + assert final_html =~ "1 files changed" + assert final_html =~ "Show whitespace changes" + end) + end end describe "DiffLiveView diffs list" do