From 245d8603e15d7ae159375aaa8b4119e9233a53ad Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Mon, 10 Jan 2022 12:39:13 +0200 Subject: [PATCH 01/14] test: Enhance support libs --- test/support/menu_item_protocols.ex | 115 +++++++++++++++++++++++++++- test/support/xml_builder.ex | 2 + 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/test/support/menu_item_protocols.ex b/test/support/menu_item_protocols.ex index 69acbaf..d4b41de 100644 --- a/test/support/menu_item_protocols.ex +++ b/test/support/menu_item_protocols.ex @@ -1,44 +1,139 @@ defimpl ExSni.XML.Builder, for: ExSni.Menu.Item do - def encode!(item) do + def build!(item, []) do Saxy.Builder.build(item) + end + + def build!(item, opts) do + built_item = build!(item, []) + + case Keyword.get(opts, :only) do + [] -> + built_item + + keys when is_list(keys) -> + only_keys(built_item, Enum.map(keys, &Atom.to_string/1)) + + _ -> + built_item + end + end + + def encode!(item) do + item + |> build!([]) + |> Saxy.encode!() + end + + def encode!(item, opts) do + item + |> build!(opts) |> Saxy.encode!() end + + defp only_keys(elem, []) do + elem + end + + defp only_keys({tag, attrs, children}, keys) do + { + tag, + Enum.filter(attrs, fn {key, _} -> + Enum.member?(keys, key) + end), + Enum.map(children, &only_keys(&1, keys)) + } + end end defimpl ExSni.XML.Builder, for: ExSni.Menu do - def encode!(menu) do + def build!(menu, []) do Saxy.Builder.build(menu) + end + + def build!(%{root: nil} = item, _opts) do + build!(item, []) + end + + def build!(%{root: root} = item, opts) do + item + |> Map.put(root, ExSni.XML.Builder.build!(item, opts)) + |> build!([]) + end + + def encode!(menu) do + menu + |> build!([]) + |> Saxy.encode!() + end + + def encode!(menu, opts) do + menu + |> build!(opts) |> Saxy.encode!() end end defimpl ExSni.XML.Builder, for: Atom do + def build!(atom, _opts) do + atom + end + def encode!(atom) do Atom.to_string(atom) end + + def encode!(atom, _opts) do + encode!(atom) + end end defimpl ExSni.XML.Builder, for: String do + def build!(string, _opts) do + string + end + def encode!(string) do string end + + def encode!(string, _opts) do + encode!(string) + end end defimpl ExSni.XML.Builder, for: List do + def build!(list, _opts) do + list + end + def encode!(list) do - # Enum.map(list, &ExSni.XML.Builder.encode!/1) - # |> Enum.join("") Enum.map_join(list, "", &ExSni.XML.Builder.encode!/1) end + + def encode!(list, _opts) do + encode!(list) + end end defimpl ExSni.XML.Builder, for: Integer do + def build!(number, _opts) do + number + end + def encode!(number) do "#{number}" end + + def encode!(integer, _opts) do + encode!(integer) + end end defimpl ExSni.XML.Builder, for: Tuple do + def build!(tuple, _opts) do + tuple + end + def encode!({a, b}) do ExSni.XML.Builder.encode!([a, b]) end @@ -50,12 +145,24 @@ defimpl ExSni.XML.Builder, for: Tuple do def encode!({a, b, c, d}) do ExSni.XML.Builder.encode!([a, b, c, d]) end + + def encode!(tuple, _opts) do + encode!(tuple) + end end defimpl ExSni.XML.Builder, for: Any do + def build!(value, _opts) do + value + end + def encode!(value) do "#{inspect(value)}" end + + def encode!(value, _opts) do + encode!(value) + end end defimpl Saxy.Builder, for: ExSni.Menu do diff --git a/test/support/xml_builder.ex b/test/support/xml_builder.ex index 7aad457..f4c9137 100644 --- a/test/support/xml_builder.ex +++ b/test/support/xml_builder.ex @@ -1,4 +1,6 @@ defprotocol ExSni.XML.Builder do @fallback_to_any true + def build!(t, opts) def encode!(t) + def encode!(t, opts) end From 0a04241a73693442ce51030393aab3cb227e33be Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Mon, 10 Jan 2022 12:39:44 +0200 Subject: [PATCH 02/14] test: Add test for submenus last id assignment bug --- test/diff_test.exs | 104 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 1 deletion(-) diff --git a/test/diff_test.exs b/test/diff_test.exs index db40339..9d393dc 100644 --- a/test/diff_test.exs +++ b/test/diff_test.exs @@ -218,7 +218,10 @@ defmodule ExSni.MenuDiffTest do {:ok, %{old_root: old_root, new_root: new_root}} end - test "something", %{old_root: old_root, new_root: new_root} do + test "Adding menu creates the proper diff and new root", %{ + old_root: old_root, + new_root: new_root + } do {layout, updates, root} = MenuDiff.diff(new_root, old_root) assert layout == 0 @@ -228,6 +231,105 @@ defmodule ExSni.MenuDiffTest do assert String.replace(@next_menu, ~r/\n\s*/, "") == ExSni.XML.Builder.encode!(root) end + test "Identifying the last id traverses child submenus too" do + old_menu = """ + + + + + + + + + + + + + + + + + + + + + + """ + + new_menu = """ + + + + + + + + + + + + + + + + + + + + + + + + + + """ + + next_menu = """ + + + + + + + + + + + + + + + + + + + + + + + + + + """ + + old_root = build_root(old_menu) + new_root = build_root(new_menu) + + {layout, updates, root} = MenuDiff.diff(new_root, old_root) + + assert layout == 0 + assert updates == [] + + assert String.replace(old_menu, ~r/\n\s*/, "") == + ExSni.XML.Builder.encode!(old_root, only: [:id, :uid, :type, :label]) + + assert String.replace(new_menu, ~r/\n\s*/, "") == + ExSni.XML.Builder.encode!(new_root, only: [:id, :uid, :type, :label]) + + assert String.replace(next_menu, ~r/\n\s*/, "") == + ExSni.XML.Builder.encode!(root, only: [:id, :uid, :type, :label]) + end + defp build_root(source) do case Saxy.SimpleForm.parse_string(source) do {:ok, {"root", _, children}} -> From db639745097c5a8d9fa0e218979abe02e504a2ea Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Mon, 10 Jan 2022 12:40:31 +0200 Subject: [PATCH 03/14] Fix submenu last id assign bug --- lib/menu_diff.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/menu_diff.ex b/lib/menu_diff.ex index 4772df4..b4e0678 100644 --- a/lib/menu_diff.ex +++ b/lib/menu_diff.ex @@ -198,7 +198,7 @@ defmodule ExSni.MenuDiff do end defp get_last_id([node | nodes], last_id, except_nodes) do - get_last_id(nodes, get_last_node_id(node, last_id, except_nodes)) + get_last_id(nodes, get_last_id(node, last_id, except_nodes), except_nodes) end defp get_last_node_id(%{id: id}, last_id, []) do From 2b4a59d2be18c56c1475cfca7307fad3663ef06e Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Fri, 14 Jan 2022 12:34:49 +0200 Subject: [PATCH 04/14] Add telemetry dependency --- mix.exs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index 2ba8348..fb00036 100644 --- a/mix.exs +++ b/mix.exs @@ -21,7 +21,7 @@ defmodule ExSni.MixProject do # Run "mix help compile.app" to learn about applications. def application do [ - extra_applications: [:logger] + extra_applications: [:logger, :telemetry] ] end @@ -34,6 +34,7 @@ defmodule ExSni.MixProject do [ {:ex_dbus, "~> 0.1"}, {:xdiff_plus, "~> 0.1"}, + {:telemetry, "~> 1.0"}, # XML utility - ex_dbus already requires it {:saxy, "~> 1.4.0"}, From 930f23f87da68b219edc70ba89d2c4a203a848f7 Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Fri, 14 Jan 2022 12:36:02 +0200 Subject: [PATCH 05/14] Move xml builders into lib --- lib/menu.ex | 45 ++++ lib/menu/menu_item.ex | 78 +++++++ .../support => lib/protocols}/xml_builder.ex | 0 lib/xml_builder.ex | 91 ++++++++ test/support/menu_item_protocols.ex | 205 ------------------ 5 files changed, 214 insertions(+), 205 deletions(-) rename {test/support => lib/protocols}/xml_builder.ex (100%) create mode 100644 lib/xml_builder.ex delete mode 100644 test/support/menu_item_protocols.ex diff --git a/lib/menu.ex b/lib/menu.ex index f739e13..e775d74 100644 --- a/lib/menu.ex +++ b/lib/menu.ex @@ -28,6 +28,15 @@ defmodule ExSni.Menu do @dbus_menu_item_type {:struct, [:int32, {:dict, :string, :variant}, {:array, :variant}]} + @spec get_root(t() | any()) :: Item.t() | nil + def get_root(%__MODULE__{root: root}) do + root + end + + def get_root(_) do + nil + end + @spec get_layout(t(), integer(), list(String.t())) :: {:ok, list(), list()} | {:error, binary(), binary()} def get_layout(%__MODULE__{} = menu, depth, properties) do @@ -295,3 +304,39 @@ defimpl ExSni.DbusProtocol, for: ExSni.Menu do end) end end + +defimpl ExSni.XML.Builder, for: ExSni.Menu do + def build!(menu, []) do + Saxy.Builder.build(menu) + end + + def build!(%{root: nil} = item, _opts) do + build!(item, []) + end + + def build!(%{root: root} = item, opts) do + item + |> Map.put(root, ExSni.XML.Builder.build!(item, opts)) + |> build!([]) + end + + def encode!(menu) do + menu + |> build!([]) + |> Saxy.encode!() + end + + def encode!(menu, opts) do + menu + |> build!(opts) + |> Saxy.encode!() + end +end + +defimpl Saxy.Builder, for: ExSni.Menu do + import Saxy.XML + + def build(%{version: version, root: root}) do + element("dbus_menu", [version: version], [Saxy.Builder.build(root)]) + end +end diff --git a/lib/menu/menu_item.ex b/lib/menu/menu_item.ex index 9218e89..5739a42 100644 --- a/lib/menu/menu_item.ex +++ b/lib/menu/menu_item.ex @@ -568,4 +568,82 @@ defmodule ExSni.Menu.Item do cb_to_attrs_string(callbacks) end end + + defimpl ExSni.XML.Builder do + def build!(item, []) do + Saxy.Builder.build(item) + end + + def build!(item, opts) do + built_item = build!(item, []) + + case Keyword.get(opts, :only) do + [] -> + built_item + + keys when is_list(keys) -> + only_keys(built_item, Enum.map(keys, &Atom.to_string/1)) + + _ -> + built_item + end + end + + def encode!(item) do + item + |> build!([]) + |> Saxy.encode!() + end + + def encode!(item, opts) do + item + |> build!(opts) + |> Saxy.encode!() + end + + defp only_keys(elem, []) do + elem + end + + defp only_keys({tag, attrs, children}, keys) do + { + tag, + Enum.filter(attrs, fn {key, _} -> + Enum.member?(keys, key) + end), + Enum.map(children, &only_keys(&1, keys)) + } + end + end + + defimpl Saxy.Builder do + import Saxy.XML + + def build(%{type: :root, children: children} = item) do + element("root", build_attrs(item), Enum.map(children, &build/1)) + end + + def build(%{type: :menu, children: children} = item) do + element("menu", build_attrs(item), Enum.map(children, &build/1)) + end + + def build(%{children: children} = item) do + element("item", build_item_attrs(item), Enum.map(children, &build/1)) + end + + defp build_item_attrs(item) when is_map(item) do + [:id, :uid, :type, :enabled, :visible, :label, :checked] + |> build_attrs(item) + end + + defp build_attrs(item) when is_map(item) do + [:id, :uid, :enabled, :visible, :label, :checked] + |> build_attrs(item) + end + + defp build_attrs(attrs, item) when is_list(attrs) do + attrs + |> Enum.map(fn attr -> {attr, Map.get(item, attr)} end) + end + end end diff --git a/test/support/xml_builder.ex b/lib/protocols/xml_builder.ex similarity index 100% rename from test/support/xml_builder.ex rename to lib/protocols/xml_builder.ex diff --git a/lib/xml_builder.ex b/lib/xml_builder.ex new file mode 100644 index 0000000..c47a9ed --- /dev/null +++ b/lib/xml_builder.ex @@ -0,0 +1,91 @@ +defimpl ExSni.XML.Builder, for: Atom do + def build!(atom, _opts) do + atom + end + + def encode!(atom) do + Atom.to_string(atom) + end + + def encode!(atom, _opts) do + encode!(atom) + end +end + +defimpl ExSni.XML.Builder, for: String do + def build!(string, _opts) do + string + end + + def encode!(string) do + string + end + + def encode!(string, _opts) do + encode!(string) + end +end + +defimpl ExSni.XML.Builder, for: List do + def build!(list, _opts) do + list + end + + def encode!(list) do + Enum.map_join(list, "", &ExSni.XML.Builder.encode!/1) + end + + def encode!(list, _opts) do + encode!(list) + end +end + +defimpl ExSni.XML.Builder, for: Integer do + def build!(number, _opts) do + number + end + + def encode!(number) do + "#{number}" + end + + def encode!(integer, _opts) do + encode!(integer) + end +end + +defimpl ExSni.XML.Builder, for: Tuple do + def build!(tuple, _opts) do + tuple + end + + def encode!({a, b}) do + ExSni.XML.Builder.encode!([a, b]) + end + + def encode!({a, b, c}) do + ExSni.XML.Builder.encode!([a, b, c]) + end + + def encode!({a, b, c, d}) do + ExSni.XML.Builder.encode!([a, b, c, d]) + end + + def encode!(tuple, _opts) do + encode!(tuple) + end +end + +defimpl ExSni.XML.Builder, for: Any do + def build!(value, _opts) do + value + end + + def encode!(value) do + "#{inspect(value)}" + end + + def encode!(value, _opts) do + encode!(value) + end +end diff --git a/test/support/menu_item_protocols.ex b/test/support/menu_item_protocols.ex deleted file mode 100644 index d4b41de..0000000 --- a/test/support/menu_item_protocols.ex +++ /dev/null @@ -1,205 +0,0 @@ -defimpl ExSni.XML.Builder, for: ExSni.Menu.Item do - def build!(item, []) do - Saxy.Builder.build(item) - end - - def build!(item, opts) do - built_item = build!(item, []) - - case Keyword.get(opts, :only) do - [] -> - built_item - - keys when is_list(keys) -> - only_keys(built_item, Enum.map(keys, &Atom.to_string/1)) - - _ -> - built_item - end - end - - def encode!(item) do - item - |> build!([]) - |> Saxy.encode!() - end - - def encode!(item, opts) do - item - |> build!(opts) - |> Saxy.encode!() - end - - defp only_keys(elem, []) do - elem - end - - defp only_keys({tag, attrs, children}, keys) do - { - tag, - Enum.filter(attrs, fn {key, _} -> - Enum.member?(keys, key) - end), - Enum.map(children, &only_keys(&1, keys)) - } - end -end - -defimpl ExSni.XML.Builder, for: ExSni.Menu do - def build!(menu, []) do - Saxy.Builder.build(menu) - end - - def build!(%{root: nil} = item, _opts) do - build!(item, []) - end - - def build!(%{root: root} = item, opts) do - item - |> Map.put(root, ExSni.XML.Builder.build!(item, opts)) - |> build!([]) - end - - def encode!(menu) do - menu - |> build!([]) - |> Saxy.encode!() - end - - def encode!(menu, opts) do - menu - |> build!(opts) - |> Saxy.encode!() - end -end - -defimpl ExSni.XML.Builder, for: Atom do - def build!(atom, _opts) do - atom - end - - def encode!(atom) do - Atom.to_string(atom) - end - - def encode!(atom, _opts) do - encode!(atom) - end -end - -defimpl ExSni.XML.Builder, for: String do - def build!(string, _opts) do - string - end - - def encode!(string) do - string - end - - def encode!(string, _opts) do - encode!(string) - end -end - -defimpl ExSni.XML.Builder, for: List do - def build!(list, _opts) do - list - end - - def encode!(list) do - Enum.map_join(list, "", &ExSni.XML.Builder.encode!/1) - end - - def encode!(list, _opts) do - encode!(list) - end -end - -defimpl ExSni.XML.Builder, for: Integer do - def build!(number, _opts) do - number - end - - def encode!(number) do - "#{number}" - end - - def encode!(integer, _opts) do - encode!(integer) - end -end - -defimpl ExSni.XML.Builder, for: Tuple do - def build!(tuple, _opts) do - tuple - end - - def encode!({a, b}) do - ExSni.XML.Builder.encode!([a, b]) - end - - def encode!({a, b, c}) do - ExSni.XML.Builder.encode!([a, b, c]) - end - - def encode!({a, b, c, d}) do - ExSni.XML.Builder.encode!([a, b, c, d]) - end - - def encode!(tuple, _opts) do - encode!(tuple) - end -end - -defimpl ExSni.XML.Builder, for: Any do - def build!(value, _opts) do - value - end - - def encode!(value) do - "#{inspect(value)}" - end - - def encode!(value, _opts) do - encode!(value) - end -end - -defimpl Saxy.Builder, for: ExSni.Menu do - import Saxy.XML - - def build(%{version: version, root: root}) do - element("dbus_menu", [version: version], [Saxy.Builder.build(root)]) - end -end - -defimpl Saxy.Builder, for: ExSni.Menu.Item do - import Saxy.XML - - def build(%{type: :root, children: children} = item) do - element("root", build_attrs(item), Enum.map(children, &build/1)) - end - - def build(%{type: :menu, children: children} = item) do - element("menu", build_attrs(item), Enum.map(children, &build/1)) - end - - def build(%{children: children} = item) do - element("item", build_item_attrs(item), Enum.map(children, &build/1)) - end - - defp build_item_attrs(item) when is_map(item) do - [:id, :uid, :type, :enabled, :visible, :label, :checked] - |> build_attrs(item) - end - - defp build_attrs(item) when is_map(item) do - [:id, :uid, :enabled, :visible, :label, :checked] - |> build_attrs(item) - end - - defp build_attrs(attrs, item) when is_list(attrs) do - attrs - |> Enum.map(fn attr -> {attr, Map.get(item, attr)} end) - end -end From 4eedf36c40f6bbdc0509fff4722a31ee290b2b7b Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Fri, 14 Jan 2022 12:36:17 +0200 Subject: [PATCH 06/14] Add Menu.Debug module --- lib/menu/debug.ex | 82 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 lib/menu/debug.ex diff --git a/lib/menu/debug.ex b/lib/menu/debug.ex new file mode 100644 index 0000000..23b388e --- /dev/null +++ b/lib/menu/debug.ex @@ -0,0 +1,82 @@ +defmodule ExSni.Menu.Debug do + def parse_layout_response({:ok, _, [version, [0, [], []]]}) do + {"layout", [{"version", "#{version}"}], []} + end + + def parse_layout_response({:ok, _, [version, root_node]}) do + {"layout", [{"version", "#{version}"}], [parse_layout_item(root_node)]} + end + + def parse_layout_response({:error, type, reason}) do + {:error, "#{type}: #{reason}"} + end + + def parse_signal_layout_updated([version, item_id]) do + {"signal", [{"name", "LayoutUpdated"}, {"version", "#{version}"}, {"id", "#{item_id}"}], []} + end + + def parse_signal_items_properties_updated([updated, removed]) do + {"signal", [{"name", "ItemPropertiesUpdated"}], + [ + {"updated", [], parse_group_items(updated)}, + {"removed", [], parse_group_items(removed)} + ]} + end + + def parse_get_group_properties([]) do + {"group_properties", [], []} + end + + def parse_get_group_properties(items) do + {"group_properties", [], parse_group_items(items)} + end + + defp parse_group_items(items) do + items + |> Enum.map(fn [id, prop_list] -> + {"item", [{"id", "#{id}"} | parse_group_item_properties(prop_list)], []} + end) + end + + defp parse_group_item_properties(properties) do + properties + |> Enum.map(fn {name, {:dbus_variant, _type, value}} -> + {name, "#{value}"} + end) + end + + def parse_layout_item({id, properties, children}) do + properties = parse_properties(properties) + + children = + Enum.map(children, fn {:dbus_variant, _, child} -> + parse_layout_item(child) + end) + + case properties["children-display"] do + "submenu" -> + {"menu", [{"id", "#{id}"}], children} + + _ -> + if properties["type"] == "separator" do + {"hr", [], []} + else + {"item", [{"id", "#{id}"}], []} + end + end + end + + def debug_root(nil) do + "" + end + + def debug_root(root) do + ExSni.XML.Builder.encode!(root, only: [:id, :label]) + end + + defp parse_properties(properties) do + Enum.reduce(properties, %{}, fn {key, {:dbus_variant, _type, value}}, acc -> + Map.put(acc, key, "#{value}") + end) + end +end From fea49ada4857d2ba09d978e6a959a40af662e586 Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Fri, 14 Jan 2022 12:40:58 +0200 Subject: [PATCH 07/14] Integrate telemetry events --- lib/ex_sni.ex | 3 + lib/menu/server.ex | 103 +++++++++++++++++++- lib/menu_diff.ex | 234 +++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 329 insertions(+), 11 deletions(-) diff --git a/lib/ex_sni.ex b/lib/ex_sni.ex index 6319c0c..04cb5d0 100644 --- a/lib/ex_sni.ex +++ b/lib/ex_sni.ex @@ -3,6 +3,7 @@ defmodule ExSni do alias ExSni.Bus alias ExSni.{Icon, Menu} + alias ExSni.Debugger def start_link(init_opts \\ [], start_opts \\ []) do Supervisor.start_link(__MODULE__, init_opts, start_opts) @@ -22,6 +23,8 @@ defmodule ExSni do menu: menu_server_pid } + Debugger.init() + children = [ {Registry, keys: :unique, name: ExSniRegistry}, %{ diff --git a/lib/menu/server.ex b/lib/menu/server.ex index c3b18da..0e0a805 100644 --- a/lib/menu/server.ex +++ b/lib/menu/server.ex @@ -193,12 +193,30 @@ defmodule ExSni.Menu.Server do # Menu queue is empty, so current menu is still valid # Return that we want to skip any dbus menu updates defp do_menu_update(%{menu_queue: []} = state) do + :telemetry.execute([:ex_sni, :do_menu_update], %{}, %{ + branch: 0, + description: "Menu queue is empty, so current menu is still valid", + menu_queue: :empty, + returns: :skip, + new_root: nil, + old_root: Menu.get_root(state.menu) + }) + {:skip, state} end # Current menu is nil and a reset is queued # Skip the reset and try again defp do_menu_update(%{menu: %Menu{root: nil}, menu_queue: [:reset | menu_queue]} = state) do + :telemetry.execute([:ex_sni, :do_menu_update], %{}, %{ + branch: 1, + description: "Current menu is nil and reset is queued. Skip the reset and try again", + menu_queue: :reset, + returns: :rerun, + new_root: :reset, + old_root: nil + }) + state |> set_menu_queue(menu_queue) |> do_menu_update() @@ -213,6 +231,16 @@ defmodule ExSni.Menu.Server do # Increment the version of an empty menu menu = %{old_menu | root: nil, version: old_version + 1} + :telemetry.execute([:ex_sni, :do_menu_update], %{}, %{ + branch: 2, + description: + "There is a reset pending, and the current menu is not empty. Action on the reset", + menu_queue: :reset, + returns: :trigger_layout_update_signal, + new_root: nil, + old_root: Menu.get_root(old_menu) + }) + # Update the queue # Send a LayoutUpdated signal, so that D-Bus can fetch the new empty menu @@ -229,8 +257,26 @@ defmodule ExSni.Menu.Server do state ) do if menu_empty?(new_menu) do + :telemetry.execute([:ex_sni, :do_menu_update], %{}, %{ + branch: 3, + description: "Current menu is nil, no reset pending and full new empty menu", + menu_queue: :empty, + returns: :skip, + new_root: nil, + old_root: nil + }) + {:skip, set_menu_queue(state, [])} else + :telemetry.execute([:ex_sni, :do_menu_update], %{}, %{ + branch: 3, + description: "Current menu is nil, no reset pending and full new non-empty menu", + menu_queue: :empty, + returns: :trigger_layout_update_signale, + new_root: Menu.get_root(new_menu), + old_root: nil + }) + menu = %{new_menu | version: old_version + 1} # Update the queue # Send a LayoutUpdated signal, so that D-Bus can fetch the new empty menu @@ -249,6 +295,15 @@ defmodule ExSni.Menu.Server do menu_queue: [%Menu{root: new_root} = new_menu] } = state ) do + :telemetry.execute([:ex_sni, :do_menu_update], %{}, %{ + branch: 4, + description: "No reset pending, just a possible menu update", + menu_queue: :new_root, + returns: :menu_diff_result, + new_root: new_root, + old_root: old_root + }) + menu_diff = MenuDiff.diff(new_root, old_root) case menu_diff do @@ -397,10 +452,20 @@ defmodule ExSni.Menu.Server do end defp send_dbus_signal(service_pid, "LayoutUpdated" = signal, args) do + :telemetry.execute([:ex_sni, :send_dbus_signal], %{}, %{ + signal: "LayoutUpdated", + args: args + }) + service_send_signal(service_pid, signal, {"ui", [:uint32, :int32], args}) end defp send_dbus_signal(service_pid, "ItemsPropertiesUpdated" = signal, args) do + :telemetry.execute([:ex_sni, :send_dbus_signal], %{}, %{ + signal: "ItemsPropertiesUpdated", + args: args + }) + service_send_signal(service_pid, signal, { "a(ia{sv})a(ias)", [ @@ -413,7 +478,6 @@ defmodule ExSni.Menu.Server do defp service_send_signal(service_pid, signal, args) do # IO.inspect([signal, args], label: "Sending D-Bus signal", limit: :infinity) - ExDBus.Service.send_signal( service_pid, "/MenuBar", @@ -432,6 +496,11 @@ defmodule ExSni.Menu.Server do %{menu: %Menu{version: version} = menu, items_properties_updated_queue: ipu_queue} = state ) do + :telemetry.execute([:ex_sni, :dbus_method, :call], %{}, %{ + method: "GetLayout", + args: {parentId, depth, properties} + }) + {ipu_item, ipu_queue} = fetch_items_properties_updated(version, ipu_queue) case ipu_item do @@ -448,6 +517,11 @@ defmodule ExSni.Menu.Server do end defp handle_method("GetGroupProperties", {ids, properties}, _from, %{menu: menu} = state) do + :telemetry.execute([:ex_sni, :dbus_method, :call], %{}, %{ + method: "GetGroupProperties", + args: {ids, properties} + }) + state = %{state | get_group_properties: {time(state), menu}} {:reply, method_reply("GetGroupProperties", {ids, properties}, menu), state} end @@ -492,7 +566,15 @@ defmodule ExSni.Menu.Server do # label: "[#{System.os_time(:millisecond)}] [ExSni][Menu.Server] GetLayout" # ) - Menu.get_layout(menu, parentId, depth, properties) + result = Menu.get_layout(menu, parentId, depth, properties) + + :telemetry.execute([:ex_sni, :dbus_method, :reply], %{}, %{ + method: "GetLayout", + request: {parentId, depth, properties}, + response: result + }) + + result # |> IO.inspect(label: "GetLayout reply", limit: :infinity) end @@ -503,7 +585,11 @@ defmodule ExSni.Menu.Server do result = Menu.get_group_properties(menu, ids, properties) - # |> IO.inspect(label: "GetGroupProperties reply", limit: :infinity) + :telemetry.execute([:ex_sni, :dbus_method, :reply], %{}, %{ + method: "GetGroupProperties", + request: {ids, properties}, + response: result + }) {:ok, [{:array, {:struct, [:int32, {:dict, :string, :variant}]}}], [result]} end @@ -686,6 +772,11 @@ defmodule ExSni.Menu.Server do end defp set_current_menu(state, menu) do + :telemetry.execute([:ex_sni, :set_current_menu], %{}, %{ + menu: menu, + root: Menu.get_root(menu) + }) + backup_current_menu(%{state | menu: menu}) end @@ -717,6 +808,12 @@ defmodule ExSni.Menu.Server do end defp set_menu_queue(state, queue) do + :telemetry.execute([:ex_sni, :set_menu_queue], %{}, %{ + current_menu: state.menu, + old_menu_queue: state.menu_queue, + new_menu_queue: queue + }) + %{state | menu_queue: queue} end diff --git a/lib/menu_diff.ex b/lib/menu_diff.ex index b4e0678..94968b4 100644 --- a/lib/menu_diff.ex +++ b/lib/menu_diff.ex @@ -20,43 +20,141 @@ defmodule ExSni.MenuDiff do case {inserts, deletes, updates, nops} do {[], [], [], []} -> - # IO.inspect("{[], [], [], []}", label: "[DIFF::[1]]") # [x] Implemented # Nothing to update. Both new and old menus are empty # No layout changes # No items to signal updated properties for + + :telemetry.execute([:ex_sni, :menu_diff, :branch], %{}, %{ + branch: 1, + description: "Nothing to update. Both new and old menus are empty.", + layout_changes: :no, + items_updated_properties_signal: false, + params: %{ + inserts: [], + deletes: [], + updates: [], + nops: [] + }, + new_root: new_root, + old_root: old_root + }) + + # IO.inspect("{[], [], [], []}", label: "[DIFF::[1]]") + + :telemetry.execute([:ex_sni, :menu_diff, :result], %{}, %{ + branch: 1, + description: "Nothing to update. Both new and old menus are empty.", + layout_changes: :no, + items_updated_properties_signal: false, + code: -1, + group_properties: [], + node: old_root + }) + {-1, [], old_root} {[], [], [], _unchanged} -> - # IO.inspect("{[], [], [], _unchanged}", label: "[DIFF::[2]]") # [x] Implemented # Nothing to update. Both menus are equal # No layout changes # No items to signal updated properties for + + :telemetry.execute([:ex_sni, :menu_diff, :branch], %{}, %{ + branch: 2, + description: "Nothing to update. Both menus are equal.", + layout_changes: :no, + items_updated_properties_signal: false, + params: %{ + inserts: [], + deletes: [], + updates: [], + nops: :ignored + }, + new_root: new_root, + old_root: old_root + }) + + # IO.inspect("{[], [], [], _unchanged}", label: "[DIFF::[2]]") + + :telemetry.execute([:ex_sni, :menu_diff, :result], %{}, %{ + branch: 2, + description: "Nothing to update. Both menus are equal.", + layout_changes: :no, + items_updated_properties_signal: false, + code: -1, + group_properties: [], + node: old_root + }) + {-1, [], old_root} {_inserts, _deletes, [], []} -> - # IO.inspect("{_inserts, _deletes, [], []}", label: "[DIFF::[3]]") # [x] Implemented # If there are inserts or deletes, but there are no updates and no unchanged # then this is a completely different new menu # Full layout change # Signal property changes for all items in the new menu + :telemetry.execute([:ex_sni, :menu_diff, :branch], %{}, %{ + branch: 3, + description: "Completely new menu.", + layout_changes: :full, + items_updated_properties_signal: false, + params: %{ + inserts: :ignored, + deletes: :ignored, + updates: [], + nops: [] + }, + new_root: new_root, + old_root: old_root + }) + + # IO.inspect("{_inserts, _deletes, [], []}", label: "[DIFF::[3]]") + {node, _last_id, _new_ids_map} = assign_ids(new_root, %{}, 0) # This should be signaled as a menu reset and then switching to this new menu # {0, [], node} + :telemetry.execute([:ex_sni, :menu_diff, :result], %{}, %{ + branch: 3, + description: "Completely new menu.", + layout_changes: :full, + items_updated_properties_signal: false, + code: -2, + group_properties: [], + node: node + }) + {-2, [], node} {[], [], updates, []} -> - # IO.inspect("{[], [], updates, []}", label: "[DIFF::[4]]") # [x] Implemented # All items in the old menu have changes. # Return the new menu, but assign old menu IDs to all the new items # No layout changes # Signal property changes for all updated items + :telemetry.execute([:ex_sni, :menu_diff, :branch], %{}, %{ + branch: 4, + description: + "All items in old menu have changes. " <> + "Return the new menu, but assign old menu IDs to all the new items.", + layout_changes: :no, + items_updated_properties_signal: true, + params: %{ + inserts: [], + deletes: [], + updates: updates, + nops: [] + }, + new_root: new_root, + old_root: old_root + }) + + # IO.inspect("{[], [], updates, []}", label: "[DIFF::[4]]") + last_id = get_last_id(old_root, 0) {node, _last_id, _new_ids_map} = assign_ids(new_root, mapping, last_id + 1) @@ -66,16 +164,46 @@ defmodule ExSni.MenuDiff do [[id, Item.get_dbus_changed_properties(new_node, old_node, :ignore_default)] | acc] end) + :telemetry.execute([:ex_sni, :menu_diff, :result], %{}, %{ + branch: 4, + description: + "All items in old menu have changes. " <> + "Return the new menu, but assign old menu IDs to all the new items.", + layout_changes: :no, + items_updated_properties_signal: true, + code: -1, + group_properties: group_properties, + node: node + }) + {-1, group_properties, node} {[], [], updates, _unchanged} -> - # IO.inspect("{[], [], updates, _unchanged}", label: "[DIFF::[5]]") # [x] Implemented # This is a partial update of the menu. # Return the new menu, and copy old menu IDs to the updated items # No layout changes # Signal property changes for some updated items + :telemetry.execute([:ex_sni, :menu_diff, :branch], %{}, %{ + branch: 5, + description: + "Partial update of old menu. " <> + "Return the new menu, but assign old menu IDs to all the new items.", + layout_changes: :no, + items_updated_properties_signal: true, + params: %{ + inserts: [], + deletes: [], + updates: updates, + nops: :unchanged + }, + new_root: new_root, + old_root: old_root + }) + + # IO.inspect("{[], [], updates, _unchanged}", label: "[DIFF::[5]]") + last_id = get_last_id(old_root, 0) {node, _last_id, _new_ids_map} = assign_ids(new_root, mapping, last_id + 1) @@ -87,10 +215,21 @@ defmodule ExSni.MenuDiff do # {updated_ids, menu} = todo() # {-1, updated_ids, menu} + :telemetry.execute([:ex_sni, :menu_diff, :result], %{}, %{ + branch: 5, + description: + "Partial update of old menu. " <> + "Return the new menu, but assign old menu IDs to all the new items.", + layout_changes: :no, + items_updated_properties_signal: true, + code: -1, + group_properties: group_properties, + node: node + }) + {-1, group_properties, node} {_inserts, deletes, updates, []} -> - # IO.inspect("{inserts, deletes, updates, []}", label: "[DIFF::[6]]") # [x] Implemented # This is updates to all items in the menu and some deletes and/or some inserts # Some layout changes @@ -98,6 +237,23 @@ defmodule ExSni.MenuDiff do # but libdbusmenu ignores it # Signal property changes for all updated items + # IO.inspect("{inserts, deletes, updates, []}", label: "[DIFF::[6]]") + + :telemetry.execute([:ex_sni, :menu_diff, :branch], %{}, %{ + branch: 6, + description: "Updates to all items, with deletes and inserts.", + layout_changes: :some, + items_updated_properties_signal: true, + params: %{ + inserts: :ignored, + deletes: deletes, + updates: updates, + nops: [] + }, + new_root: new_root, + old_root: old_root + }) + # Copy old ids from to the new menu from the old menu # and assign new ids for the inserts. delete_nodes = @@ -119,10 +275,19 @@ defmodule ExSni.MenuDiff do [[id, Item.get_dbus_changed_properties(new_node, old_node, :ignore_default)] | acc] end) + :telemetry.execute([:ex_sni, :menu_diff, :result], %{}, %{ + branch: 6, + description: "Updates to all items, with deletes and inserts.", + layout_changes: :some, + items_updated_properties_signal: true, + code: 0, + group_properties: group_properties, + node: node + }) + {0, group_properties, node} {_inserts, deletes, [], _unchanged} -> - # IO.inspect("{inserts, deletes, [], unchanged}", label: "[DIFF::[7]]") # [x] Implemented # This is only a layout change where items have been removed from the old menu # or added in the new menu. @@ -131,6 +296,23 @@ defmodule ExSni.MenuDiff do # but libdbusmenu ignores it # No items to signal updated properties for + # IO.inspect("{inserts, deletes, [], unchanged}", label: "[DIFF::[7]]") + + :telemetry.execute([:ex_sni, :menu_diff, :branch], %{}, %{ + branch: 7, + description: "Menu items removed or new items inserted.", + layout_changes: :some, + items_updated_properties_signal: false, + params: %{ + inserts: :ignored, + deletes: deletes, + updates: [], + nops: :ignored + }, + new_root: new_root, + old_root: old_root + }) + delete_nodes = Enum.reduce(deletes, [], fn {node, _}, acc -> [node | acc] @@ -145,10 +327,19 @@ defmodule ExSni.MenuDiff do {node, _last_id, _new_ids_map} = assign_ids(new_root, mapping, last_id + 1) + :telemetry.execute([:ex_sni, :menu_diff, :result], %{}, %{ + branch: 7, + description: "Menu items removed or new items inserted.", + layout_changes: :some, + items_updated_properties_signal: false, + code: 0, + group_properties: [], + node: node + }) + {0, [], node} {_inserts, deletes, updates, _unchanged} -> - # IO.inspect("{inserts, deletes, updates, unchanged}", label: "[DIFF::[8]]") # [x] Implemented # There are mixed changes in our menus: # inserts and/or deletes, updates but also unchanged items @@ -157,6 +348,23 @@ defmodule ExSni.MenuDiff do # but libdbusmenu ignores it # Signal property changes for some updated items + # IO.inspect("{inserts, deletes, updates, unchanged}", label: "[DIFF::[8]]") + + :telemetry.execute([:ex_sni, :menu_diff, :branch], %{}, %{ + branch: 8, + description: "Mixed changes: inserts and/or deletes, updates but also unchanged items.", + layout_changes: :some, + items_updated_properties_signal: true, + params: %{ + inserts: :ignored, + deletes: deletes, + updates: updates, + nops: :ignored + }, + new_root: new_root, + old_root: old_root + }) + # Copy old ids from to the new menu from the old menu # and assign new ids for the inserts. delete_nodes = @@ -178,6 +386,16 @@ defmodule ExSni.MenuDiff do [[id, Item.get_dbus_changed_properties(new_node, old_node, :ignore_default)] | acc] end) + :telemetry.execute([:ex_sni, :menu_diff, :result], %{}, %{ + branch: 8, + description: "Mixed changes: inserts and/or deletes, updates but also unchanged items.", + layout_changes: :some, + items_updated_properties_signal: true, + code: 0, + group_properties: group_properties, + node: node + }) + {0, group_properties, node} end end From 724bb95df3c9746b5748672ea6e2b9684078044f Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Fri, 14 Jan 2022 12:41:14 +0200 Subject: [PATCH 08/14] Adds Debugger to attach to telemetry events --- lib/debugger.ex | 349 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 349 insertions(+) create mode 100644 lib/debugger.ex diff --git a/lib/debugger.ex b/lib/debugger.ex new file mode 100644 index 0000000..5611a88 --- /dev/null +++ b/lib/debugger.ex @@ -0,0 +1,349 @@ +defmodule ExSni.Debugger do + @module """ + Debugger module that attaches to telemetry events. + Configurable using the EXSNI_DEBUG environment variable, with the following events, + given as a comma-separated list: + - menu_diff - Outputs the old menu structure, the new menu structure, and the diff result + - menu_update - Outputs branch and menu update structures + - menu_set - Outputs the structure that is set as the current menu + - dbus_method - Outputs D-Bus method calls and replies + - dbus_signal - Outputs D-Bus signals sent + - menu - alias for "menu_diff,menu_update,menu_set" (all menu events) + - dbus - alias for "dbus_method,dbus_signal" (all dbus events) + - all - alias for "menu,dbus" (all events) + """ + require Logger + + @debug_env_name "EXSNI_DEBUG" + + def init(_opts \\ []) do + get_debug_options() + |> Enum.each(fn metric -> + unique_id = + metric + |> Enum.map(&Atom.to_string/1) + |> Enum.join("_") + + :telemetry.attach( + unique_id, + metric, + &handle_event/4, + nil + ) + end) + end + + # Menu Diff telemetry + def handle_event( + [:ex_sni, :menu_diff, :branch], + _metrics, + %{ + branch: branch, + description: description, + layout_changes: layout_changes, + items_updated_properties_signal: item_updates, + params: _params, + new_root: new_root, + old_root: old_root + }, + _config + ) do + xml_new_root = ExSni.Menu.Debug.debug_root(new_root) + xml_old_root = ExSni.Menu.Debug.debug_root(old_root) + + {"ex_sni", [{"timestamp", "#{now()}"}], + [ + {"menu_diff", + [ + {"branch", "#{branch}"}, + {"layout_changes", "#{inspect(layout_changes)}"}, + {"item_updates", "#{inspect(item_updates)}"}, + {"description", "#{description}"} + ], + [ + "#{xml_old_root}#{xml_new_root}" + ]} + ]} + |> Saxy.encode!() + |> _debug() + end + + def handle_event( + [:ex_sni, :menu_diff, :result], + _metrics, + %{ + branch: branch, + description: description, + layout_changes: layout_changes, + items_updated_properties_signal: item_updates, + code: code, + node: node + }, + _config + ) do + xml_node = ExSni.Menu.Debug.debug_root(node) + + {"ex_sni", [{"timestamp", "#{now()}"}], + [ + {"menu_diff_result", + [ + {"branch", "#{branch}"}, + {"code", "#{code}"}, + {"layout_changes", "#{inspect(layout_changes)}"}, + {"item_updates", "#{inspect(item_updates)}"}, + {"description", "#{description}"} + ], + [ + "#{xml_node}" + ]} + ]} + |> Saxy.encode!() + |> _debug() + end + + # ExSni.Menu.Server metrics + + def handle_event( + [:ex_sni, :do_menu_update], + _metrics, + %{ + branch: branch, + description: description, + menu_queue: menu_queue, + returns: returns, + new_root: new_root, + old_root: old_root + }, + _config + ) do + xml_new_root = + if is_atom(new_root) do + Atom.to_string(new_root) + else + ExSni.Menu.Debug.debug_root(new_root) + end + + xml_old_root = ExSni.Menu.Debug.debug_root(old_root) + + {"ex_sni", [{"timestamp", "#{now()}"}], + [ + {"do_menu_update", + [ + {"branch", "#{branch}"}, + {"menu_queue", "#{inspect(menu_queue)}"}, + {"returns", "#{inspect(returns)}"}, + {"description", "#{description}"} + ], + [ + "#{xml_old_root}#{xml_new_root}" + ]} + ]} + |> Saxy.encode!() + |> _debug() + end + + def handle_event( + [:ex_sni, :send_dbus_signal], + _metrics, + %{signal: "LayoutUpdated", args: args}, + _config + ) do + xml_updates = + args + |> ExSni.Menu.Debug.parse_signal_layout_updated() + |> Saxy.encode!() + + {"dbus", [{"timestamp", "#{now()}"}], + [ + "#{xml_updates}" + ]} + |> Saxy.encode!() + |> _debug() + end + + def handle_event( + [:ex_sni, :send_dbus_signal], + _metrics, + %{signal: "ItemsPropertiesUpdated", args: args}, + _config + ) do + xml_updates = + args + |> ExSni.Menu.Debug.parse_signal_items_properties_updated() + |> Saxy.encode!() + + {"dbus", [{"timestamp", "#{now()}"}], + [ + "#{xml_updates}" + ]} + |> Saxy.encode!() + |> _debug() + end + + def handle_event( + [:ex_sni, :dbus_method, :call], + _metrics, + %{method: "GetLayout", args: {id, depth, properties}}, + _config + ) do + xml_item = + {"get_layout", + [{"item_id", "#{id}"}, {"depth", "#{depth}"}, {"properties", Enum.join(properties, ",")}], + []} + |> Saxy.encode!() + + {"dbus", [{"timestamp", "#{now()}"}], + [ + {"method_call", [{"name", "GetLayout"}], ["#{xml_item}"]} + ]} + |> Saxy.encode!() + |> _debug() + end + + def handle_event( + [:ex_sni, :dbus_method, :call], + _metrics, + %{method: "GetGroupProperties", args: {ids, properties}}, + _config + ) do + xml_item = + {"get_group_properties", + [{"ids", Enum.join(ids, ",")}, {"properties", Enum.join(properties, ",")}], []} + |> Saxy.encode!() + + {"dbus", [{"timestamp", "#{now()}"}], + [ + {"method_call", [{"name", "GetGroupProperties"}], ["#{xml_item}"]} + ]} + |> Saxy.encode!() + |> _debug() + end + + def handle_event( + [:ex_sni, :dbus_method, :reply], + _metrics, + %{method: "GetLayout", response: response}, + _config + ) do + xml_layout = + response + |> ExSni.Menu.Debug.parse_layout_response() + |> Saxy.encode!() + + {"dbus", [{"timestamp", "#{now()}"}], + [ + {"method_reply", [{"name", "GetLayout"}], ["#{xml_layout}"]} + ]} + |> Saxy.encode!() + |> _debug() + end + + def handle_event( + [:ex_sni, :dbus_method, :reply], + _metrics, + %{method: "GetGroupProperties", response: response}, + _config + ) do + xml_properties = + response + |> ExSni.Menu.Debug.parse_get_group_properties() + |> Saxy.encode!() + + {"dbus", [{"timestamp", "#{now()}"}], + [ + {"method_reply", [{"name", "GetGroupProperties"}], ["#{xml_properties}"]} + ]} + |> Saxy.encode!() + |> _debug() + end + + def handle_event([:ex_sni, :set_current_menu], _metrics, %{root: root}, _config) do + xml_new_root = ExSni.Menu.Debug.debug_root(root) + + {"ex_sni", [{"timestamp", "#{now()}"}], + [ + {"set_current_menu", [], + [ + "#{xml_new_root}" + ]} + ]} + |> Saxy.encode!() + |> _debug() + end + + defp _debug(str) do + Logger.debug(str) + end + + defp now() do + DateTime.to_unix(DateTime.utc_now()) + end + + # Options parsing + + defp get_debug_options() do + System.fetch_env(@debug_env_name) + |> case do + {:ok, value} -> + value + |> String.split(",") + |> Enum.reject(&(&1 == "")) + + {:error} -> + false + end + |> process_opts() + end + + defp process_opts(opts) do + opts = + cond do + env_opts_contains?(opts, "all") -> + ["all"] + + env_opts_contains?(opts, "menu") -> + Enum.reject(opts, &(&1 in ["menu_diff", "menu_update", "menu_set"])) + + env_opts_contains?(opts, "dbus") -> + Enum.reject(opts, &(&1 in ["dbus_method", "dbus_signal"])) + end + + Enum.reduce(opts, [], &env_opt_to_event/2) + end + + defp env_opts_contains?(opts, what) do + Enum.any?(opts, &(&1 == what)) + end + + defp env_opt_to_event("all", acc) do + Enum.reduce(["menu", "dbus"], acc, &env_opt_to_event/2) + end + + defp env_opt_to_event("menu", acc) do + Enum.reduce(["menu_diff", "menu_update", "menu_set"], acc, &env_opt_to_event/2) + end + + defp env_opt_to_event("menu_diff", acc) do + [[:ex_sni, :menu_diff, :branch], [:ex_sni, :menu_diff, :result] | acc] + end + + defp env_opt_to_event("menu_update", acc) do + [[:ex_sni, :do_menu_update] | acc] + end + + defp env_opt_to_event("menu_set", acc) do + [[:ex_sni, :set_current_menu] | acc] + end + + defp env_opt_to_event("dbus", acc) do + Enum.reduce(["dbus_method", "dbus_signal"], acc, &env_opt_to_event/2) + end + + defp env_opt_to_event("dbus_method", acc) do + [[:ex_sni, :dbus_method, :call], [:ex_sni, :dbus_method, :reply] | acc] + end + + defp env_opt_to_event("dbus_signal", acc) do + [[:ex_sni, :send_dbus_signal] | acc] + end +end From 7637426adef3d8cfa0b4b85717fbd6159ca2edd9 Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Mon, 17 Jan 2022 10:16:15 +0200 Subject: [PATCH 09/14] Fix fetch_env case branch typo --- lib/debugger.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/debugger.ex b/lib/debugger.ex index 5611a88..d0ed0cc 100644 --- a/lib/debugger.ex +++ b/lib/debugger.ex @@ -289,7 +289,7 @@ defmodule ExSni.Debugger do |> String.split(",") |> Enum.reject(&(&1 == "")) - {:error} -> + :error -> false end |> process_opts() From 826b4c68e31b476d4af99755217a2e1e7377bd1d Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Mon, 17 Jan 2022 10:23:51 +0200 Subject: [PATCH 10/14] Fix opts processing for Debugger --- lib/debugger.ex | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/debugger.ex b/lib/debugger.ex index d0ed0cc..c9752e1 100644 --- a/lib/debugger.ex +++ b/lib/debugger.ex @@ -286,11 +286,10 @@ defmodule ExSni.Debugger do |> case do {:ok, value} -> value - |> String.split(",") - |> Enum.reject(&(&1 == "")) + |> String.split(",", trim: true) :error -> - false + [] end |> process_opts() end @@ -306,6 +305,9 @@ defmodule ExSni.Debugger do env_opts_contains?(opts, "dbus") -> Enum.reject(opts, &(&1 in ["dbus_method", "dbus_signal"])) + + true -> + [] end Enum.reduce(opts, [], &env_opt_to_event/2) From 85746a22a7ffb7acf4d49b057dfd10aa8e3675d8 Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Mon, 17 Jan 2022 10:42:32 +0200 Subject: [PATCH 11/14] Fix Debugger options parsing order --- lib/debugger.ex | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/lib/debugger.ex b/lib/debugger.ex index c9752e1..6b243b6 100644 --- a/lib/debugger.ex +++ b/lib/debugger.ex @@ -1,5 +1,5 @@ defmodule ExSni.Debugger do - @module """ + @moduledoc """ Debugger module that attaches to telemetry events. Configurable using the EXSNI_DEBUG environment variable, with the following events, given as a comma-separated list: @@ -295,22 +295,31 @@ defmodule ExSni.Debugger do end defp process_opts(opts) do - opts = - cond do - env_opts_contains?(opts, "all") -> - ["all"] + opts + |> process_filter_opts() + |> Enum.reduce([], &env_opt_to_event/2) + end - env_opts_contains?(opts, "menu") -> + defp process_filter_opts(opts) do + if env_opts_contains?(opts, "all") do + ["all"] + else + opts = + if env_opts_contains?(opts, "menu") do Enum.reject(opts, &(&1 in ["menu_diff", "menu_update", "menu_set"])) + else + opts + end - env_opts_contains?(opts, "dbus") -> + opts = + if env_opts_contains?(opts, "dbus") do Enum.reject(opts, &(&1 in ["dbus_method", "dbus_signal"])) + else + opts + end - true -> - [] - end - - Enum.reduce(opts, [], &env_opt_to_event/2) + opts + end end defp env_opts_contains?(opts, what) do From 1ec9f1aae2ba59267e394bb11476b47a55afd812 Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Tue, 18 Jan 2022 17:04:50 +0200 Subject: [PATCH 12/14] Bug fixes --- lib/menu/menu_item.ex | 36 ++++++++++++++++++++++++---------- lib/menu_diff.ex | 45 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/lib/menu/menu_item.ex b/lib/menu/menu_item.ex index 5739a42..64d3051 100644 --- a/lib/menu/menu_item.ex +++ b/lib/menu/menu_item.ex @@ -501,7 +501,11 @@ defmodule ExSni.Menu.Item do "hr" end - def name(%{type: type}) when type in [:root, :menu] do + def name(%{type: type}) when type in [:root] do + "root" + end + + def name(%{type: type}) when type in [:menu] do "menu" end @@ -526,7 +530,7 @@ defmodule ExSni.Menu.Item do end def value(%{type: type, checked: checked} = node) do - "type=#{type};checked=#{checked}" <> default_value(node) + "type=#{type};checked=#{checked};" <> default_value(node) end # Maybe HANDLE ICON into value @@ -577,15 +581,23 @@ defmodule ExSni.Menu.Item do def build!(item, opts) do built_item = build!(item, []) - case Keyword.get(opts, :only) do - [] -> - built_item + built_item = + case Keyword.get(opts, :only) do + [] -> + built_item - keys when is_list(keys) -> - only_keys(built_item, Enum.map(keys, &Atom.to_string/1)) + keys when is_list(keys) -> + only_keys(built_item, Enum.map(keys, &Atom.to_string/1)) - _ -> - built_item + _ -> + built_item + end + + if Keyword.get(opts, :no_children, false) == false do + built_item + else + {type, attrs, _} = built_item + {type, attrs, []} end end @@ -611,7 +623,11 @@ defmodule ExSni.Menu.Item do Enum.filter(attrs, fn {key, _} -> Enum.member?(keys, key) end), - Enum.map(children, &only_keys(&1, keys)) + if Enum.member?(keys, :children) do + [] + else + Enum.map(children, &only_keys(&1, keys)) + end } end end diff --git a/lib/menu_diff.ex b/lib/menu_diff.ex index 94968b4..363787f 100644 --- a/lib/menu_diff.ex +++ b/lib/menu_diff.ex @@ -511,7 +511,10 @@ defmodule ExSni.MenuDiff do {%{ref: node}, {:nop, %{ref: old_node}}}, {inserts, nops, mapping} -> {inserts, [{node, old_node} | nops], Map.put(mapping, node, old_node)} - {%{ref: node}, {_, %{ref: old_node}}}, {inserts, nops, mapping} -> + {%{ref: node}, {:mov, %{ref: old_node}}}, {inserts, nops, mapping} -> + {inserts, nops, Map.put(mapping, node, old_node)} + + {%{ref: node}, {_op, %{ref: old_node}}}, {inserts, nops, mapping} -> {inserts, nops, Map.put(mapping, node, old_node)} _, acc -> @@ -529,8 +532,46 @@ defmodule ExSni.MenuDiff do {%{ref: node}, :del}, {updates, deletes} -> {updates, [{node, nil} | deletes]} - _, acc -> + _x, acc -> acc end) end + + # Debug functions + # + # defp reduce_op_map(op_map) do + # op_map + # |> Enum.reduce([], fn {n_node, op}, acc -> + # case op do + # {:ins, p_node, prev_node} -> + # [{:ins, trim_node(n_node), trim_node(p_node), trim_node(prev_node)} | acc] + + # {op, o_node} when is_atom(op) -> + # [{op, trim_node(n_node), trim_node(o_node)} | acc] + + # op when is_atom(op) -> + # [{op, trim_node(n_node)} | acc] + + # other -> + # [{op, trim_node(n_node), other} | acc] + # end + # end) + # |> Enum.reverse() + # end + + # defp trim_node(nil) do + # nil + # end + + # defp trim_node(%{} = node) do + # ref_to_xml(node, only: [:id, :type, :label], no_children: true) + # end + + # defp ref_to_xml(%{ref: node}, opts) do + # ExSni.XML.Builder.encode!(node, opts) + # end + + # defp ref_to_xml(v, opts) do + # ExSni.XML.Builder.encode!(v, opts) + # end end From 2f8d977f6d4f2a22583cd09696b3ba8335163034 Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Tue, 18 Jan 2022 17:05:07 +0200 Subject: [PATCH 13/14] Added more menu diff tests --- test/diff_test.exs | 294 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 293 insertions(+), 1 deletion(-) diff --git a/test/diff_test.exs b/test/diff_test.exs index 9d393dc..f64523b 100644 --- a/test/diff_test.exs +++ b/test/diff_test.exs @@ -330,6 +330,296 @@ defmodule ExSni.MenuDiffTest do ExSni.XML.Builder.encode!(root, only: [:id, :uid, :type, :label]) end + test "Fix node remove and inserts bug" do + old_menu = """ + + + + + + + + """ + + new_menu = """ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + """ + + next_menu = """ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + """ + + old_menu + |> from_menu() + |> to_menu(new_menu) + |> test_menu_diff() + |> assert_layout(0) + |> assert_updates([]) + |> assert_menu(next_menu) + end + + test "Insert and delete items in root" do + from_menu(""" + + """) + |> to_menu(""" + + + + """) + |> test_menu_diff() + |> assert_menu(""" + + + + """) + |> to_menu(""" + + + + + + """) + |> test_menu_diff() + |> assert_menu(""" + + + + + + """) + |> to_menu(""" + + + + """) + |> test_menu_diff() + |> assert_menu(""" + + + + """) + end + + test "Insert and delete items in submenus" do + from_menu(""" + + """) + |> to_menu(""" + + + + + + + + + + + + + """) + |> test_menu_diff() + |> assert_menu(""" + + + + + + + + + + + + + """) + |> to_menu(""" + + + + + + + + + + + + + + + + """) + |> test_menu_diff() + |> assert_menu(""" + + + + + + + + + + + + + + + + """) + end + + # Private utility functions + + defp from_menu(old_menu) do + from_menu({nil, nil}, old_menu) + end + + defp from_menu({_, new_root}, old_menu) do + {build_root(old_menu), new_root} + end + + defp to_menu({old_root, _}, new_menu) do + {old_root, build_root(new_menu)} + end + + defp to_menu({_layout, _updates, old_root}, new_menu) do + {old_root, build_root(new_menu)} + end + + defp test_menu_diff({old_root, new_root}) do + assert {layout, updates, root} = MenuDiff.diff(new_root, old_root) + {layout, updates, root} + end + + defp assert_menu( + {_, _, root} = diff_result, + expected_menu, + encode_opts \\ [only: [:id, :type, :label]] + ) do + expected_menu = trim_xml_menu(expected_menu) + + assert expected_menu == + ExSni.XML.Builder.encode!(root, encode_opts) + + diff_result + end + + defp assert_layout({layout, _, _} = diff_result, expected_layout) do + assert layout == expected_layout + diff_result + end + + defp assert_updates({_, updates, _} = diff_result, expected_updates) do + assert updates == expected_updates + diff_result + end + + defp trim_xml_menu(menu) when is_binary(menu) do + menu + |> String.replace(~r/^\s*/, "") + |> String.replace(~r/\s*$/, "") + |> String.replace(~r/\n\s*/, "") + end + defp build_root(source) do case Saxy.SimpleForm.parse_string(source) do {:ok, {"root", _, children}} -> @@ -358,7 +648,9 @@ defmodule ExSni.MenuDiffTest do %Item{type: :menu, children: children} |> Item.assign_unique_int() - Enum.reduce(attrs, node, fn {name, value}, node -> + attrs + |> Enum.reject(&(&1 == "type")) + |> Enum.reduce(node, fn {name, value}, node -> set_attr(node, name, value) end) end From 3665c24fae439d674978a6a1eff55e6e59e166aa Mon Sep 17 00:00:00 2001 From: Mihai Potra Date: Thu, 20 Jan 2022 12:10:29 +0200 Subject: [PATCH 14/14] Improve layout debug output --- lib/menu/debug.ex | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/lib/menu/debug.ex b/lib/menu/debug.ex index 23b388e..88e6674 100644 --- a/lib/menu/debug.ex +++ b/lib/menu/debug.ex @@ -53,17 +53,7 @@ defmodule ExSni.Menu.Debug do parse_layout_item(child) end) - case properties["children-display"] do - "submenu" -> - {"menu", [{"id", "#{id}"}], children} - - _ -> - if properties["type"] == "separator" do - {"hr", [], []} - else - {"item", [{"id", "#{id}"}], []} - end - end + {"item", [{"id", "#{id}"} | properties], children} end def debug_root(nil) do @@ -75,8 +65,9 @@ defmodule ExSni.Menu.Debug do end defp parse_properties(properties) do - Enum.reduce(properties, %{}, fn {key, {:dbus_variant, _type, value}}, acc -> - Map.put(acc, key, "#{value}") + Enum.reduce(properties, [], fn {key, {:dbus_variant, _type, value}}, acc -> + [{key, "#{value}"} | acc] end) + |> Enum.reverse() end end