From 34b5eb6fa59d9ca5b898a09c104268a6df36d2a4 Mon Sep 17 00:00:00 2001 From: Mateusz Front Date: Fri, 14 Dec 2018 20:01:52 +0100 Subject: [PATCH 1/6] added :default_formatter option parameter and Membrane.Time.pretty_duration/1 --- .../element/base/mixin/common_behaviour.ex | 7 ++++- lib/membrane/time.ex | 28 +++++++++++++++++++ test/membrane/time_test.exs | 7 +++++ 3 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 test/membrane/time_test.exs diff --git a/lib/membrane/element/base/mixin/common_behaviour.ex b/lib/membrane/element/base/mixin/common_behaviour.ex index 6804edb0c..551be49ec 100644 --- a/lib/membrane/element/base/mixin/common_behaviour.ex +++ b/lib/membrane/element/base/mixin/common_behaviour.ex @@ -189,6 +189,8 @@ defmodule Membrane.Element.Base.Mixin.CommonBehaviour do For others typespec is set to `t:any/0` * `default:` default value for option. If not present, value for this option will have to be provided each time options struct is created + * `default_formatter:` function converting value provided as `default` to string. + Used upon creating documentation instead of `inspect/1` * `description:` string describing an option. It will be present in value returned by `options/0` and in typedoc for the struct. """ @@ -203,8 +205,11 @@ defmodule Membrane.Element.Base.Mixin.CommonBehaviour do default_val_desc = if Keyword.has_key?(v, :default) do + default_formatter = + v |> Keyword.get(:default_formatter, quote(do: &"`#{inspect(&1)}`")) + quote do - "Defaults to `#{inspect(unquote(v)[:default])}`" + "Defaults to #{unquote(default_formatter).(unquote(v)[:default])}" end else "" diff --git a/lib/membrane/time.ex b/lib/membrane/time.ex index 2dd5c4802..67792e6a0 100644 --- a/lib/membrane/time.ex +++ b/lib/membrane/time.ex @@ -60,6 +60,34 @@ defmodule Membrane.Time do """ defguard is_native_t(value) when is_integer(value) + @doc """ + Returns duration as a string with unit. Chosen unit is the biggest possible one + that doesn't involve precission loss. + + ## Examples + + iex> 10 |> #{inspect(__MODULE__)}.milliseconds() |> #{inspect(__MODULE__)}.pretty_duration() + "10 ms" + iex> 60_000_000 |> #{inspect(__MODULE__)}.microseconds() |> #{inspect(__MODULE__)}.pretty_duration() + "1 min" + + """ + @spec pretty_duration(t) :: String.t() + def pretty_duration(time) when is_t(time) do + [ns: 1000, us: 1000, ms: 1000, s: 60, min: 60, h: 24] + |> Enum.reduce_while(time, fn {unit, divisor}, time -> + if time |> rem(divisor) == 0 do + {:cont, time |> div(divisor)} + else + {:halt, {time, unit}} + end + end) + |> case do + {time, unit} -> "#{time} #{unit}" + time -> "#{time} d" + end + end + @doc """ Returns current time in pretty format (currently iso8601), as string Uses system_time/0 under the hood. diff --git a/test/membrane/time_test.exs b/test/membrane/time_test.exs new file mode 100644 index 000000000..a1f381658 --- /dev/null +++ b/test/membrane/time_test.exs @@ -0,0 +1,7 @@ +defmodule Membrane.TimeTest do + use ExUnit.Case, async: true + + @module Membrane.Time + + doctest @module +end From d144b25d66aeff53898492f5e7858f7206bccb53 Mon Sep 17 00:00:00 2001 From: Mateusz Front Date: Mon, 17 Dec 2018 11:13:23 +0100 Subject: [PATCH 2/6] fixes according to review #134 --- .../element/base/mixin/common_behaviour.ex | 4 +- lib/membrane/time.ex | 38 +++++++++++-------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/lib/membrane/element/base/mixin/common_behaviour.ex b/lib/membrane/element/base/mixin/common_behaviour.ex index 551be49ec..2482e90de 100644 --- a/lib/membrane/element/base/mixin/common_behaviour.ex +++ b/lib/membrane/element/base/mixin/common_behaviour.ex @@ -189,8 +189,8 @@ defmodule Membrane.Element.Base.Mixin.CommonBehaviour do For others typespec is set to `t:any/0` * `default:` default value for option. If not present, value for this option will have to be provided each time options struct is created - * `default_formatter:` function converting value provided as `default` to string. - Used upon creating documentation instead of `inspect/1` + * `default_formatter:` function converting value provided as `default` to a string. + Used when creating documentation instead of `inspect/1` * `description:` string describing an option. It will be present in value returned by `options/0` and in typedoc for the struct. """ diff --git a/lib/membrane/time.ex b/lib/membrane/time.ex index 67792e6a0..c4930f3f0 100644 --- a/lib/membrane/time.ex +++ b/lib/membrane/time.ex @@ -61,31 +61,24 @@ defmodule Membrane.Time do defguard is_native_t(value) when is_integer(value) @doc """ - Returns duration as a string with unit. Chosen unit is the biggest possible one + Returns duration as a string with unit. Chosen unit is the biggest possible that doesn't involve precission loss. ## Examples - - iex> 10 |> #{inspect(__MODULE__)}.milliseconds() |> #{inspect(__MODULE__)}.pretty_duration() + iex> import #{inspect(__MODULE__)} + iex> 10 |> milliseconds() |> pretty_duration() "10 ms" - iex> 60_000_000 |> #{inspect(__MODULE__)}.microseconds() |> #{inspect(__MODULE__)}.pretty_duration() + iex> 60_000_000 |> microseconds() |> pretty_duration() "1 min" + iex> 2 |> nanoseconds() |> pretty_duration() + "2 ns" """ @spec pretty_duration(t) :: String.t() def pretty_duration(time) when is_t(time) do - [ns: 1000, us: 1000, ms: 1000, s: 60, min: 60, h: 24] - |> Enum.reduce_while(time, fn {unit, divisor}, time -> - if time |> rem(divisor) == 0 do - {:cont, time |> div(divisor)} - else - {:halt, {time, unit}} - end - end) - |> case do - {time, unit} -> "#{time} #{unit}" - time -> "#{time} d" - end + {unit, divisor} = units() |> Enum.find(fn {_unit, divisor} -> time |> rem(divisor) == 0 end) + + "#{time |> div(divisor)} #{unit}" end @doc """ @@ -400,4 +393,17 @@ defmodule Membrane.Time do def to_days(value) when is_t(value) do (value / (1 |> day)) |> round end + + @spec units() :: Keyword.t(t) + defp units() do + [ + d: 1 |> day(), + h: 1 |> hour(), + min: 1 |> minute(), + s: 1 |> second(), + ms: 1 |> millisecond(), + us: 1 |> microsecond(), + ns: 1 |> nanosecond() + ] + end end From d722d61b976242c758a8c16f4bc5aa5b80b6b6dc Mon Sep 17 00:00:00 2001 From: Mateusz Front Date: Wed, 19 Dec 2018 15:48:16 +0100 Subject: [PATCH 3/6] renamed default_formatter -> inspector, added time type --- .../element/base/mixin/common_behaviour.ex | 60 +++++++------------ mix.exs | 3 +- 2 files changed, 24 insertions(+), 39 deletions(-) diff --git a/lib/membrane/element/base/mixin/common_behaviour.ex b/lib/membrane/element/base/mixin/common_behaviour.ex index 2482e90de..2c87f05b3 100644 --- a/lib/membrane/element/base/mixin/common_behaviour.ex +++ b/lib/membrane/element/base/mixin/common_behaviour.ex @@ -7,10 +7,12 @@ defmodule Membrane.Element.Base.Mixin.CommonBehaviour do For more information on implementing elements, see `Membrane.Element.Base`. """ - alias Membrane.{Action, Core, Element, Event} + alias Membrane.{Action, Core, Element, Event, Time} alias Core.CallbackHandler alias Element.{Action, CallbackContext, Pad} + use Bunch + @typedoc """ Type that defines all valid return values from most callbacks. """ @@ -148,31 +150,14 @@ defmodule Membrane.Element.Base.Mixin.CommonBehaviour do """ @callback handle_shutdown(state :: Element.state_t()) :: :ok - @default_quoted_specs %{ - atom: - quote do - atom() - end, - boolean: - quote do - boolean() - end, - string: - quote do - String.t() - end, - keyword: - quote do - keyword() - end, - struct: - quote do - struct() - end, - caps: - quote do - struct() - end + @default_types_params %{ + atom: [spec: quote_expr(atom)], + boolean: [spec: quote_expr(boolean)], + string: [spec: quote_expr(String.t())], + keyword: [spec: quote_expr(keyword)], + struct: [spec: quote_expr(struct)], + caps: [spec: quote_expr(struct)], + time: [spec: quote_expr(Time.t()), inspector: &Time.pretty_duration/1] } @doc """ @@ -185,12 +170,12 @@ defmodule Membrane.Element.Base.Mixin.CommonBehaviour do * `type:` atom, used for parsing * `spec:` typespec for value in struct. If ommitted, for types: - `#{inspect(Map.keys(@default_quoted_specs))}` the default typespec is provided. + `#{inspect(Map.keys(@default_types_params))}` the default typespec is provided. For others typespec is set to `t:any/0` * `default:` default value for option. If not present, value for this option will have to be provided each time options struct is created - * `default_formatter:` function converting value provided as `default` to a string. - Used when creating documentation instead of `inspect/1` + * `inspector:` function converting fields' value to a string. Used when + creating documentation instead of `inspect/1` * `description:` string describing an option. It will be present in value returned by `options/0` and in typedoc for the struct. """ @@ -205,11 +190,15 @@ defmodule Membrane.Element.Base.Mixin.CommonBehaviour do default_val_desc = if Keyword.has_key?(v, :default) do - default_formatter = - v |> Keyword.get(:default_formatter, quote(do: &"`#{inspect(&1)}`")) + inspector = + v + |> Keyword.get( + :inspector, + @default_types_params[v[:type]][:inspector] || quote(do: &"`#{inspect(&1)}`") + ) quote do - "Defaults to #{unquote(default_formatter).(unquote(v)[:default])}" + "Defaults to #{unquote(inspector).(unquote(v)[:default])}" end else "" @@ -268,12 +257,7 @@ defmodule Membrane.Element.Base.Mixin.CommonBehaviour do with_default_specs = kw |> Enum.map(fn {k, v} -> - quoted_any = - quote do - any() - end - - default_val = @default_quoted_specs |> Map.get(v[:type], quoted_any) + default_val = @default_types_params[v[:type]][:spec] || quote_expr(any) {k, v |> Keyword.put_new(:spec, default_val)} end) diff --git a/mix.exs b/mix.exs index 578983e0e..73faf105f 100644 --- a/mix.exs +++ b/mix.exs @@ -62,7 +62,8 @@ defmodule Membrane.Mixfile do {:espec, "~> 1.6", only: :test}, {:excoveralls, "~> 0.8", only: :test}, {:qex, "~> 0.3"}, - {:bunch, "~> 0.1.2"} + # {:bunch, "~> 0.1.2"} + {:bunch, path: "../bunch"} ] end end From 98074c77e7900f8be2be1ccd8d2a37690ee7ddf2 Mon Sep 17 00:00:00 2001 From: Mateusz Front Date: Wed, 19 Dec 2018 16:54:50 +0100 Subject: [PATCH 4/6] added Time.to_code/1 and Time.to_code_str/1, used as default time inspector --- .../element/base/mixin/common_behaviour.ex | 6 +- lib/membrane/time.ex | 67 ++++++++++++++++--- 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/lib/membrane/element/base/mixin/common_behaviour.ex b/lib/membrane/element/base/mixin/common_behaviour.ex index 2c87f05b3..d49d756bb 100644 --- a/lib/membrane/element/base/mixin/common_behaviour.ex +++ b/lib/membrane/element/base/mixin/common_behaviour.ex @@ -157,7 +157,7 @@ defmodule Membrane.Element.Base.Mixin.CommonBehaviour do keyword: [spec: quote_expr(keyword)], struct: [spec: quote_expr(struct)], caps: [spec: quote_expr(struct)], - time: [spec: quote_expr(Time.t()), inspector: &Time.pretty_duration/1] + time: [spec: quote_expr(Time.t()), inspector: &Time.to_code_str/1] } @doc """ @@ -194,11 +194,11 @@ defmodule Membrane.Element.Base.Mixin.CommonBehaviour do v |> Keyword.get( :inspector, - @default_types_params[v[:type]][:inspector] || quote(do: &"`#{inspect(&1)}`") + @default_types_params[v[:type]][:inspector] || quote(do: &inspect/1) ) quote do - "Defaults to #{unquote(inspector).(unquote(v)[:default])}" + "Defaults to `#{unquote(inspector).(unquote(v)[:default])}`" end else "" diff --git a/lib/membrane/time.ex b/lib/membrane/time.ex index c4930f3f0..30cd98cf6 100644 --- a/lib/membrane/time.ex +++ b/lib/membrane/time.ex @@ -50,6 +50,16 @@ defmodule Membrane.Time do @type non_neg_t :: non_neg_integer @type native_t :: integer + @units %{ + days: :d, + hours: :h, + minutes: :min, + seconds: :s, + milliseconds: :ms, + microseconds: :us, + nanoseconds: :ns + } + @doc """ Checks whether value is Membrane.Time.t """ @@ -65,6 +75,7 @@ defmodule Membrane.Time do that doesn't involve precission loss. ## Examples + iex> import #{inspect(__MODULE__)} iex> 10 |> milliseconds() |> pretty_duration() "10 ms" @@ -76,9 +87,41 @@ defmodule Membrane.Time do """ @spec pretty_duration(t) :: String.t() def pretty_duration(time) when is_t(time) do - {unit, divisor} = units() |> Enum.find(fn {_unit, divisor} -> time |> rem(divisor) == 0 end) + {time, unit} = time |> best_unit() + + "#{time} #{@units[unit]}" + end + + @doc """ + Returns quoted code producing given amount time. Chosen unit is the biggest possible + that doesn't involve precission loss. + + ## Examples + + iex> import #{inspect(__MODULE__)} + iex> 10 |> milliseconds() |> to_code() |> Macro.to_string() + quote do 10 |> Membrane.Time.milliseconds() end |> Macro.to_string() + iex> 60_000_000 |> microseconds() |> to_code() |> Macro.to_string() + quote do 1 |> Membrane.Time.minutes() end |> Macro.to_string() + iex> 2 |> nanoseconds() |> to_code() |> Macro.to_string() + quote do 2 |> #{inspect(__MODULE__)}.nanoseconds() end |> Macro.to_string() + + """ + @spec pretty_duration(t) :: Macro.t() + def to_code(time) when is_t(time) do + {time, unit} = time |> best_unit() - "#{time |> div(divisor)} #{unit}" + quote do + unquote(time) |> unquote(__MODULE__).unquote(unit)() + end + end + + @doc """ + Returns string representation of result of `to_code/1`. + """ + @spec pretty_duration(t) :: Macro.t() + def to_code_str(time) when is_t(time) do + time |> to_code() |> Macro.to_string() end @doc """ @@ -397,13 +440,19 @@ defmodule Membrane.Time do @spec units() :: Keyword.t(t) defp units() do [ - d: 1 |> day(), - h: 1 |> hour(), - min: 1 |> minute(), - s: 1 |> second(), - ms: 1 |> millisecond(), - us: 1 |> microsecond(), - ns: 1 |> nanosecond() + days: 1 |> day(), + hours: 1 |> hour(), + minutes: 1 |> minute(), + seconds: 1 |> second(), + milliseconds: 1 |> millisecond(), + microseconds: 1 |> microsecond(), + nanoseconds: 1 |> nanosecond() ] end + + defp best_unit(time) do + {unit, divisor} = units() |> Enum.find(fn {_unit, divisor} -> time |> rem(divisor) == 0 end) + + {time |> div(divisor), unit} + end end From 4608dc01b284125927854170dda7a7366b7b16f9 Mon Sep 17 00:00:00 2001 From: Mateusz Front Date: Fri, 21 Dec 2018 14:52:03 +0100 Subject: [PATCH 5/6] small fixes and refactor in Membrane.Time --- lib/membrane/time.ex | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/membrane/time.ex b/lib/membrane/time.ex index 30cd98cf6..ee1ef4cc6 100644 --- a/lib/membrane/time.ex +++ b/lib/membrane/time.ex @@ -50,14 +50,14 @@ defmodule Membrane.Time do @type non_neg_t :: non_neg_integer @type native_t :: integer - @units %{ - days: :d, - hours: :h, - minutes: :min, - seconds: :s, - milliseconds: :ms, - microseconds: :us, - nanoseconds: :ns + @units_abbreviations %{ + days: "d", + hours: "h", + minutes: "min", + seconds: "s", + milliseconds: "ms", + microseconds: "us", + nanoseconds: "ns" } @doc """ @@ -89,7 +89,7 @@ defmodule Membrane.Time do def pretty_duration(time) when is_t(time) do {time, unit} = time |> best_unit() - "#{time} #{@units[unit]}" + "#{time} #{@units_abbreviations[unit]}" end @doc """ @@ -107,7 +107,7 @@ defmodule Membrane.Time do quote do 2 |> #{inspect(__MODULE__)}.nanoseconds() end |> Macro.to_string() """ - @spec pretty_duration(t) :: Macro.t() + @spec to_code(t) :: Macro.t() def to_code(time) when is_t(time) do {time, unit} = time |> best_unit() From 45cb2a19ab66b208be2b5b846b9763b0bd1d84a8 Mon Sep 17 00:00:00 2001 From: Mateusz Front Date: Fri, 21 Dec 2018 14:52:58 +0100 Subject: [PATCH 6/6] getting Bunch from github --- mix.exs | 2 +- mix.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mix.exs b/mix.exs index 73faf105f..1f0d815fc 100644 --- a/mix.exs +++ b/mix.exs @@ -63,7 +63,7 @@ defmodule Membrane.Mixfile do {:excoveralls, "~> 0.8", only: :test}, {:qex, "~> 0.3"}, # {:bunch, "~> 0.1.2"} - {:bunch, path: "../bunch"} + {:bunch, github: "membraneframework/bunch"} ] end end diff --git a/mix.lock b/mix.lock index 597afe102..5429b4e4b 100644 --- a/mix.lock +++ b/mix.lock @@ -1,5 +1,5 @@ %{ - "bunch": {:hex, :bunch, "0.1.2", "2389a4bcdb62382fbfeed9e19952cc22452dc0c01b4bcac941cce00ef212d7b4", [:mix], [], "hexpm"}, + "bunch": {:git, "https://github.com/membraneframework/bunch.git", "ad163a027e7dce7fedd09313cdb78edca2188604", []}, "certifi": {:hex, :certifi, "2.4.2", "75424ff0f3baaccfd34b1214184b6ef616d89e420b258bb0a5ea7d7bc628f7f0", [:rebar3], [{:parse_trans, "~>3.3", [hex: :parse_trans, repo: "hexpm", optional: false]}], "hexpm"}, "earmark": {:hex, :earmark, "1.2.6", "b6da42b3831458d3ecc57314dff3051b080b9b2be88c2e5aa41cd642a5b044ed", [:mix], [], "hexpm"}, "espec": {:hex, :espec, "1.6.3", "d9355788e508b82743a1b1b9aa5ac64ba37b0547c6210328d909e8a6eb56d42e", [:mix], [{:meck, "0.8.12", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm"},