From 8dec4c0b56ab501ae7d57fb2ed3da5fcb5b59172 Mon Sep 17 00:00:00 2001 From: Pavel Shpak Date: Tue, 10 Dec 2024 15:30:53 +0200 Subject: [PATCH 1/2] Relocate check for context files generation into context generator. Invoke context files copy before parent generator files, to skip extra binding creation. It is not only skipping not used data passing in arguments, but also improve work with code as well (no need to double check if it is used data in context or not). --- lib/mix/tasks/phx.gen.context.ex | 4 ++++ lib/mix/tasks/phx.gen.html.ex | 17 ++++++----------- lib/mix/tasks/phx.gen.json.ex | 13 +++---------- lib/mix/tasks/phx.gen.live.ex | 17 +++++------------ 4 files changed, 18 insertions(+), 33 deletions(-) diff --git a/lib/mix/tasks/phx.gen.context.ex b/lib/mix/tasks/phx.gen.context.ex index 1dfad60e39..d40fd6a321 100644 --- a/lib/mix/tasks/phx.gen.context.ex +++ b/lib/mix/tasks/phx.gen.context.ex @@ -149,6 +149,8 @@ defmodule Mix.Tasks.Phx.Gen.Context do end @doc false + def files_to_be_generated(%Context{generate?: false}), do: [] + def files_to_be_generated(%Context{schema: schema}) do if schema.generate? do Gen.Schema.files_to_be_generated(schema) @@ -158,6 +160,8 @@ defmodule Mix.Tasks.Phx.Gen.Context do end @doc false + def copy_new_files(%Context{generate?: false} = context, _, _), do: context + def copy_new_files(%Context{schema: schema} = context, paths, binding) do if schema.generate?, do: Gen.Schema.copy_new_files(schema, paths, binding) inject_schema_access(context, paths, binding) diff --git a/lib/mix/tasks/phx.gen.html.ex b/lib/mix/tasks/phx.gen.html.ex index e9222dacc5..270e7507ae 100644 --- a/lib/mix/tasks/phx.gen.html.ex +++ b/lib/mix/tasks/phx.gen.html.ex @@ -107,7 +107,7 @@ defmodule Mix.Tasks.Phx.Gen.Html do {context, schema} = Gen.Context.build(args) Gen.Context.prompt_for_code_injection(context) - binding = [context: context, schema: schema, inputs: inputs(schema)] + binding = [context: context, schema: schema] paths = Mix.Phoenix.generator_paths() prompt_for_conflicts(context) @@ -120,18 +120,10 @@ defmodule Mix.Tasks.Phx.Gen.Html do defp prompt_for_conflicts(context) do context |> files_to_be_generated() - |> Kernel.++(context_files(context)) + |> Kernel.++(Gen.Context.files_to_be_generated(context)) |> Mix.Phoenix.prompt_for_conflicts() end - defp context_files(%Context{generate?: true} = context) do - Gen.Context.files_to_be_generated(context) - end - - defp context_files(%Context{generate?: false}) do - [] - end - @doc false def files_to_be_generated(%Context{schema: schema, context_app: context_app}) do singular = schema.singular @@ -157,9 +149,12 @@ defmodule Mix.Tasks.Phx.Gen.Html do @doc false def copy_new_files(%Context{} = context, paths, binding) do + Gen.Context.copy_new_files(context, paths, binding) + + binding = Keyword.merge(binding, inputs: inputs(context.schema)) files = files_to_be_generated(context) Mix.Phoenix.copy_from(paths, "priv/templates/phx.gen.html", binding, files) - if context.generate?, do: Gen.Context.copy_new_files(context, paths, binding) + context end diff --git a/lib/mix/tasks/phx.gen.json.ex b/lib/mix/tasks/phx.gen.json.ex index 97177b4935..fba1b042f3 100644 --- a/lib/mix/tasks/phx.gen.json.ex +++ b/lib/mix/tasks/phx.gen.json.ex @@ -122,18 +122,10 @@ defmodule Mix.Tasks.Phx.Gen.Json do defp prompt_for_conflicts(context) do context |> files_to_be_generated() - |> Kernel.++(context_files(context)) + |> Kernel.++(Gen.Context.files_to_be_generated(context)) |> Mix.Phoenix.prompt_for_conflicts() end - defp context_files(%Context{generate?: true} = context) do - Gen.Context.files_to_be_generated(context) - end - - defp context_files(%Context{generate?: false}) do - [] - end - @doc false def files_to_be_generated(%Context{schema: schema, context_app: context_app}) do singular = schema.singular @@ -154,9 +146,10 @@ defmodule Mix.Tasks.Phx.Gen.Json do @doc false def copy_new_files(%Context{} = context, paths, binding) do + Gen.Context.copy_new_files(context, paths, binding) + files = files_to_be_generated(context) Mix.Phoenix.copy_from(paths, "priv/templates/phx.gen.json", binding, files) - if context.generate?, do: Gen.Context.copy_new_files(context, paths, binding) context end diff --git a/lib/mix/tasks/phx.gen.live.ex b/lib/mix/tasks/phx.gen.live.ex index 3aa01cdd02..1f9f50ebb9 100644 --- a/lib/mix/tasks/phx.gen.live.ex +++ b/lib/mix/tasks/phx.gen.live.ex @@ -118,7 +118,7 @@ defmodule Mix.Tasks.Phx.Gen.Live do {context, schema} = Gen.Context.build(args) Gen.Context.prompt_for_code_injection(context) - binding = [context: context, schema: schema, inputs: inputs(schema)] + binding = [context: context, schema: schema] paths = Mix.Phoenix.generator_paths() prompt_for_conflicts(context) @@ -132,18 +132,10 @@ defmodule Mix.Tasks.Phx.Gen.Live do defp prompt_for_conflicts(context) do context |> files_to_be_generated() - |> Kernel.++(context_files(context)) + |> Kernel.++(Gen.Context.files_to_be_generated(context)) |> Mix.Phoenix.prompt_for_conflicts() end - defp context_files(%Context{generate?: true} = context) do - Gen.Context.files_to_be_generated(context) - end - - defp context_files(%Context{generate?: false}) do - [] - end - defp files_to_be_generated(%Context{schema: schema, context_app: context_app}) do web_prefix = Mix.Phoenix.web_path(context_app) test_prefix = Mix.Phoenix.web_test_path(context_app) @@ -163,18 +155,19 @@ defmodule Mix.Tasks.Phx.Gen.Live do end defp copy_new_files(%Context{} = context, binding, paths) do - files = files_to_be_generated(context) + Gen.Context.copy_new_files(context, paths, binding) binding = Keyword.merge(binding, + inputs: inputs(context.schema), assigns: %{ web_namespace: inspect(context.web_module), gettext: true } ) + files = files_to_be_generated(context) Mix.Phoenix.copy_from(paths, "priv/templates/phx.gen.live", binding, files) - if context.generate?, do: Gen.Context.copy_new_files(context, paths, binding) context end From 6a40c3658d496d47c3224ceb1d8c8244928b6bea Mon Sep 17 00:00:00 2001 From: Pavel Shpak Date: Tue, 10 Dec 2024 18:42:28 +0200 Subject: [PATCH 2/2] Apply the same improvement to schema generation check. Also: - Apply same improvement for `print_shell_instructions` function. - There were auto-formatting triggered on save for `phx.gen.schema.ex` file. --- lib/mix/tasks/phx.gen.context.ex | 17 +++------ lib/mix/tasks/phx.gen.html.ex | 2 +- lib/mix/tasks/phx.gen.json.ex | 2 +- lib/mix/tasks/phx.gen.live.ex | 2 +- lib/mix/tasks/phx.gen.schema.ex | 65 +++++++++++++++++++++++--------- 5 files changed, 56 insertions(+), 32 deletions(-) diff --git a/lib/mix/tasks/phx.gen.context.ex b/lib/mix/tasks/phx.gen.context.ex index d40fd6a321..2f7bdc81d7 100644 --- a/lib/mix/tasks/phx.gen.context.ex +++ b/lib/mix/tasks/phx.gen.context.ex @@ -152,18 +152,15 @@ defmodule Mix.Tasks.Phx.Gen.Context do def files_to_be_generated(%Context{generate?: false}), do: [] def files_to_be_generated(%Context{schema: schema}) do - if schema.generate? do - Gen.Schema.files_to_be_generated(schema) - else - [] - end + Gen.Schema.files_to_be_generated(schema) end @doc false def copy_new_files(%Context{generate?: false} = context, _, _), do: context def copy_new_files(%Context{schema: schema} = context, paths, binding) do - if schema.generate?, do: Gen.Schema.copy_new_files(schema, paths, binding) + Gen.Schema.copy_new_files(schema, paths, binding) + inject_schema_access(context, paths, binding) inject_tests(context, paths, binding) inject_test_fixture(context, paths, binding) @@ -298,12 +295,10 @@ defmodule Mix.Tasks.Phx.Gen.Context do end @doc false + def print_shell_instructions(%Context{generate?: false}), do: :ok + def print_shell_instructions(%Context{schema: schema}) do - if schema.generate? do - Gen.Schema.print_shell_instructions(schema) - else - :ok - end + Gen.Schema.print_shell_instructions(schema) end defp schema_access_template(%Context{schema: schema}) do diff --git a/lib/mix/tasks/phx.gen.html.ex b/lib/mix/tasks/phx.gen.html.ex index 270e7507ae..08eb1a6a6b 100644 --- a/lib/mix/tasks/phx.gen.html.ex +++ b/lib/mix/tasks/phx.gen.html.ex @@ -180,7 +180,7 @@ defmodule Mix.Tasks.Phx.Gen.Html do """) end - if context.generate?, do: Gen.Context.print_shell_instructions(context) + Gen.Context.print_shell_instructions(context) end @doc false diff --git a/lib/mix/tasks/phx.gen.json.ex b/lib/mix/tasks/phx.gen.json.ex index fba1b042f3..bfea745c6f 100644 --- a/lib/mix/tasks/phx.gen.json.ex +++ b/lib/mix/tasks/phx.gen.json.ex @@ -176,6 +176,6 @@ defmodule Mix.Tasks.Phx.Gen.Json do """) end - if context.generate?, do: Gen.Context.print_shell_instructions(context) + Gen.Context.print_shell_instructions(context) end end diff --git a/lib/mix/tasks/phx.gen.live.ex b/lib/mix/tasks/phx.gen.live.ex index 1f9f50ebb9..f6b9b68729 100644 --- a/lib/mix/tasks/phx.gen.live.ex +++ b/lib/mix/tasks/phx.gen.live.ex @@ -243,7 +243,7 @@ defmodule Mix.Tasks.Phx.Gen.Live do """) end - if context.generate?, do: Gen.Context.print_shell_instructions(context) + Gen.Context.print_shell_instructions(context) maybe_print_upgrade_info() end diff --git a/lib/mix/tasks/phx.gen.schema.ex b/lib/mix/tasks/phx.gen.schema.ex index 68f3c82ddf..b85227632b 100644 --- a/lib/mix/tasks/phx.gen.schema.ex +++ b/lib/mix/tasks/phx.gen.schema.ex @@ -41,7 +41,7 @@ defmodule Mix.Tasks.Phx.Gen.Schema do The following types are supported: - #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect attr}`\n"} + #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect(attr)}`\n"} * `:datetime` - An alias for `:naive_datetime` The generator also supports references, which we will properly @@ -152,14 +152,24 @@ defmodule Mix.Tasks.Phx.Gen.Schema do alias Mix.Phoenix.Schema - @switches [migration: :boolean, binary_id: :boolean, table: :string, web: :string, - context_app: :string, prefix: :string, repo: :string, migration_dir: :string, - primary_key: :string] + @switches [ + migration: :boolean, + binary_id: :boolean, + table: :string, + web: :string, + context_app: :string, + prefix: :string, + repo: :string, + migration_dir: :string, + primary_key: :string + ] @doc false def run(args) do if Mix.Project.umbrella?() do - Mix.raise "mix phx.gen.schema must be invoked from within your *_web application root directory" + Mix.raise( + "mix phx.gen.schema must be invoked from within your *_web application root directory" + ) end schema = build(args, []) @@ -201,17 +211,26 @@ defmodule Mix.Tasks.Phx.Gen.Schema do end defp put_context_app(opts, nil), do: opts + defp put_context_app(opts, string) do Keyword.put(opts, :context_app, String.to_atom(string)) end @doc false + def files_to_be_generated(%Schema{generate?: false}), do: [] + def files_to_be_generated(%Schema{} = schema) do [{:eex, "schema.ex", schema.file}] end @doc false - def copy_new_files(%Schema{context_app: ctx_app, repo: repo, opts: opts} = schema, paths, binding) do + def copy_new_files(%Schema{generate?: false} = schema, _, _), do: schema + + def copy_new_files( + %Schema{context_app: ctx_app, repo: repo, opts: opts} = schema, + paths, + binding + ) do files = files_to_be_generated(schema) Mix.Phoenix.copy_from(paths, "priv/templates/phx.gen.schema", binding, files) @@ -231,23 +250,25 @@ defmodule Mix.Tasks.Phx.Gen.Schema do migration_path = Path.join(migration_dir, "#{timestamp()}_create_#{schema.table}.exs") - Mix.Phoenix.copy_from paths, "priv/templates/phx.gen.schema", binding, [ - {:eex, "migration.exs", migration_path}, - ] + Mix.Phoenix.copy_from(paths, "priv/templates/phx.gen.schema", binding, [ + {:eex, "migration.exs", migration_path} + ]) end schema end @doc false + def print_shell_instructions(%Schema{generate?: false}), do: :ok + def print_shell_instructions(%Schema{} = schema) do if schema.migration? do - Mix.shell().info """ + Mix.shell().info(""" Remember to update your repository by running migrations: $ mix ecto.migrate - """ + """) end end @@ -255,21 +276,28 @@ defmodule Mix.Tasks.Phx.Gen.Schema do def validate_args!([schema, plural | _] = args, help) do cond do not Schema.valid?(schema) -> - help.raise_with_help "Expected the schema argument, #{inspect schema}, to be a valid module name" + help.raise_with_help( + "Expected the schema argument, #{inspect(schema)}, to be a valid module name" + ) + String.contains?(plural, ":") or plural != Phoenix.Naming.underscore(plural) -> - help.raise_with_help "Expected the plural argument, #{inspect plural}, to be all lowercase using snake_case convention" + help.raise_with_help( + "Expected the plural argument, #{inspect(plural)}, to be all lowercase using snake_case convention" + ) + true -> args end end + def validate_args!(_, help) do - help.raise_with_help "Invalid arguments" + help.raise_with_help("Invalid arguments") end @doc false - @spec raise_with_help(String.t) :: no_return() + @spec raise_with_help(String.t()) :: no_return() def raise_with_help(msg) do - Mix.raise """ + Mix.raise(""" #{msg} mix phx.gen.schema expects both a module name and @@ -277,13 +305,14 @@ defmodule Mix.Tasks.Phx.Gen.Schema do any number of attributes: mix phx.gen.schema Blog.Post blog_posts title:string - """ + """) end defp timestamp do {{y, m, d}, {hh, mm, ss}} = :calendar.universal_time() "#{y}#{pad(m)}#{pad(d)}#{pad(hh)}#{pad(mm)}#{pad(ss)}" end - defp pad(i) when i < 10, do: << ?0, ?0 + i >> + + defp pad(i) when i < 10, do: <> defp pad(i), do: to_string(i) end