Skip to content
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

Formatter plugins #11246

Merged
merged 6 commits into from
Sep 15, 2021
Merged

Formatter plugins #11246

merged 6 commits into from
Sep 15, 2021

Conversation

josevalim
Copy link
Member

@josevalim josevalim commented Sep 14, 2021

This PR adds formatter plugins which customizes how the formatter behaves.
Plugins must implement the Mix.Tasks.Format behaviour. For example, imagine
that your project uses Markdown in two distinct ways: via a custom ~M sigil and
via files with the .md and .markdown extensions. A custom plugin would look
like this:

    defmodule MixMarkdownFormatter do
      @behaviour Mix.Tasks.Format

      def features(_opts) do
        [sigils: [:M], extensions: [".md", ".markdown"]]
      end

      def format(contents, opts) do
        # logic that formats markdown
      end
    end

Now any application can use your formatter as follows:

    # .formatters.exs
    [
      # Define the desired plugins
      plugins: [MixMarkdownFormatter],
      # Remember to update the inputs list to include the new extensions
      inputs: ["{mix,.formatter}.exs", "{config,lib,test}/**/*.{ex,exs}", "posts/*.{md,markdown}"]
    ]

Remember that, when running the formatter with plugins, you must make
sure that your dependencies and your application has been been compiled,
so the relevant plugin code can be loaded. Otherwise a warning is logged.

This can be extremely useful for projects like Phoenix LiveView and Surface,
as they can now directly hook into Elixir's formatter.

Mix.Tasks.Format.formatter_opts_for_file/2 is soft deprecated.
Mix already doesn't load unused deps, so there is
no need to run such checks and slow down the feedback
of commands such as `mix format`.
Elixir does not perform a blank loading of lib/*/ebin,
so there is actually no need to prune deps.
@paulstatezny
Copy link
Contributor

On behalf of the entire Surface community, a big thank you! This is going to scratch a common itch. 🙂

@josevalim josevalim force-pushed the jv-upsigil-formatting branch 2 times, most recently from 234dc10 to 76313df Compare September 14, 2021 15:59
@doc """
Returns which features this plugin should plug into.
"""
@callback features(Keyword.t()) :: [sigils: [atom()], extensions: [binary()]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about inputs, sources, locations, or identifiers as a replacement for features? For some reason, the term "feature" does not feel right to me 🙂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a big fan of features but I don't think any of the other ones make it justice. I thought maybe call it plugin, but I don't think it is better either. :(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! It could be plug_in.

Another alternative is to split features in two, sigils and extensions, but not a big fan of that option either :(

plugins = Keyword.get(formatter_opts, :plugins, [])

Enum.find(plugins, fn plugin ->
Code.ensure_loaded?(plugin) and function_exported?(plugin, :features, 1) and
Copy link
Member

@fertapric fertapric Sep 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this required? It seems the list of plugins is already validated in eval_deps_and_subdirectories, and not checking that here could be a perf improvement 🚀

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are validated BUT they are still kept because we only validate it when .formatter.exs is loaded but then the result of .formatter.exs is cached. So we need to look at the cached result and see if they have been loaded post caching or not.

end

formatter_opts = Keyword.put(formatter_opts, :plugins, plugins)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about moving all this logic to a new function, like validate_formatter_opts or similar?

path
end

prune_deps(config, load_paths, "--no-deps-check" in args)
Copy link
Member

@fertapric fertapric Sep 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are dep builds no longer pruned? Performance?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit message has some context but this is not necessary anymore. before we loaded everything in _build/ENV/lib/*/ebin but now we only load your deps, so we never load something we should not.

@josevalim josevalim merged commit fbaf408 into master Sep 15, 2021
@josevalim josevalim deleted the jv-upsigil-formatting branch September 15, 2021 07:45
@josevalim
Copy link
Member Author

💚 💙 💜 💛 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants