-
-
Notifications
You must be signed in to change notification settings - Fork 30
Allow reading multiple file solutions and exemploids #298
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
Changes from 7 commits
4cdb3d3
8275bd4
203e279
a371d72
cb6670f
940718d
f70197c
b26fd75
70185ff
fd2351b
27dc4ca
b0b04ce
10c9dc6
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 | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -165,19 +165,49 @@ defmodule ElixirAnalyzer do | |||||||||||||||||||
|
||||||||||||||||||||
defp do_init(params, exercise_config) do | ||||||||||||||||||||
meta_config = Path.join(params.path, @meta_config) |> File.read!() |> Jason.decode!() | ||||||||||||||||||||
relative_code_path = meta_config["files"]["solution"] |> hd() | ||||||||||||||||||||
code_path = Path.join(params.path, relative_code_path) | ||||||||||||||||||||
solution_path = meta_config["files"]["solution"] |> Enum.map(&Path.join(params.path, &1)) | ||||||||||||||||||||
if Enum.empty?(solution_path), do: raise("No solution file specified") | ||||||||||||||||||||
|
||||||||||||||||||||
code_path = | ||||||||||||||||||||
params.path | ||||||||||||||||||||
|> Path.join("lib") | ||||||||||||||||||||
|> ls_r() | ||||||||||||||||||||
|> Enum.filter(&String.ends_with?(&1, ".ex")) | ||||||||||||||||||||
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. I initially read On the other hand, I wonder if this function can be replaced by:
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. Oh, that's nice! |
||||||||||||||||||||
|> Enum.concat(solution_path) | ||||||||||||||||||||
|> Enum.uniq() | ||||||||||||||||||||
|> Enum.sort() | ||||||||||||||||||||
|
||||||||||||||||||||
editor_path = Map.get(meta_config["files"], "editor", []) | ||||||||||||||||||||
|
||||||||||||||||||||
{exercise_type, exemploid_path} = | ||||||||||||||||||||
case meta_config["files"] do | ||||||||||||||||||||
%{"exemplar" => [path | _]} -> {:concept, Path.join(params.path, path)} | ||||||||||||||||||||
%{"example" => [path | _]} -> {:practice, Path.join(params.path, path)} | ||||||||||||||||||||
%{"exemplar" => path} -> {:concept, path} | ||||||||||||||||||||
%{"example" => path} -> {:practice, path} | ||||||||||||||||||||
end | ||||||||||||||||||||
|
||||||||||||||||||||
exemploid_path = | ||||||||||||||||||||
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. The singular variable names confuse me now... may I suggest renaming?
Maybe the keys in the 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. Yes, I got lazy there :D |
||||||||||||||||||||
(editor_path ++ exemploid_path) |> Enum.sort() |> Enum.map(&Path.join(params.path, &1)) | ||||||||||||||||||||
|
||||||||||||||||||||
{code_path, exercise_type, exemploid_path, | ||||||||||||||||||||
exercise_config[:analyzer_module] || ElixirAnalyzer.TestSuite.Default} | ||||||||||||||||||||
end | ||||||||||||||||||||
|
||||||||||||||||||||
defp ls_r(path) do | ||||||||||||||||||||
cond do | ||||||||||||||||||||
File.regular?(path) -> | ||||||||||||||||||||
[path] | ||||||||||||||||||||
|
||||||||||||||||||||
File.dir?(path) -> | ||||||||||||||||||||
File.ls!(path) | ||||||||||||||||||||
|> Enum.map(&Path.join(path, &1)) | ||||||||||||||||||||
|> Enum.map(&ls_r/1) | ||||||||||||||||||||
|> Enum.concat() | ||||||||||||||||||||
|
||||||||||||||||||||
true -> | ||||||||||||||||||||
[] | ||||||||||||||||||||
end | ||||||||||||||||||||
end | ||||||||||||||||||||
|
||||||||||||||||||||
# Check | ||||||||||||||||||||
# - check if the file exists | ||||||||||||||||||||
# - read in the code | ||||||||||||||||||||
|
@@ -190,15 +220,15 @@ defmodule ElixirAnalyzer do | |||||||||||||||||||
end | ||||||||||||||||||||
|
||||||||||||||||||||
defp check(%Submission{source: source} = submission, _params) do | ||||||||||||||||||||
Logger.info("Attempting to read code file", code_file_path: source.code_path) | ||||||||||||||||||||
Logger.info("Attempting to read code files", code_file_path: source.code_path) | ||||||||||||||||||||
|
||||||||||||||||||||
with {:code_read, {:ok, code_string}} <- {:code_read, File.read(source.code_path)}, | ||||||||||||||||||||
with {:code_read, {:ok, code_string}} <- {:code_read, read_files(source.code_path)}, | ||||||||||||||||||||
source <- %{source | code_string: code_string}, | ||||||||||||||||||||
Logger.info("Code file read successfully"), | ||||||||||||||||||||
Logger.info("Code files read successfully"), | ||||||||||||||||||||
Logger.info("Attempting to read exemploid", exemploid_path: source.exemploid_path), | ||||||||||||||||||||
{:exemploid_read, _, {:ok, exemploid_string}} <- | ||||||||||||||||||||
{:exemploid_read, source, File.read(source.exemploid_path)}, | ||||||||||||||||||||
Logger.info("Exemploid file read successfully, attempting to parse"), | ||||||||||||||||||||
{:exemploid_read, source, read_files(source.exemploid_path)}, | ||||||||||||||||||||
Logger.info("Exemploid files read successfully, attempting to parse"), | ||||||||||||||||||||
{:exemploid_ast, _, {:ok, exemploid_ast}} <- | ||||||||||||||||||||
{:exemploid_ast, source, Code.string_to_quoted(exemploid_string)} do | ||||||||||||||||||||
Logger.info("Exemploid file parsed successfully") | ||||||||||||||||||||
|
@@ -238,6 +268,20 @@ defmodule ElixirAnalyzer do | |||||||||||||||||||
end | ||||||||||||||||||||
end | ||||||||||||||||||||
|
||||||||||||||||||||
defp read_files(paths) do | ||||||||||||||||||||
Enum.reduce_while( | ||||||||||||||||||||
paths, | ||||||||||||||||||||
{:ok, nil}, | ||||||||||||||||||||
fn path, {:ok, code} -> | ||||||||||||||||||||
case File.read(path) do | ||||||||||||||||||||
{:ok, file} when is_nil(code) -> {:cont, {:ok, file}} | ||||||||||||||||||||
{:ok, file} -> {:cont, {:ok, code <> "\n" <> file}} | ||||||||||||||||||||
Comment on lines
+256
to
+260
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. I assume that the case
Suggested change
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. It almost doesn't matter, but in some unlikely cases it makes a line in an error message off-by-one (see CI fail before reverted commit). Line number kind of loses meaning when you have multiple files, but after reflection, I'd still like to keep the |
||||||||||||||||||||
{:error, err} -> {:halt, {:error, err}} | ||||||||||||||||||||
end | ||||||||||||||||||||
end | ||||||||||||||||||||
) | ||||||||||||||||||||
end | ||||||||||||||||||||
|
||||||||||||||||||||
# Analyze | ||||||||||||||||||||
# - Start the static analysis | ||||||||||||||||||||
defp analyze(%Submission{halted: true} = submission, _params) do | ||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -164,7 +164,7 @@ defmodule ElixirAnalyzer.ExerciseTestCase do | |
@practice_exercise_path "elixir/exercises/practice" | ||
@meta_config ".meta/config.json" | ||
def find_source(test_module) do | ||
%Source{} | ||
%Source{code_path: []} | ||
|> find_source_slug(test_module) | ||
|> find_source_type | ||
|> find_source_exemploid_path | ||
|
@@ -192,33 +192,32 @@ defmodule ElixirAnalyzer.ExerciseTestCase do | |
end | ||
|
||
defp find_source_exemploid_path(%Source{slug: slug, exercise_type: :concept} = source) do | ||
%{"files" => %{"exemplar" => [exemploid_path | _]}} = | ||
%{"files" => %{"exemplar" => exemploid_path}} = | ||
[@concept_exercise_path, slug, @meta_config] | ||
|> Path.join() | ||
|> File.read!() | ||
|> Jason.decode!() | ||
|
||
exemploid_path = Path.join([@concept_exercise_path, slug, exemploid_path]) | ||
%{source | exemploid_path: exemploid_path} | ||
exemploid_path = Enum.map(exemploid_path, &Path.join([@concept_exercise_path, slug, &1])) | ||
%{source | exemploid_path: [exemploid_path]} | ||
end | ||
|
||
defp find_source_exemploid_path(%Source{slug: slug, exercise_type: :practice} = source) do | ||
%{"files" => %{"example" => [exemploid_path | _]}} = | ||
%{"files" => %{"example" => exemploid_path}} = | ||
[@practice_exercise_path, slug, @meta_config] | ||
|> Path.join() | ||
|> File.read!() | ||
|> Jason.decode!() | ||
|
||
exemploid_path = Path.join([@practice_exercise_path, slug, exemploid_path]) | ||
|
||
exemploid_path = Enum.map(exemploid_path, &Path.join([@practice_exercise_path, slug, &1])) | ||
%{source | exemploid_path: exemploid_path} | ||
end | ||
|
||
defp find_source_exemploid_path(source), do: source | ||
|
||
defp find_source_exemploid(%Source{exemploid_path: exemploid_path} = source) | ||
when is_binary(exemploid_path) do | ||
exemploid_string = File.read!(exemploid_path) | ||
when is_list(exemploid_path) do | ||
{:ok, exemploid_string} = read_files(exemploid_path) | ||
|
||
exemploid_ast = | ||
exemploid_string | ||
|
@@ -230,4 +229,18 @@ defmodule ElixirAnalyzer.ExerciseTestCase do | |
end | ||
|
||
defp find_source_exemploid(source), do: source | ||
|
||
defp read_files(paths) do | ||
Enum.reduce_while( | ||
paths, | ||
{:ok, nil}, | ||
fn path, {:ok, code} -> | ||
case File.read(path) do | ||
{:ok, file} when is_nil(code) -> {:cont, {:ok, file}} | ||
{:ok, file} -> {:cont, {:ok, code <> "\n" <> file}} | ||
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. This line brings the coverage down a bit, I can't test it because we don't have a multiple file exemploid. |
||
{:error, err} -> {:halt, {:error, err}} | ||
end | ||
end | ||
) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
{ | ||
"authors": [ | ||
"angelikatyborska" | ||
], | ||
"contributors": [ | ||
"jiegillet" | ||
], | ||
"files": { | ||
"solution": [ | ||
"lib/dancing_dots/animation.ex", | ||
"lib/dancing_dots/flicker.ex", | ||
"lib/dancing_dots/zoom.ex" | ||
], | ||
"test": [ | ||
"test/dancing_dots/animation_test.exs" | ||
], | ||
"exemplar": [ | ||
".meta/exemplar/animation.ex", | ||
".meta/exemplar/flicker.ex", | ||
".meta/exemplar/zoom.ex" | ||
], | ||
"editor": [ | ||
"lib/dancing_dots/dot.ex", | ||
"lib/dancing_dots/dot_group.ex" | ||
] | ||
}, | ||
"language_versions": ">=1.10", | ||
"blurb": "Learn about behaviours by writing animations for dot-based generative art." | ||
} |
Uh oh!
There was an error while loading. Please reload this page.