Mix task for creating component templates by Noarkhh · Pull Request #1059 · membraneframework/membrane_core

Noarkhh · 2026-01-29T14:56:14Z

No description provided.

mat-hek

I have mixed feelings about generating commented-out code. I think it would be better to use comments to mark things as optional, but no strong preference

mat-hek · 2026-02-10T09:36:04Z

-      * `inspector:` function converting fields' value to a string. Used when
-        creating documentation instead of `inspect/1`, eg. `inspector: &Membrane.Time.inspect/1`
-      * `description:` string describing an option. It will be used for generating the docs
+      * `spec:` Specification of the type the values of this option can have.


IMO this is hard to read. Typespec is a commonly known term, easily googlable too

Suggested change

* `spec:` Specification of the type the values of this option can have.

* `spec:` Typespec of the values of this option

mat-hek · 2026-02-10T09:40:38Z

+  # --------------------------
+  # --- OPTIONAL CALLBACKS ---
+  # --------------------------


Why are these callbacks marked as optional? AFAIK callbacks are optional 🤔

what do you mean? some, like handle_buffer, aren't

mat-hek · 2026-02-10T09:53:59Z

+  # -----------------
+  # --- CALLBACKS ---
+  # -----------------


Maybe it would be good to link to relevant documentation or component lifecycle page?

FelonEkonom · 2026-02-09T15:57:39Z

-      * `inspector:` function converting fields' value to a string. Used when
-        creating documentation instead of `inspect/1`, eg. `inspector: &Membrane.Time.inspect/1`
-      * `description:` string describing an option. It will be used for generating the docs
+      * `spec:` Specification of the type the values of this option can have.


maybe you could add also that it will be used in t() type

It is undone or I don't see something 😉

I think I misunderstood, done now

FelonEkonom · 2026-02-10T10:09:47Z

+      module =
+        case argv do
+          [] ->
+            Mix.raise("""
+            Module name not provided.
+
+            This task expects a module name, which the created #{unquote(component_type)} will have:
+
+              $ mix membrane.gen.#{String.downcase(unquote(component_name))} My#{unquote(component_name)}
+
+            """)
+
+            Mix.Tasks.Help
+
+          [module_name | _rest] ->
+            Module.concat([module_name])
+        end


Will it work if somebody types mix membrane.gen.sink -l some/path?
We could also check if the module name type is in CamelCase

no, it won't work. Do you think it could be nice to generate a module name from path if the module name wasn't provided? I think requiring the module name is reasonable, but it's not a strong opinion.

in theory every string can be converted to a module name with Module.concat, but when they're invalid (for example starting with lowercase letters or containing special characters) they're converted to :"Elixir.<string>" atoms, and I don't think anyone normal would want a module named like that, so I'll add validation.

FelonEkonom · 2026-02-10T10:12:49Z

+        end
+        |> then(&Path.join(base_dir, &1))
+
+      File.mkdir_p!(Path.dirname(component_path))


Will it work if component_path is a/b/c/d/file.ex and b doesn't exists?

Ok 👍 but let's use |> operator 😉

FelonEkonom · 2026-02-10T10:14:35Z

+    defp infer_path_from_module(module) do
+      module
+      |> inspect()
+      |> String.downcase()
+      |> String.split(".")
+      |> List.update_at(-1, &"#{&1}.ex")
+      |> Path.join()
+    end


Will it convert AlaMaKota into alamakota.ex or ala_ma_kota.ex? I think we want the second option

FelonEkonom · 2026-02-10T10:30:07Z

+
+  # def_options some_option: [
+  #               spec: typespec of the option,
+  #               default: default value,


This applies to other components as well

Suggested change

# default: default value,

# default: default value (if not set, this field will be required),

FelonEkonom · 2026-02-10T10:30:50Z

+  # @impl true
+  # def handle_pad_added(_pad, _context, state) do
+  #   {[], state}
+  # end
+
+  # @impl true
+  # def handle_pad_removed(_pad, _context, state) do
+  #   {[], state}
+  # end


Let's mention it will be executed only for dynamic pads

I know that I mentioned dynamic pads, but now I think we should avoid this term here. If target audience of this task are beginners, it will be more obvious for them if we will talk about pads with availability: :on_request, because they can see this field in the pad definition above. Please fix it here and in similar places.

mat-hek · 2026-02-18T11:13:00Z

+    defp infer_path_from_module(module_name) do
+      module_name
+      |> String.split(".")
+      |> Enum.map(&(Regex.replace(~r/(?<=[^A-Z])([A-Z])/, &1, "_\\1") |> String.downcase()))


