diff --git a/assets/css/app.css b/assets/css/app.css index a485f3f..f6263ee 100644 --- a/assets/css/app.css +++ b/assets/css/app.css @@ -198,8 +198,29 @@ input.search-input:focus { display: none; } +.ghd-chunk-header { + height: 44px; +} +.ghd-chunk-header:first-child, .ghd-chunk-header:last-child { + height: 0; +} + .ghd-chunk-header .ghd-line-number { background-color: #f8fafd; + flex-direction: column; +} + +.expanded, .expanded .ghd-line-number{ + background-color: #f5f5f5; +} + +.ghd-chunk-header .ghd-line-number div{ + background: #dbedff; + width: 100%; + text-align: center; +} +.ghd-chunk-header .ghd-line-number div:hover{ + background: #95c6fe; } .ghd-file-status { diff --git a/assets/js/app.js b/assets/js/app.js index 71c339f..b6abd99 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -57,3 +57,76 @@ fileHeaders.forEach(header => { const scrollIfNeeded = elem => { elem.getBoundingClientRect().top < 0 && elem.scrollIntoView(true) } + +const MAX_EXPAND_CONTEXT_LINES = 20 + +document.addEventListener('DOMContentLoaded', e => { + document.querySelectorAll('.ghd-expand-up').forEach(expandUp => { + expandUp.addEventListener('click', e => { + const {fileName, packageName, version, lineAfter, lineBefore, patch} = gatherInfo(expandUp) + + // expandUp always follows by diff line.. so we take the number + const toLine = numberFrom(lineAfter) - 1 + + const _fromLine = lineBefore ? numberFrom(lineBefore) + 1 : 1 + const fromLine = Math.max(toLine - MAX_EXPAND_CONTEXT_LINES, _fromLine) + + const _rightLine = lineBefore ? numberTo(lineBefore) + 1 : 1 + const rightLine = Math.max(toLine - MAX_EXPAND_CONTEXT_LINES, _rightLine) + + fetchChunkAndInsert({target: lineAfter, packageName, version, fromLine, toLine, rightLine, fileName, patch}) + }) + }) + + document.querySelectorAll('.ghd-expand-down').forEach(expandDown => { + expandDown.addEventListener('click', e => { + const {fileName, packageName, version, lineAfter, lineBefore, patch} = gatherInfo(expandDown) + + const fromLine = numberFrom(lineBefore) + 1 + const rightLine = numberTo(lineBefore) + 1 + + const _toLine = lineAfter ? numberFrom(lineAfter) - 1 : Infinity + const toLine = Math.min(fromLine + MAX_EXPAND_CONTEXT_LINES, _toLine) + + fetchChunkAndInsert({target: expandDown.closest('tr'), packageName, version, fromLine, toLine, rightLine, fileName, patch}) + + }) + }) +}) + +const numberFrom = line => parseInt(line.querySelector('.ghd-line-number .ghd-line-number-from').textContent.trim()) +const numberTo = line => parseInt(line.querySelector('.ghd-line-number .ghd-line-number-to').textContent.trim()) + +const gatherInfo = line => { + const patch = line.closest('.ghd-file') + const {fileName, packageName, version} = patch.querySelector('.ghd-file-header').dataset + + const lineAfter = line.closest('tr').nextElementSibling + const lineBefore = line.closest('tr').previousElementSibling + + return {fileName, packageName, version, lineAfter, lineBefore, patch} +} + +const fetchChunkAndInsert = params => { + if(!params.fromLine || !params.toLine || (params.fromLine >= params.toLine)){ + return + } + + const path = `/diff/${params.packageName}/${params.version}/expand/${params.fromLine}/${params.toLine}/${params.rightLine}` + const url = new URL(path, window.location) + url.searchParams.append('file_name', params.fileName) + + fetch(url) + .then(response => response.json()) + .then(({chunk, lines, errors}) => { + if(errors) return + const context = document.createElement('template') + context.innerHTML = chunk.trim() + const patchBody = params.patch.querySelector('tbody') + + Array.prototype.reduceRight.call(context.content.childNodes, (target, line) => { + return patchBody.insertBefore(line, target) + }, params.target) + }) + .catch(console.error) +} diff --git a/lib/diff/hex/chunk_extractor.ex b/lib/diff/hex/chunk_extractor.ex new file mode 100644 index 0000000..94086dc --- /dev/null +++ b/lib/diff/hex/chunk_extractor.ex @@ -0,0 +1,112 @@ +defmodule Diff.Hex.ChunkExtractor do + @enforce_keys [:file_path, :from_line, :to_line, :right_line] + defstruct Enum.map(@enforce_keys, &{&1, nil}) ++ [errors: [], raw: nil, parsed: nil] + + def run(params) do + params + |> cast() + |> parse_integers([:right_line, :from_line, :to_line]) + |> normalize([:from_line, :right_line]) + |> validate_to_line() + |> system_read_raw_chunk() + |> parse_chunk() + |> case do + %{errors: []} = chunk -> {:ok, chunk} + %{errors: _} = chunk -> {:error, chunk} + end + end + + defp cast(params) do + struct_keys = + struct(__MODULE__) + |> Map.from_struct() + |> Enum.map(fn {key, _val} -> key end) + + struct!(__MODULE__, Map.take(params, struct_keys)) + end + + defp parse_integers(chunk, fields) do + Enum.reduce(fields, chunk, &parse_integer/2) + end + + defp parse_integer(field, chunk) do + value = chunk |> Map.get(field) |> parse_value() + + case value do + :error -> %{chunk | errors: [{:parse_integer, "#{field} must be a number"} | chunk.errors]} + integer -> Map.put(chunk, field, integer) + end + end + + defp parse_value(number) when is_integer(number), do: number + + defp parse_value(number) when is_binary(number) do + with {int, _} <- Integer.parse(number), do: int + end + + defp normalize(%{errors: [_ | _]} = chunk, _), do: chunk + + defp normalize(chunk, fields) do + Enum.reduce(fields, chunk, &normalize_field/2) + end + + defp normalize_field(field, chunk) do + case Map.fetch!(chunk, field) do + value when value > 0 -> chunk + _ -> Map.put(chunk, field, 1) + end + end + + defp validate_to_line(%{errors: [_ | _]} = chunk), do: chunk + + defp validate_to_line(%{from_line: from_line, to_line: to_line} = chunk) + when from_line < to_line do + chunk + end + + defp validate_to_line(chunk) do + error = {:param, "from_line parameter must be less than to_line"} + %{chunk | errors: [error | chunk.errors]} + end + + defp system_read_raw_chunk(%{errors: [_ | _]} = chunk), do: chunk + + defp system_read_raw_chunk(chunk) do + path = chunk.file_path + from_line = to_string(chunk.from_line) + to_line = to_string(chunk.to_line) + + case System.cmd("sed", ["-n", "#{from_line},#{to_line}p", path], stderr_to_stdout: true) do + {raw_chunk, 0} -> + %{chunk | raw: raw_chunk} + + {error, code} -> + error = {:system, "System command exited with a non-zero status #{code}: #{error}"} + %{chunk | errors: [error | chunk.errors]} + end + end + + defp parse_chunk(%{errors: [_ | _]} = chunk), do: chunk + + defp parse_chunk(chunk) do + parsed = + chunk.raw + |> String.split("\n") + |> remove_trailing_newline() + |> Enum.with_index(chunk.from_line) + |> Enum.with_index(chunk.right_line) + # prepend line with whitespace to compensate the space introduced by leading + and - in diffs + |> Enum.map(fn {{line, from}, to} -> + %{text: " " <> line, from_line_number: from, to_line_number: to} + end) + + %{chunk | parsed: parsed} + end + + defp remove_trailing_newline(lines) do + lines + |> Enum.reverse() + |> tl() + |> Enum.reverse() + end +end diff --git a/lib/diff/hex/hex.ex b/lib/diff/hex/hex.ex index 21e9ef2..b236e34 100644 --- a/lib/diff/hex/hex.ex +++ b/lib/diff/hex/hex.ex @@ -39,9 +39,18 @@ defmodule Diff.Hex do end def unpack_tarball(tarball, path) when is_binary(path) do + do_unpack_tarball(tarball, :all_files, path) + end + + def unpack_tarball(tarball, file_list, path) when is_binary(path) do + file_list = Enum.map(file_list, &to_charlist/1) + do_unpack_tarball(tarball, file_list, path) + end + + def do_unpack_tarball(tarball, file_list, path) do path = to_charlist(path) - with {:ok, _} <- :hex_tarball.unpack(tarball, path) do + with {:ok, _} <- :hex_tarball.unpack(tarball, file_list, path) do :ok end end @@ -100,6 +109,30 @@ defmodule Diff.Hex do end end + def get_chunk(params) do + path = tmp_path("chunk") + + chunk_extractor_params = Map.put(params, :file_path, Path.join(path, params.file_name)) + + try do + with {:ok, tarball} <- get_tarball(params.package, params.version), + :ok <- unpack_tarball(tarball, [params.file_name], path), + {:ok, %{parsed: parsed_chunk}} <- Diff.Hex.ChunkExtractor.run(chunk_extractor_params) do + {:ok, parsed_chunk} + else + {:error, %Diff.Hex.ChunkExtractor{errors: errors} = chunk} -> + Logger.error(inspect(errors)) + {:error, chunk} + + error -> + Logger.error(inspect(error)) + {:error, error} + end + after + File.rm_rf(path) + end + end + defp git_diff(path_from, path_to, path_out) do case System.cmd("git", [ "-c", diff --git a/lib/diff_web/controllers/page_controller.ex b/lib/diff_web/controllers/page_controller.ex index 1aa0dfb..f194eba 100644 --- a/lib/diff_web/controllers/page_controller.ex +++ b/lib/diff_web/controllers/page_controller.ex @@ -28,6 +28,27 @@ defmodule DiffWeb.PageController do end end + def expand_context(conn, %{"file_name" => _file_name} = params) do + case parse_version(params["version"]) do + {:ok, version} -> + version = to_string(version) + params = Map.update!(params, "version", fn _ -> version end) + + do_expand_context(conn, params) + + :error -> + conn + |> put_status(400) + |> json(%{error: "Bad Request"}) + end + end + + def expand_context(conn, _params) do + conn + |> put_status(400) + |> json(%{error: "missing query parameter: file_name"}) + end + defp maybe_cached_diff(conn, _package, version, version) do render_error(conn, 400) end @@ -70,6 +91,32 @@ defmodule DiffWeb.PageController do end end + defp do_expand_context(conn, params) do + chunk_extractor_params = + for {key, val} <- params, into: %{} do + {String.to_existing_atom(key), val} + end + + case Diff.Hex.get_chunk(chunk_extractor_params) do + {:ok, chunk} -> + rendered_chunk = + Phoenix.View.render_to_string( + DiffWeb.RenderView, + "render_context_chunk.html", + chunk: chunk + ) + + conn + |> put_status(200) + |> json(%{chunk: rendered_chunk, lines: length(chunk)}) + + {:error, %{errors: errors}} -> + conn + |> put_status(400) + |> json(%{errors: Enum.into(errors, %{})}) + end + end + defp do_diff(conn, package, from, to) do case Diff.Hex.diff(package, from, to) do {:ok, stream} -> @@ -134,7 +181,11 @@ defmodule DiffWeb.PageController do Enum.each(stream, fn {:ok, patch} -> html_patch = - Phoenix.View.render_to_iodata(DiffWeb.RenderView, "render.html", patch: patch) + Phoenix.View.render_to_iodata(DiffWeb.RenderView, "render.html", + patch: patch, + package: package, + from_version: from + ) IO.binwrite(file, html_patch) diff --git a/lib/diff_web/router.ex b/lib/diff_web/router.ex index 302133f..b9427d0 100644 --- a/lib/diff_web/router.ex +++ b/lib/diff_web/router.ex @@ -15,6 +15,11 @@ defmodule DiffWeb.Router do pipe_through :browser live "/", SearchLiveView + + get "/diff/:package/:version/expand/:from_line/:to_line/:right_line/", + PageController, + :expand_context + get "/diff/:package/:versions", PageController, :diff get "/diffs", PageController, :diffs end diff --git a/lib/diff_web/templates/render/render.html.eex b/lib/diff_web/templates/render/render.html.eex index 793eae6..1911ac5 100644 --- a/lib/diff_web/templates/render/render.html.eex +++ b/lib/diff_web/templates/render/render.html.eex @@ -1,6 +1,6 @@ <% status = patch_status(@patch) %>
-
+
<%= status %> @@ -10,11 +10,13 @@
- <%= for chunk <- @patch.chunks do %> + <%= for {chunk, index} <- Enum.with_index(@patch.chunks) do %> <% end %> <% end %> + + + +
-
 
-
+ <%= unless index == 0 do %> +
+ <% end %> +
<%= chunk.header %>
@@ -36,6 +38,14 @@
+
+ 		+
+
diff --git a/lib/diff_web/templates/render/render_context_chunk.html.eex b/lib/diff_web/templates/render/render_context_chunk.html.eex new file mode 100644 index 0000000..90396ee --- /dev/null +++ b/lib/diff_web/templates/render/render_context_chunk.html.eex @@ -0,0 +1,15 @@ +<%= for line <- @chunk do %> + + +
+ <%= line_number(line.from_line_number) %> +
+
+ <%= line_number(line.to_line_number) %> +
+ + +
<%= line_text(line.text) %>
+ + +<% end %> diff --git a/mix.exs b/mix.exs index c021fb1..37d246f 100644 --- a/mix.exs +++ b/mix.exs @@ -43,7 +43,7 @@ defmodule Diff.MixProject do {:jason, "~> 1.0"}, {:plug_cowboy, "~> 2.0"}, {:phoenix_live_view, "~> 0.6"}, - {:hex_core, "~> 0.6.1"}, + {:hex_core, github: "RudolfMan/hex_core", branch: "unpack-list-of-files"}, {:rollbax, "~> 0.11.0"}, {:logster, "~> 1.0"}, {:git_diff, github: "hexpm/git_diff", branch: "emj/stream"}, diff --git a/mix.lock b/mix.lock index 5c93a0b..29e5e13 100644 --- a/mix.lock +++ b/mix.lock @@ -9,7 +9,7 @@ "git_diff": {:git, "https://github.com/hexpm/git_diff.git", "453ed1933e87e47c2debef1b5b77a0dc4a1c3fb7", [branch: "emj/stream"]}, "goth": {:hex, :goth, "1.2.0", "92d6d926065a72a7e0da8818cc3a133229b56edf378022c00d9886c4125ce769", [:mix], [{:httpoison, "~> 0.11 or ~> 1.0", [hex: :httpoison, repo: "hexpm", optional: false]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}, {:joken, "~> 2.0", [hex: :joken, repo: "hexpm", optional: false]}], "hexpm", "4974932ab3b782c99a6fdeb0b968ddd61436ef14de5862bd6bb0227386c63b26"}, "hackney": {:hex, :hackney, "1.15.2", "07e33c794f8f8964ee86cebec1a8ed88db5070e52e904b8f12209773c1036085", [:rebar3], [{:certifi, "2.5.1", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "6.0.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.5", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm", "e0100f8ef7d1124222c11ad362c857d3df7cb5f4204054f9f0f4a728666591fc"}, - "hex_core": {:hex, :hex_core, "0.6.6", "a253f0abd41e10bc33cb73cdb1261a33a710559243ef3d6ca4d7ade0462a3f2c", [:rebar3], [], "hexpm", "f1aa2bf3a27520055d94e7c03880314f77c46ab19e1b280e8b8e46981ae92286"}, + "hex_core": {:git, "https://github.com/RudolfMan/hex_core.git", "7886b9c8a0da00c5d021c739a36f80665716ceac", [branch: "unpack-list-of-files"]}, "html_entities": {:hex, :html_entities, "0.5.0", "40f5c5b9cbe23073b48a4e69c67b6c11974f623a76165e2b92d098c0e88ccb1d", [:mix], [], "hexpm", "8e9186e1873bea1067895f6a542b59df6c9fcf3b516ba272eeff3ea0c7b755cd"}, "httpoison": {:hex, :httpoison, "1.6.2", "ace7c8d3a361cebccbed19c283c349b3d26991eff73a1eaaa8abae2e3c8089b6", [:mix], [{:hackney, "~> 1.15 and >= 1.15.2", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm", "aa2c74bd271af34239a3948779612f87df2422c2fdcfdbcec28d9c105f0773fe"}, "idna": {:hex, :idna, "6.0.0", "689c46cbcdf3524c44d5f3dde8001f364cd7608a99556d8fbd8239a5798d4c10", [:rebar3], [{:unicode_util_compat, "0.4.1", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "4bdd305eb64e18b0273864920695cb18d7a2021f31a11b9c5fbcd9a253f936e2"}, diff --git a/test/diff/hex/chunk_extractor_test.exs b/test/diff/hex/chunk_extractor_test.exs new file mode 100644 index 0000000..1aa6b05 --- /dev/null +++ b/test/diff/hex/chunk_extractor_test.exs @@ -0,0 +1,73 @@ +defmodule Diff.Hex.ChunkExtractorTest do + use ExUnit.Case, async: true + + alias Diff.Hex.ChunkExtractor + + setup do + content = """ + foo 1 + bar 2 + baz 3 + baf 4 + """ + + path = System.tmp_dir!() |> Path.join("test_file") + File.write!(path, content) + + on_exit(fn -> File.rm!(path) end) + + # some deafult params + %{ + params: %{file_path: path, right_line: 1, from_line: 1, to_line: 2} + } + end + + describe "reads raw chunk from the file_path" do + test "reads first 2 lines down", %{params: params} do + {:ok, %{raw: raw}} = ChunkExtractor.run(%{params | from_line: 1, to_line: 2}) + assert "foo 1\nbar 2\n" = raw + end + + test "error when file doesn't exist", %{params: params} do + {:error, %{errors: errors}} = ChunkExtractor.run(%{params | file_path: "non_existent"}) + assert Keyword.get(errors, :system) =~ ~r/non_existent: No such file/ + end + + test "returns from 1 when from_line is negative", %{params: params} do + {:ok, %{parsed: actual}} = + ChunkExtractor.run(%{params | from_line: -2, right_line: -2, to_line: 2}) + + assert [ + %{from_line_number: 1, to_line_number: 1}, + %{from_line_number: 2, to_line_number: 2} + ] = actual + end + + test "error when arguments are not valid", %{params: params} do + {:error, %{errors: errors}} = ChunkExtractor.run(%{params | from_line: -4, to_line: -3}) + assert Keyword.get(errors, :param) == "from_line parameter must be less than to_line" + end + + test "reads 2 lines from the middle", %{params: params} do + {:ok, %{raw: raw}} = ChunkExtractor.run(%{params | from_line: 2, to_line: 4}) + assert "bar 2\nbaz 3\nbaf 4\n" = raw + end + end + + describe "parse_chunk" do + test "parses raw chunk into list of structs", %{params: params} do + {:ok, %{parsed: actual}} = ChunkExtractor.run(params) + assert [%{text: " foo 1"}, %{text: " bar 2"}] = actual + end + + test "sets line_numbers", %{params: params} do + {:ok, %{parsed: actual}} = + ChunkExtractor.run(%{params | from_line: 2, right_line: 1, to_line: 3}) + + assert [ + %{from_line_number: 2, to_line_number: 1}, + %{from_line_number: 3, to_line_number: 2} + ] = actual + end + end +end diff --git a/test/diff/hex/hex_test.exs b/test/diff/hex/hex_test.exs new file mode 100644 index 0000000..e2b7add --- /dev/null +++ b/test/diff/hex/hex_test.exs @@ -0,0 +1,62 @@ +defmodule Diff.HexTest do + use ExUnit.Case, async: true + + setup do + metadata = %{"name" => "foo", "version" => "0.1.0"} + + foo = """ + foo 1 + bar 2 + baz 3 + """ + + bar = """ + defmodule Foo do + defstruct bar: nil + end + """ + + files = [{'foo', foo}, {'bar', bar}] + {:ok, %{tarball: tarball}} = :hex_tarball.create(metadata, files) + path = "tmp/diff-test" + File.mkdir_p!(path) + + on_exit(fn -> + File.rm_rf!(path) + end) + + %{tarball: tarball, files: files, path: path} + end + + describe "unpack_tarball" do + test "by default upacks all files", context do + %{tarball: tarball, files: files, path: path} = context + :ok = Diff.Hex.unpack_tarball(tarball, path) + + actual = + path + |> File.ls!() + |> MapSet.new() + + expected = + files + |> Enum.map(fn {name, _content} -> to_string(name) end) + |> MapSet.new() + + assert MapSet.subset?(expected, actual) + end + + test "unpacks only files listed in the second argument", context do + %{tarball: tarball, path: path} = context + :ok = Diff.Hex.unpack_tarball(tarball, ["foo"], path) + + actual = + path + |> File.ls!() + |> MapSet.new() + + assert MapSet.member?(actual, "foo") + refute MapSet.member?(actual, "bar") + end + end +end diff --git a/test/diff_web/controllers/page_controller_test.exs b/test/diff_web/controllers/page_controller_test.exs index 24646de..9ee0bcd 100644 --- a/test/diff_web/controllers/page_controller_test.exs +++ b/test/diff_web/controllers/page_controller_test.exs @@ -41,4 +41,24 @@ defmodule DiffWeb.PageControllerTest do assert html_response(conn, 200) =~ diff end end + + describe "/GET /diff/:package/:versions/expand/:from/:to/:right_line" do + setup :verify_on_exit! + + test "requires file_name query param", %{conn: conn} do + conn = get(conn, "/diff/phoenix/1.4.5/expand/1/1/1") + assert %{"error" => "missing query parameter: file_name"} = json_response(conn, 400) + end + + test "doesn't accept when to_line is less than from_line", %{conn: conn} do + conn = get(conn, "/diff/phoenix/1.4.5/expand/2/1/1?file_name=mix.exs") + assert json_response(conn, 400) + end + + test "returns chunk of the file", %{conn: conn} do + conn = get(conn, "/diff/phoenix/1.4.5/expand/1/4/1?file_name=mix.exs") + assert %{"chunk" => chunk} = json_response(conn, 200) + assert chunk =~ "defmodule Phoenix.MixProject do" + end + end end