-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature: expand context #53
base: main
Are you sure you want to change the base?
Changes from 9 commits
78f04c8
343a440
d751327
5af516a
ca61d2b
4d378f6
b884f73
50f290f
e85ddc5
f3526ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can use a map internally and only return the parsed part, we don't need the struct. |
||
|
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of having every function match on |
||
|
||
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,27 @@ defmodule DiffWeb.PageController do | |
end | ||
end | ||
|
||
def expand_context(conn, %{"file_name" => _file_name} = params) do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually since I'll revisit this part |
||
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,30 @@ 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 | ||
) | ||
RudolfMan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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 +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) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's avoid the underscored variable names. Please pick a different name or don't use
const
.