Isn't it what Macro.underscore does?

BTW, there's also Macro.camelize in case you need it

wow I didn't know about these, thx

FelonEkonom · 2026-02-18T11:28:47Z

-  #   max_instances: positive integer,
+  #   accepted_format: _any
+  #   availability: :on_request | :always, # default - :always
+  #   max_instances: pos_integer() | :infinity, # relevant only for dynamic pads, default - :infinity


Hm, IMO it would be better, because beginner may not know what dynamic pad is. Please fix here and in similar places.

Suggested change

# max_instances: pos_integer() | :infinity, # relevant only for dynamic pads, default - :infinity

# max_instances: pos_integer() | :infinity, # relevant only for pads with `availability: :on_request`, default - :infinity

FelonEkonom · 2026-02-18T11:31:37Z

+  # @impl true
+  # def handle_pad_added(_pad, _context, state) do
+  #   {[], state}
+  # end
+
+  # @impl true
+  # def handle_pad_removed(_pad, _context, state) do
+  #   {[], state}
+  # end


I know that I mentioned dynamic pads, but now I think we should avoid this term here. If target audience of this task are beginners, it will be more obvious for them if we will talk about pads with availability: :on_request, because they can see this field in the pad definition above. Please fix it here and in similar places.

FelonEkonom · 2026-02-18T11:45:53Z

IMO we could add some simple tests for this task with basic assertions, jut to make sure that future changes won't break it completely. E.g. we could ensure that generated templates contain all callbacks and pad definitions.

FelonEkonom · 2026-02-23T10:48:28Z

+      Module.concat(Gen, unquote(component_type)).do_run(tmp, [inspect(unquote(component_type))])
+
+      {{:module, module, _contents, __result}, []} =
+        Code.eval_file(Path.join(tmp, Macro.underscore(unquote(component_type)) <> ".ex"))


It could be such a beautiful |> sequence (with Kernel.<>(".ex")) 😞

varsill · 2026-02-12T14:50:50Z

-      * `inspector:` function converting fields' value to a string. Used when
-        creating documentation instead of `inspect/1`, eg. `inspector: &Membrane.Time.inspect/1`
-      * `description:` string describing an option. It will be used for generating the docs
+      * `spec:` Typespec of the values this option can have.


Suggested change

* `spec:` Typespec of the values this option can have.

* `spec:` Typespec of the values this option can assume.

varsill · 2026-03-04T08:58:21Z

+  #   Membrane.Logger.warning("""
+  #   Received message but no handle_info callback has been specified. Ignoring.
+  #   Message: #{inspect(message)}\
+  #   """)


One would need to add require Membrane.Logger for this to work after it get's uncommented

varsill · 2026-03-04T09:05:30Z

+
+    ## Options
+    * `-l, --location` - If a target location is provided, the #{component_name} will be created there, relative to the `lib` directory.
+      The filename must also be present and most likely have an `.ex` extension. If location is not provided, then it will be


I don't like usage of "moste likely" here - is it supposed to have .ex extension or is it not? :D

varsill · 2026-03-04T09:07:49Z

+            Mix.raise("""
+            Module name not provided.
+
+            This task expects a module name, which the created #{unquote(component_type)} will have:


It sounds a bit odd, how about:

Suggested change

This task expects a module name, which the created #{unquote(component_type)} will have:

This task expects a module name, for the newly created #{unquote(component_type)}:

varsill · 2026-03-04T09:08:47Z

+            """)
+
+          [module_name | _rest] ->
+            Module.concat([module_name]) |> inspect()


What's the purpose of this Module.concat([module_name])? 🤔

It validates the passed string in the context of module name, e.g. passing valid module name like "Abc" will result in unchanged "Abc" string, and passing invalid module name like "123Abc" will result in ":\"Elixir.123Abc\"", which we check later

It works due to a kind of a "side effect" of how Module.concat/1 treats the strings (it runs String.to_atom() under the hood) 🤔 If we were to leave it this way, it definitelly requires a comment.

I did some research on how to determine that a String is a valid alias and that's what I found:

this is how IEx searches for a valid module names within autocomplete (it resolves alias "pieces" one by one):
https://github.com/elixir-lang/elixir/blob/d33fd8e413fe98e0e54dde7ef94fadb272b1ef67/lib/iex/lib/iex/autocomplete.ex#L511

We could also use Code.string_to_quoted() and check if the returned value is an alias:

