From 78f04c8de55d6d500cb14efbf4c251e97fc4b8a7 Mon Sep 17 00:00:00 2001 From: RudolfMan <53276677+RudolfMan@users.noreply.github.com> Date: Sun, 26 Apr 2020 02:51:19 -0700 Subject: [PATCH 1/8] Update required min elixir version (#50) --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index 4f927b2..c021fb1 100644 --- a/mix.exs +++ b/mix.exs @@ -5,7 +5,7 @@ defmodule Diff.MixProject do [ app: :diff, version: "0.1.0", - elixir: "~> 1.5", + elixir: "~> 1.10", elixirc_paths: elixirc_paths(Mix.env()), compilers: [:phoenix, :gettext] ++ Mix.compilers(), start_permanent: Mix.env() == :prod, From 5af516af456b996973e577a86d3032a522835c6c Mon Sep 17 00:00:00 2001 From: Rudolf Manusadzhian Date: Mon, 18 May 2020 02:54:50 -0700 Subject: [PATCH 2/8] add an endpoint to receive the chunk of the file add chunk.sh shell script that returns cut of the file based on starting line, number of lines to read and direction (up or down) add ChunkExtractor module that sanitizes params and handles response of the chunk.sh script add a json endpoint to be able to request chunks --- lib/diff/hex/chunk_extractor.ex | 112 ++++++++++++++++++ lib/diff/hex/hex.ex | 34 +++++- lib/diff_web/controllers/page_controller.ex | 38 ++++++ lib/diff_web/router.ex | 1 + mix.exs | 2 +- mix.lock | 2 +- priv/chunk.sh | 21 ++++ test/diff/hex/chunk_extractor_test.exs | 88 ++++++++++++++ test/diff/hex/hex_test.exs | 62 ++++++++++ .../controllers/page_controller_test.exs | 32 +++++ 10 files changed, 388 insertions(+), 4 deletions(-) create mode 100644 lib/diff/hex/chunk_extractor.ex create mode 100755 priv/chunk.sh create mode 100644 test/diff/hex/chunk_extractor_test.exs create mode 100644 test/diff/hex/hex_test.exs diff --git a/lib/diff/hex/chunk_extractor.ex b/lib/diff/hex/chunk_extractor.ex new file mode 100644 index 0000000..71a49dd --- /dev/null +++ b/lib/diff/hex/chunk_extractor.ex @@ -0,0 +1,112 @@ +defmodule Diff.Hex.ChunkExtractor do + @enforce_keys [:file_path, :from_line, :lines_to_read, :direction] + defstruct Enum.map(@enforce_keys, &{&1, nil}) ++ + [ + errors: [], + raw: nil, + parsed: nil + ] + + def run(params) do + __MODULE__ + |> struct!(params) + |> parse_integers([:from_line, :lines_to_read]) + |> validate_direction() + |> system_read_raw_chunk() + |> parse_chunk() + |> remove_trailing_newline() + 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"}} + 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 validate_direction(%{direction: direction} = chunk) when direction in ["up", "down"] do + chunk + end + + defp validate_direction(chunk) do + error = {:direction, "direction must be either \"up\" or \"down\""} + %{chunk | errors: [error | chunk.errors]} + end + + defp system_read_raw_chunk(%{errors: [_ | _]} = chunk), do: chunk + + defp system_read_raw_chunk(chunk) do + chunk_sh = Application.app_dir(:diff, ["priv", "chunk.sh"]) + + path = chunk.file_path + from_line = to_string(chunk.from_line) + lines_to_read = to_string(chunk.lines_to_read) + direction = chunk.direction + + case System.cmd(chunk_sh, [path, from_line, lines_to_read, direction], 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") + |> Enum.map(fn line -> %{line_text: line} end) + + set_line_numbers(%{chunk | parsed: parsed}) + end + + defp set_line_numbers(%{direction: "down"} = chunk) do + %{chunk | parsed: parsed_with_line_numbers(chunk.parsed, chunk.from_line)} + end + + defp set_line_numbers(%{direction: "up"} = chunk) do + offset = chunk.from_line - length(chunk.parsed) + 1 + + %{chunk | parsed: parsed_with_line_numbers(chunk.parsed, offset)} + end + + defp parsed_with_line_numbers(parsed_chunk, starting_number) when is_binary(starting_number) do + parsed_with_line_numbers(parsed_chunk, starting_number) + end + + defp parsed_with_line_numbers(parsed_chunk, starting_number) do + parsed_chunk + |> Enum.with_index(starting_number) + |> Enum.map(fn {line, line_number} -> Map.put_new(line, :line_number, line_number) end) + end + + defp remove_trailing_newline(%{errors: [_ | _]} = chunk), do: {:error, chunk} + + defp remove_trailing_newline(chunk) do + [trailing_line | reversed_tail] = Enum.reverse(chunk.parsed) + + chunk = + case trailing_line do + %{line_text: ""} -> %{chunk | parsed: Enum.reverse(reversed_tail)} + %{line_text: _text} -> chunk + end + + {:ok, chunk} + end +end diff --git a/lib/diff/hex/hex.ex b/lib/diff/hex/hex.ex index 21e9ef2..6a664e7 100644 --- a/lib/diff/hex/hex.ex +++ b/lib/diff/hex/hex.ex @@ -38,10 +38,11 @@ defmodule Diff.Hex do end end - def unpack_tarball(tarball, path) when is_binary(path) do + def unpack_tarball(tarball, file_list \\ [], path) when is_binary(path) do path = to_charlist(path) + file_list = Enum.map(file_list, &to_charlist/1) - with {:ok, _} <- :hex_tarball.unpack(tarball, path) do + with {:ok, _} <- :hex_tarball.unpack(tarball, file_list, path) do :ok end end @@ -100,6 +101,35 @@ defmodule Diff.Hex do end end + def get_chunk(package, version, file_name, from_line, direction) do + path = tmp_path("package-#{package}-#{version}-") + + chunk_extractor_params = %{ + file_path: Path.join(path, file_name), + from_line: from_line, + direction: direction, + lines_to_read: 20 + } + + try do + with {:ok, tarball} <- get_tarball(package, version), + :ok <- unpack_tarball(tarball, [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..f63b45d 100644 --- a/lib/diff_web/controllers/page_controller.ex +++ b/lib/diff_web/controllers/page_controller.ex @@ -28,6 +28,32 @@ defmodule DiffWeb.PageController do end end + def expand_context(conn, %{"file_name" => file_name} = params) do + %{ + "version" => version, + "package" => package, + "from_line" => from_line, + "direction" => direction + } = params + + case parse_version(version) do + {:ok, version} -> + version = to_string(version) + do_expand_context(conn, package, version, file_name, from_line, direction) + + :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 +96,18 @@ defmodule DiffWeb.PageController do end end + defp do_expand_context(conn, package, version, file_name, from_line, direction) do + case Diff.Hex.get_chunk(package, version, file_name, from_line, direction) do + {:ok, chunk} -> + json(conn, %{chunk: 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} -> diff --git a/lib/diff_web/router.ex b/lib/diff_web/router.ex index 302133f..191e07e 100644 --- a/lib/diff_web/router.ex +++ b/lib/diff_web/router.ex @@ -15,6 +15,7 @@ defmodule DiffWeb.Router do pipe_through :browser live "/", SearchLiveView + get "/diff/:package/:version/expand/:from_line/:direction", PageController, :expand_context get "/diff/:package/:versions", PageController, :diff get "/diffs", PageController, :diffs 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..44f427f 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", "8295d4b67f7d461a991fa34507b0e2c74a3201ea", [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/priv/chunk.sh b/priv/chunk.sh new file mode 100755 index 0000000..2754bfb --- /dev/null +++ b/priv/chunk.sh @@ -0,0 +1,21 @@ +#!/bin/sh + +file=$1 +from_line=$2 +lines_to_read=$3 +direction=$4 + +if [ $direction = "down" ] +then + TAIL="$(tail -n+$from_line $file)" + TMP_STATUS=$? + printf '%s' "$TAIL" | head -n$lines_to_read + FINAL_STATUS=$? +else + HEAD="$(head -n$from_line $file)" + TMP_STATUS=$? + printf '%s' "$HEAD" | tail -n$lines_to_read + FINAL_STATUS=$? +fi + +[ $TMP_STATUS -gt 0 ] && exit $TMP_STATUS || exit $FINAL_STATUS \ No newline at end of file diff --git a/test/diff/hex/chunk_extractor_test.exs b/test/diff/hex/chunk_extractor_test.exs new file mode 100644 index 0000000..7acc518 --- /dev/null +++ b/test/diff/hex/chunk_extractor_test.exs @@ -0,0 +1,88 @@ +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, lines_to_read: 2, from_line: 1, direction: "down"}} + end + + describe "validates direction" do + test "down", %{params: params} do + {:ok, %{errors: errors}} = ChunkExtractor.run(%{params | direction: "down"}) + assert [] = errors + end + + test "up", %{params: params} do + {:ok, %{errors: errors}} = ChunkExtractor.run(%{params | direction: "up"}) + assert [] = errors + end + + test "error when direction is neither up nor down", %{params: params} do + {:error, %{errors: errors}} = ChunkExtractor.run(%{params | direction: "left"}) + assert "direction must be either \"up\" or \"down\"" = Keyword.get(errors, :direction) + end + 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 | direction: "down"}) + assert "foo 1\nbar 2\n" = raw + end + + test "reads first 2 lines up", %{params: params} do + {:ok, %{raw: raw}} = ChunkExtractor.run(%{params | direction: "up", from_line: 2}) + assert "foo 1\nbar 2" = 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 "error when arguments are not valid", %{params: params} do + {:error, %{errors: errors}} = ChunkExtractor.run(%{params | from_line: -1}) + assert Keyword.get(errors, :system) =~ ~r/illegal offset/ + end + + test "reads 2 lines up from the middle", %{params: params} do + {:ok, %{raw: raw}} = ChunkExtractor.run(%{params | direction: "up", from_line: 3}) + assert "bar 2\nbaz 3" = raw + end + + test "reads 2 lines down from the middle", %{params: params} do + {:ok, %{raw: raw}} = ChunkExtractor.run(%{params | direction: "down", from_line: 2}) + assert "bar 2\nbaz 3\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 [%{line_text: "foo 1"}, %{line_text: "bar 2"}] = actual + end + + test "sets line_numbers when direction is down", %{params: params} do + {:ok, %{parsed: actual}} = ChunkExtractor.run(%{params | direction: "down", from_line: 2}) + assert [%{line_number: 2}, %{line_number: 3}] = actual + end + + test "sets line_numbers when direction is up", %{params: params} do + {:ok, %{parsed: actual}} = ChunkExtractor.run(%{params | direction: "up", from_line: 3}) + assert [%{line_number: 2}, %{line_number: 3}] = 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..7110c3a 100644 --- a/test/diff_web/controllers/page_controller_test.exs +++ b/test/diff_web/controllers/page_controller_test.exs @@ -41,4 +41,36 @@ defmodule DiffWeb.PageControllerTest do assert html_response(conn, 200) =~ diff end end + + describe "/GET /diff/:package/:versions/expand/:from_line/:direction" do + setup :verify_on_exit! + + test "requires file_name query param", %{conn: conn} do + conn = get(conn, "/diff/phoenix/1.4.5/expand/1/down") + assert %{"error" => "missing query parameter: file_name"} = json_response(conn, 400) + end + + test "doesn't accept direction other that up or down", %{conn: conn} do + conn = get(conn, "/diff/phoenix/1.4.5/expand/1/left?file_name=mix.exs") + assert %{"errors" => %{"direction" => error}} = json_response(conn, 400) + assert error =~ ~r/direction must be either \"up\" or \"down\"/ + end + + test "doesn't accept negative from_line", %{conn: conn} do + conn = get(conn, "/diff/phoenix/1.4.5/expand/-2/up?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/2/up?file_name=mix.exs") + assert %{"chunk" => chunk} = json_response(conn, 200) + + assert [ + %{"line_text" => "defmodule Phoenix.MixProject do"}, + %{"line_text" => " use Mix.Project"} + ] = chunk + + assert 2 = length(chunk) + end + end end From ca61d2bc4d9bf8dec748765d29e897e82a44d2b6 Mon Sep 17 00:00:00 2001 From: Rudolf Manusadzhian Date: Wed, 10 Jun 2020 02:53:11 -0700 Subject: [PATCH 3/8] Refactor chunk_extractor api simplify the logic, get rid of direction parameter now it only tries to retrieve the chunk from the top to the down got rid of direction param and custom chunk.sh script use shell sed command --- lib/diff/hex/chunk_extractor.ex | 112 ++++++++++++------------- lib/diff/hex/hex.ex | 15 ++-- priv/chunk.sh | 21 ----- test/diff/hex/chunk_extractor_test.exs | 67 ++++++--------- 4 files changed, 87 insertions(+), 128 deletions(-) delete mode 100755 priv/chunk.sh diff --git a/lib/diff/hex/chunk_extractor.ex b/lib/diff/hex/chunk_extractor.ex index 71a49dd..94086dc 100644 --- a/lib/diff/hex/chunk_extractor.ex +++ b/lib/diff/hex/chunk_extractor.ex @@ -1,20 +1,28 @@ defmodule Diff.Hex.ChunkExtractor do - @enforce_keys [:file_path, :from_line, :lines_to_read, :direction] - defstruct Enum.map(@enforce_keys, &{&1, nil}) ++ - [ - errors: [], - raw: nil, - parsed: nil - ] + @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 - __MODULE__ - |> struct!(params) - |> parse_integers([:from_line, :lines_to_read]) - |> validate_direction() + params + |> cast() + |> parse_integers([:right_line, :from_line, :to_line]) + |> normalize([:from_line, :right_line]) + |> validate_to_line() |> system_read_raw_chunk() |> parse_chunk() - |> remove_trailing_newline() + |> 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 @@ -25,7 +33,7 @@ defmodule Diff.Hex.ChunkExtractor do value = chunk |> Map.get(field) |> parse_value() case value do - :error -> %{chunk | errors: {:parse_integer, "#{field} must be a number"}} + :error -> %{chunk | errors: [{:parse_integer, "#{field} must be a number"} | chunk.errors]} integer -> Map.put(chunk, field, integer) end end @@ -36,26 +44,39 @@ defmodule Diff.Hex.ChunkExtractor do with {int, _} <- Integer.parse(number), do: int end - defp validate_direction(%{direction: direction} = chunk) when direction in ["up", "down"] do + 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_direction(chunk) do - error = {:direction, "direction must be either \"up\" or \"down\""} + 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 - chunk_sh = Application.app_dir(:diff, ["priv", "chunk.sh"]) - path = chunk.file_path from_line = to_string(chunk.from_line) - lines_to_read = to_string(chunk.lines_to_read) - direction = chunk.direction + to_line = to_string(chunk.to_line) - case System.cmd(chunk_sh, [path, from_line, lines_to_read, direction], stderr_to_stdout: true) do + case System.cmd("sed", ["-n", "#{from_line},#{to_line}p", path], stderr_to_stdout: true) do {raw_chunk, 0} -> %{chunk | raw: raw_chunk} @@ -71,42 +92,21 @@ defmodule Diff.Hex.ChunkExtractor do parsed = chunk.raw |> String.split("\n") - |> Enum.map(fn line -> %{line_text: line} end) - - set_line_numbers(%{chunk | parsed: parsed}) + |> 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 set_line_numbers(%{direction: "down"} = chunk) do - %{chunk | parsed: parsed_with_line_numbers(chunk.parsed, chunk.from_line)} - end - - defp set_line_numbers(%{direction: "up"} = chunk) do - offset = chunk.from_line - length(chunk.parsed) + 1 - - %{chunk | parsed: parsed_with_line_numbers(chunk.parsed, offset)} - end - - defp parsed_with_line_numbers(parsed_chunk, starting_number) when is_binary(starting_number) do - parsed_with_line_numbers(parsed_chunk, starting_number) - end - - defp parsed_with_line_numbers(parsed_chunk, starting_number) do - parsed_chunk - |> Enum.with_index(starting_number) - |> Enum.map(fn {line, line_number} -> Map.put_new(line, :line_number, line_number) end) - end - - defp remove_trailing_newline(%{errors: [_ | _]} = chunk), do: {:error, chunk} - - defp remove_trailing_newline(chunk) do - [trailing_line | reversed_tail] = Enum.reverse(chunk.parsed) - - chunk = - case trailing_line do - %{line_text: ""} -> %{chunk | parsed: Enum.reverse(reversed_tail)} - %{line_text: _text} -> chunk - end - - {:ok, chunk} + 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 6a664e7..1a5e0aa 100644 --- a/lib/diff/hex/hex.ex +++ b/lib/diff/hex/hex.ex @@ -101,19 +101,14 @@ defmodule Diff.Hex do end end - def get_chunk(package, version, file_name, from_line, direction) do - path = tmp_path("package-#{package}-#{version}-") + def get_chunk(params) do + path = tmp_path("chunk") - chunk_extractor_params = %{ - file_path: Path.join(path, file_name), - from_line: from_line, - direction: direction, - lines_to_read: 20 - } + chunk_extractor_params = Map.put(params, :file_path, Path.join(path, params.file_name)) try do - with {:ok, tarball} <- get_tarball(package, version), - :ok <- unpack_tarball(tarball, [file_name], path), + 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 diff --git a/priv/chunk.sh b/priv/chunk.sh deleted file mode 100755 index 2754bfb..0000000 --- a/priv/chunk.sh +++ /dev/null @@ -1,21 +0,0 @@ -#!/bin/sh - -file=$1 -from_line=$2 -lines_to_read=$3 -direction=$4 - -if [ $direction = "down" ] -then - TAIL="$(tail -n+$from_line $file)" - TMP_STATUS=$? - printf '%s' "$TAIL" | head -n$lines_to_read - FINAL_STATUS=$? -else - HEAD="$(head -n$from_line $file)" - TMP_STATUS=$? - printf '%s' "$HEAD" | tail -n$lines_to_read - FINAL_STATUS=$? -fi - -[ $TMP_STATUS -gt 0 ] && exit $TMP_STATUS || exit $FINAL_STATUS \ No newline at end of file diff --git a/test/diff/hex/chunk_extractor_test.exs b/test/diff/hex/chunk_extractor_test.exs index 7acc518..1aa6b05 100644 --- a/test/diff/hex/chunk_extractor_test.exs +++ b/test/diff/hex/chunk_extractor_test.exs @@ -17,72 +17,57 @@ defmodule Diff.Hex.ChunkExtractorTest do on_exit(fn -> File.rm!(path) end) # some deafult params - %{params: %{file_path: path, lines_to_read: 2, from_line: 1, direction: "down"}} - end - - describe "validates direction" do - test "down", %{params: params} do - {:ok, %{errors: errors}} = ChunkExtractor.run(%{params | direction: "down"}) - assert [] = errors - end - - test "up", %{params: params} do - {:ok, %{errors: errors}} = ChunkExtractor.run(%{params | direction: "up"}) - assert [] = errors - end - - test "error when direction is neither up nor down", %{params: params} do - {:error, %{errors: errors}} = ChunkExtractor.run(%{params | direction: "left"}) - assert "direction must be either \"up\" or \"down\"" = Keyword.get(errors, :direction) - end + %{ + 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 | direction: "down"}) + {:ok, %{raw: raw}} = ChunkExtractor.run(%{params | from_line: 1, to_line: 2}) assert "foo 1\nbar 2\n" = raw end - test "reads first 2 lines up", %{params: params} do - {:ok, %{raw: raw}} = ChunkExtractor.run(%{params | direction: "up", from_line: 2}) - assert "foo 1\nbar 2" = 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 "error when arguments are not valid", %{params: params} do - {:error, %{errors: errors}} = ChunkExtractor.run(%{params | from_line: -1}) - assert Keyword.get(errors, :system) =~ ~r/illegal offset/ + 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 "reads 2 lines up from the middle", %{params: params} do - {:ok, %{raw: raw}} = ChunkExtractor.run(%{params | direction: "up", from_line: 3}) - assert "bar 2\nbaz 3" = raw + 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 down from the middle", %{params: params} do - {:ok, %{raw: raw}} = ChunkExtractor.run(%{params | direction: "down", from_line: 2}) - assert "bar 2\nbaz 3\n" = raw + 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 [%{line_text: "foo 1"}, %{line_text: "bar 2"}] = actual + assert [%{text: " foo 1"}, %{text: " bar 2"}] = actual end - test "sets line_numbers when direction is down", %{params: params} do - {:ok, %{parsed: actual}} = ChunkExtractor.run(%{params | direction: "down", from_line: 2}) - assert [%{line_number: 2}, %{line_number: 3}] = actual - end + test "sets line_numbers", %{params: params} do + {:ok, %{parsed: actual}} = + ChunkExtractor.run(%{params | from_line: 2, right_line: 1, to_line: 3}) - test "sets line_numbers when direction is up", %{params: params} do - {:ok, %{parsed: actual}} = ChunkExtractor.run(%{params | direction: "up", from_line: 3}) - assert [%{line_number: 2}, %{line_number: 3}] = actual + assert [ + %{from_line_number: 2, to_line_number: 1}, + %{from_line_number: 3, to_line_number: 2} + ] = actual end end end From 4d378f65b757045626b42632b11ecfedb14f490e Mon Sep 17 00:00:00 2001 From: Rudolf Manusadzhian Date: Wed, 10 Jun 2020 03:00:54 -0700 Subject: [PATCH 4/8] change the web layer according to previous refactoring add template to render the chunk --- lib/diff_web/controllers/page_controller.ex | 33 +++++++++++-------- lib/diff_web/router.ex | 6 +++- .../render/render_context_chunk.html.eex | 15 +++++++++ .../controllers/page_controller_test.exs | 24 ++++---------- 4 files changed, 46 insertions(+), 32 deletions(-) create mode 100644 lib/diff_web/templates/render/render_context_chunk.html.eex diff --git a/lib/diff_web/controllers/page_controller.ex b/lib/diff_web/controllers/page_controller.ex index f63b45d..e78f1a2 100644 --- a/lib/diff_web/controllers/page_controller.ex +++ b/lib/diff_web/controllers/page_controller.ex @@ -28,18 +28,13 @@ defmodule DiffWeb.PageController do end end - def expand_context(conn, %{"file_name" => file_name} = params) do - %{ - "version" => version, - "package" => package, - "from_line" => from_line, - "direction" => direction - } = params - - case parse_version(version) do + def expand_context(conn, %{"file_name" => _file_name} = params) do + case parse_version(params["version"]) do {:ok, version} -> version = to_string(version) - do_expand_context(conn, package, version, file_name, from_line, direction) + params = Map.update!(params, "version", fn _ -> version end) + + do_expand_context(conn, params) :error -> conn @@ -96,10 +91,22 @@ defmodule DiffWeb.PageController do end end - defp do_expand_context(conn, package, version, file_name, from_line, direction) do - case Diff.Hex.get_chunk(package, version, file_name, from_line, direction) do + 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} -> - json(conn, %{chunk: 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 diff --git a/lib/diff_web/router.ex b/lib/diff_web/router.ex index 191e07e..b9427d0 100644 --- a/lib/diff_web/router.ex +++ b/lib/diff_web/router.ex @@ -15,7 +15,11 @@ defmodule DiffWeb.Router do pipe_through :browser live "/", SearchLiveView - get "/diff/:package/:version/expand/:from_line/:direction", PageController, :expand_context + + 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_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/test/diff_web/controllers/page_controller_test.exs b/test/diff_web/controllers/page_controller_test.exs index 7110c3a..9ee0bcd 100644 --- a/test/diff_web/controllers/page_controller_test.exs +++ b/test/diff_web/controllers/page_controller_test.exs @@ -42,35 +42,23 @@ defmodule DiffWeb.PageControllerTest do end end - describe "/GET /diff/:package/:versions/expand/:from_line/:direction" do + 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/down") + 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 direction other that up or down", %{conn: conn} do - conn = get(conn, "/diff/phoenix/1.4.5/expand/1/left?file_name=mix.exs") - assert %{"errors" => %{"direction" => error}} = json_response(conn, 400) - assert error =~ ~r/direction must be either \"up\" or \"down\"/ - end - - test "doesn't accept negative from_line", %{conn: conn} do - conn = get(conn, "/diff/phoenix/1.4.5/expand/-2/up?file_name=mix.exs") + 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/2/up?file_name=mix.exs") + conn = get(conn, "/diff/phoenix/1.4.5/expand/1/4/1?file_name=mix.exs") assert %{"chunk" => chunk} = json_response(conn, 200) - - assert [ - %{"line_text" => "defmodule Phoenix.MixProject do"}, - %{"line_text" => " use Mix.Project"} - ] = chunk - - assert 2 = length(chunk) + assert chunk =~ "defmodule Phoenix.MixProject do" end end end From b884f73e847691a6a2a0cf48a690806362ff798c Mon Sep 17 00:00:00 2001 From: Rudolf Manusadzhian Date: Wed, 10 Jun 2020 03:01:39 -0700 Subject: [PATCH 5/8] front end work --- assets/css/app.css | 21 ++++++ assets/js/app.js | 74 +++++++++++++++++++ lib/diff_web/controllers/page_controller.ex | 6 +- lib/diff_web/templates/render/render.html.eex | 18 ++++- 4 files changed, 114 insertions(+), 5 deletions(-) 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..f4241b9 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -57,3 +57,77 @@ 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_web/controllers/page_controller.ex b/lib/diff_web/controllers/page_controller.ex index e78f1a2..487af3c 100644 --- a/lib/diff_web/controllers/page_controller.ex +++ b/lib/diff_web/controllers/page_controller.ex @@ -179,7 +179,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/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 @@
+
+ 		+
+
From 50f290fd6be3d32940499ad05644a72e656e1974 Mon Sep 17 00:00:00 2001 From: Rudolf Manusadzhian Date: Wed, 10 Jun 2020 04:19:07 -0700 Subject: [PATCH 6/8] update hex_core dependency from branch --- mix.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.lock b/mix.lock index 44f427f..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": {:git, "https://github.com/RudolfMan/hex_core.git", "8295d4b67f7d461a991fa34507b0e2c74a3201ea", [branch: "unpack-list-of-files"]}, + "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"}, From e85ddc58764a87124071ca79de250a39cf7259c8 Mon Sep 17 00:00:00 2001 From: Rudolf Manusadzhian Date: Wed, 10 Jun 2020 04:20:47 -0700 Subject: [PATCH 7/8] tweak Hex.unpack_taball considering new change to hex_core (using atom :all_files instead of empty list for unpacking all files) --- lib/diff/hex/hex.ex | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/diff/hex/hex.ex b/lib/diff/hex/hex.ex index 1a5e0aa..b236e34 100644 --- a/lib/diff/hex/hex.ex +++ b/lib/diff/hex/hex.ex @@ -38,9 +38,17 @@ defmodule Diff.Hex do end end - def unpack_tarball(tarball, file_list \\ [], path) when is_binary(path) do - path = to_charlist(path) + 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, file_list, path) do :ok From f3526ec3a45d570040d27b66b58a7811ff9ef209 Mon Sep 17 00:00:00 2001 From: RudolfMan <53276677+RudolfMan@users.noreply.github.com> Date: Wed, 10 Jun 2020 12:53:04 -0700 Subject: [PATCH 8/8] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Eric Meadows-Jönsson --- assets/js/app.js | 5 ++--- lib/diff_web/controllers/page_controller.ex | 4 +++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/assets/js/app.js b/assets/js/app.js index f4241b9..b6abd99 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -108,8 +108,7 @@ const gatherInfo = line => { } const fetchChunkAndInsert = params => { - if( !(params.fromLine && params.toLine) || - (params.fromLine >= params.toLine) ){ + if(!params.fromLine || !params.toLine || (params.fromLine >= params.toLine)){ return } @@ -120,7 +119,7 @@ const fetchChunkAndInsert = params => { fetch(url) .then(response => response.json()) .then(({chunk, lines, errors}) => { - if(errors){return} + if(errors) return const context = document.createElement('template') context.innerHTML = chunk.trim() const patchBody = params.patch.querySelector('tbody') diff --git a/lib/diff_web/controllers/page_controller.ex b/lib/diff_web/controllers/page_controller.ex index 487af3c..f194eba 100644 --- a/lib/diff_web/controllers/page_controller.ex +++ b/lib/diff_web/controllers/page_controller.ex @@ -100,7 +100,9 @@ defmodule DiffWeb.PageController do case Diff.Hex.get_chunk(chunk_extractor_params) do {:ok, chunk} -> rendered_chunk = - Phoenix.View.render_to_string(DiffWeb.RenderView, "render_context_chunk.html", + Phoenix.View.render_to_string( + DiffWeb.RenderView, + "render_context_chunk.html", chunk: chunk )