def proper_module_name?(name) do case Code.string_to_quoted(name) do {:ok, {:__aliases__, _, _}} -> true _ -> false end end

did the second thing

varsill · 2026-03-04T09:10:26Z

+
+    @spec do_run(binary(), [binary()]) :: any()
+    def do_run(base_dir, argv) do
+      {path_option, argv} = OptionParser.parse!(argv, aliases: @aliases, strict: @switches)


Let's just call it options, as this name will get outdated if we add another option to the switches.

varsill

🥇

Noarkhh self-assigned this Jan 29, 2026

Noarkhh added this to Smackore Jan 29, 2026

Noarkhh moved this to In Progress in Smackore Jan 29, 2026

Noarkhh linked an issue Jan 29, 2026 that may be closed by this pull request

Add mix tasks for easier start with membrane #1016

Closed

Noarkhh changed the title ~~Membrane elements tasks~~ Mix task for creating component templates Jan 29, 2026

Noarkhh marked this pull request as ready for review February 4, 2026 15:24

Noarkhh requested a review from mat-hek as a code owner February 4, 2026 15:24

Noarkhh moved this from In Progress to In Review in Smackore Feb 4, 2026

Noarkhh requested review from FelonEkonom and varsill February 5, 2026 11:30

Noarkhh force-pushed the membrane-elements-tasks branch 2 times, most recently from d85fa4a to 64ee4df Compare February 5, 2026 16:15

mat-hek reviewed Feb 10, 2026

View reviewed changes

FelonEkonom requested changes Feb 10, 2026

View reviewed changes

Noarkhh requested review from FelonEkonom and mat-hek February 12, 2026 11:14

mat-hek approved these changes Feb 18, 2026

View reviewed changes

FelonEkonom reviewed Feb 18, 2026

View reviewed changes

Noarkhh force-pushed the membrane-elements-tasks branch from acc2f40 to bcd5d07 Compare February 20, 2026 10:23

Noarkhh requested a review from FelonEkonom February 20, 2026 12:09

FelonEkonom approved these changes Feb 23, 2026

View reviewed changes

varsill reviewed Mar 4, 2026

View reviewed changes

Noarkhh requested a review from varsill March 5, 2026 10:00

varsill approved these changes Mar 9, 2026

View reviewed changes

Noarkhh added 5 commits March 9, 2026 16:24

WIP

e3ba777

Add a prototype of task and templates

a6ff011

Rework task structure

4d82791

Add filled templates

05cc19b

Update changelog

9c30363

Noarkhh added 11 commits March 9, 2026 16:25

Improve docs

b87edf5

Satisfy credo

65ed0cd

Add mix app to dialyzer

a662a1a

Add membrane.gen.ex task

3406110

Add templates to the release

aef571a

Add typespecg

dd41853

Add missing callbacks and uncomment pad specs

9553ea6

Apply reviewers suggestions

b3de3e5

Apply reviewers suggestions, add tests

3cf421c

Apply reviewers suggestions

541c25f

Improve validating module names

e34bcd6

Noarkhh force-pushed the membrane-elements-tasks branch from b84e862 to e34bcd6 Compare March 9, 2026 15:29

Noarkhh merged commit 4e9b701 into master Mar 9, 2026
5 of 6 checks passed

Noarkhh deleted the membrane-elements-tasks branch March 9, 2026 15:42

github-project-automation Bot moved this from In Review to Done in Smackore Mar 9, 2026

	* `spec:` Specification of the type the values of this option can have.
	* `spec:` Typespec of the values of this option

	# default: default value,
	# default: default value (if not set, this field will be required),

	# max_instances: pos_integer() \| :infinity, # relevant only for dynamic pads, default - :infinity
	# max_instances: pos_integer() \| :infinity, # relevant only for pads with `availability: :on_request`, default - :infinity

	* `spec:` Typespec of the values this option can have.
	* `spec:` Typespec of the values this option can assume.

	This task expects a module name, which the created #{unquote(component_type)} will have:
	This task expects a module name, for the newly created #{unquote(component_type)}:

Uh oh!

Conversation

Noarkhh commented Jan 29, 2026

Uh oh!

mat-hek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mat-hek Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Noarkhh Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FelonEkonom commented Feb 18, 2026

Uh oh!

FelonEkonom Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

varsill Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mat-hek Feb 10, 2026 •

edited

Loading

Noarkhh Feb 11, 2026 •

edited

Loading

FelonEkonom Feb 23, 2026 •

edited

Loading

varsill Mar 6, 2026 •

edited

Loading