From 4e0915674d929a3da0e602067a1bc48dcec27da0 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sat, 27 Jan 2018 01:08:15 -0500 Subject: [PATCH 1/6] gears: Add a new sort module The first use case is a topological sort to handle dependency graphs Closes #2159 --- lib/gears/init.lua | 1 + lib/gears/sort/init.lua | 9 +++ lib/gears/sort/topological.lua | 117 +++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+) create mode 100644 lib/gears/sort/init.lua create mode 100644 lib/gears/sort/topological.lua diff --git a/lib/gears/init.lua b/lib/gears/init.lua index 89f13580..b8909de3 100644 --- a/lib/gears/init.lua +++ b/lib/gears/init.lua @@ -21,6 +21,7 @@ return math = require("gears.math"); table = require("gears.table"); string = require("gears.string"); + sort = require("gears.sort"); filesystem = require("gears.filesystem"); } diff --git a/lib/gears/sort/init.lua b/lib/gears/sort/init.lua new file mode 100644 index 00000000..2f5e3b9a --- /dev/null +++ b/lib/gears/sort/init.lua @@ -0,0 +1,9 @@ +--------------------------------------------------------------------------- +--- Extra sorting algorithms. +-- +-- @module gears.sort +--------------------------------------------------------------------------- + +return { + topological = require("gears.sort.topological") +} diff --git a/lib/gears/sort/topological.lua b/lib/gears/sort/topological.lua new file mode 100644 index 00000000..58ff32c7 --- /dev/null +++ b/lib/gears/sort/topological.lua @@ -0,0 +1,117 @@ +--------------------------------------------------------------------------- +--- Extra sorting algorithms. +-- +-- @submodule gears.sort +--------------------------------------------------------------------------- + +local tsort = {} +local gtable = require("gears.table") + +local mt = { __index = tsort } + +local function add_node(self, node) + if not self._edges[node] then + self._edges[node] = {} + end +end + +--- Ensure that `node` appears after all `dependencies`. +-- @param node The node that edges are added to. +-- @tparam table dependencies List of nodes that have to appear before `node`. +function tsort:append(node, dependencies) + add_node(self, node) + for _, dep in ipairs(dependencies) do + add_node(self, dep) + self._edges[node][dep] = true + end +end + +--- Ensure that `node` appears before all `subordinates`. +-- @param node The node that edges are added to. +-- @tparam table subordinates List of nodes that have to appear after `node`. +function tsort:prepend(node, subordinates) + for _, dep in ipairs(subordinates) do + self:append(dep, { node }) + end +end + +local HANDLING, DONE = 1, 2 + +local function visit(result, self, state, node) + if state[node] == DONE then + -- This node is already in the output + return + end + if state[node] == HANDLING then + -- We are handling this node already and managed to visit it again + -- from itself. Thus, there must be a loop. + result.BAD = node + return true + end + + state[node] = HANDLING + -- Before this node, all nodes that it depends on must appear + for dep in pairs(self._edges[node]) do + if visit(result, self, state, dep) then + return true + end + end + state[node] = DONE + table.insert(result, node) +end + +--- Create a copy of this topological sort. +-- This is useful to backup it before adding elements that can potentially +-- have circular dependencies and thus render the original useless. +function tsort:clone() + local new = tsort.topological() + + -- Disable deep copy as the sorted values may be objects or tables + new._edges = gtable.clone(self._edges, false) + + return new +end + +--- Remove a node from the topological map. +-- +-- @param node The node +function tsort:remove(node) + self._edges[node] = nil + for _, deps in pairs(self._edges) do + deps[node] = nil + end +end + +--- Try to sort the nodes. +-- @treturn[1] table A sorted list of nodes +-- @treturn[2] nil +-- @return[2] A node around which a loop exists +function tsort:sort() + local result, state = {}, {} + for node in pairs(self._edges) do + if visit(result, self, state, node) then + return nil, result.BAD + end + end + return result +end + +--- A topological sorting class. +-- +-- The object returned by this function allows to create simple dependency +-- graphs. It can be used for decision making or ordering of complex sequences. +-- +-- +--@DOC_text_gears_sort_topological_EXAMPLE@ +-- +-- @function gears.sort.topological + +function tsort.topological() + return setmetatable({ + _edges = {}, + }, mt) +end + +return setmetatable(tsort, {__call = function(_, ...) + return tsort.topological(...) +end}) From 20db37f89254ba1b46acfa20c163b1e92c3ab74f Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sat, 27 Jan 2018 01:08:57 -0500 Subject: [PATCH 2/6] tests: Test the gears.sort module --- spec/gears/sort_topological_spec.lua | 173 ++++++++++++++++++ .../examples/text/gears/sort/topological.lua | 22 +++ 2 files changed, 195 insertions(+) create mode 100644 spec/gears/sort_topological_spec.lua create mode 100644 tests/examples/text/gears/sort/topological.lua diff --git a/spec/gears/sort_topological_spec.lua b/spec/gears/sort_topological_spec.lua new file mode 100644 index 00000000..62fb79d1 --- /dev/null +++ b/spec/gears/sort_topological_spec.lua @@ -0,0 +1,173 @@ +local tsort = require("gears.sort.topological") + +describe("gears.sort.topological", function() + describe("prepend_1", function() + local ts = tsort.topological() + ts:prepend(1, { 2, 3, 4 }) + ts:prepend(2, { 3, 4 }) + ts:prepend(3, { 4 }) + local res = ts:sort() + + assert.is.equal(type(res), "table") + + assert(#res == 4) + for k, v in pairs(res) do + assert.is.equal(k, v) + end + end) + + describe("append_1", function() + + local ts = tsort.topological() + ts:append(4, { 1, 2, 3 }) + ts:append(3, { 1, 2 }) + ts:append(2, { 1 }) + local res = ts:sort() + + assert.is.equal(type(res), "table") + + assert(#res == 4) + for k, v in pairs(res) do + assert.is.equal(k, v) + end + end) + + describe("mixed_1", function() + local ts = tsort.topological() + + ts:prepend(1, { 2, 3 }) + ts:append(3, { 1, 2 }) + ts:append(2, { 1 }) + ts:prepend(2, { 3 }) + + local res, err = ts:sort() + assert.is.equal(res[1], 1 ) + assert.is.equal(res[2], 2 ) + assert.is.equal(res[3], 3 ) + assert.is.equal(type(err), "nil") + end) + + describe("mixed_2", function() + local ts = tsort.topological() + ts:append(11, { 2, 9, 3, 5, 7, 10 }) + ts:append(8, { 7, 3 }) + ts:prepend(8, { 9 }) + ts:prepend(3, { 10 }) + ts:prepend(3, { 7, 5 }) + ts:prepend(2, { 3, 5 }) + ts:append(5, { 3 }) + ts:append(10, { 9 }) + ts:prepend(5, { 7 }) + + local res, err = ts:sort() + assert.is.equal(type(res), "table") + assert.is.equal(type(err), "nil") + + assert.is.equal(#res, 8 ) + + assert.is.equal(res[1], 2) + assert.is.equal(res[2], 3) + assert.is.equal(res[3], 5) + assert.is.equal(res[4], 7) + assert.is.equal(res[5], 8) + assert.is.equal(res[6], 9) + assert.is.equal(res[7], 10) + assert.is.equal(res[8], 11) + end) + + describe("mixed_2_and_clone", function() + local ts = tsort.topological() + ts:append(11, { 2, 9, 3, 5, 7, 10 }) + ts:append(8, { 7, 3 }) + ts:prepend(8, { 9 }) + ts:prepend(3, { 10 }) + ts:prepend(3, { 7, 5 }) + ts:prepend(2, { 3, 5 }) + ts:append(5, { 3 }) + ts:append(10, { 9 }) + ts:prepend(5, { 7 }) + + local res, err = ts:sort() + assert.is.equal(type(res), "table") + assert.is.equal(type(err), "nil") + + assert.is.equal(#res, 8 ) + + assert.is.equal(res[1], 2) + assert.is.equal(res[2], 3) + assert.is.equal(res[3], 5) + assert.is.equal(res[4], 7) + assert.is.equal(res[5], 8) + assert.is.equal(res[6], 9) + assert.is.equal(res[7], 10) + assert.is.equal(res[8], 11) + + local ts2 = ts:clone() + local res2, err2 = ts2:sort() + assert.is.equal(type(res2), "table") + assert.is.equal(type(err2), "nil") + assert.is.equal(#res, #res2) + + for i=1, #res do + assert.is.equal(res[i], res2[i]) + end + end) + + describe("simple_remove", function() + local ts = tsort.topological() + + ts:prepend(1, { 2, 3 }) + ts:append(3, { 1, 2 }) + ts:append(2, { 1 }) + ts:prepend(2, { 3 }) + + local res, err = ts:sort() + + assert.is.equal(#res, 3) + assert.is.equal(type(err), "nil") + + ts:remove(2) + + res, err = ts:sort() + assert.is.equal(#res, 2) + assert.is.equal(type(err), "nil") + end) + + describe("simple_error_1", function() + local ts = tsort.topological() + ts:append(2, { 2 }) + + local res, err = ts:sort() + assert.is.equal(type(err), "number") + assert.is.equal(type(res), "nil") + end) + + describe("simple_error_2", function() + local ts = tsort.topological() + ts:append(2, { 2, 3 }) + + local res, err = ts:sort() + assert.is.equal(type(err), "number") + assert.is.equal(type(res), "nil") + end) + + describe("simple_error_3", function() + local ts = tsort.topological() + ts:append(2, { 3 }) + ts:append(3, { 2 }) + + local res, err = ts:sort() + assert.is.equal(type(err), "number") + assert.is.equal(type(res), "nil") + end) + + describe("simple_error_4", function() + local ts = tsort.topological() + ts:prepend(2, { 3 }) + ts:prepend(3, { 2 }) + + local res, err = ts:sort() + assert.is.equal(type(err), "number") + assert.is.equal(type(res), "nil") + end) +end) diff --git a/tests/examples/text/gears/sort/topological.lua b/tests/examples/text/gears/sort/topological.lua new file mode 100644 index 00000000..08e2846c --- /dev/null +++ b/tests/examples/text/gears/sort/topological.lua @@ -0,0 +1,22 @@ +local gears = {sort={topological = require("gears.sort.topological")}} --DOC_HIDE + +local tsort = gears.sort.topological() +tsort:prepend('a', { 'b' }) +tsort:prepend('b', { 'c' }) +tsort:prepend('c', { 'd' }) +tsort:append('e', { 'd' }) +tsort:append('f', { 'e', 'd' }) + +local res = assert(tsort:sort()) + +for k, v in ipairs(res) do + print("The position #"..k.." is: "..v) +end + +assert(#res == 6) --DOC_HIDE +assert(res[1] == 'a') --DOC_HIDE +assert(res[2] == 'b') --DOC_HIDE +assert(res[3] == 'c') --DOC_HIDE +assert(res[4] == 'd') --DOC_HIDE +assert(res[5] == 'e') --DOC_HIDE +assert(res[6] == 'f') --DOC_HIDE From 74508098de42c24b37fde7d4b50102a9ab72b656 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sat, 28 Jan 2017 16:12:39 -0500 Subject: [PATCH 3/6] rules: Add a rule building chain of responsability Before this commit, there was a conflict between the spawn and awful.rules rules. Also, modules such as Tryannical monkey-patched this function to add their own rules to the mix. This commit introduce a proper API to add handlers. The order is crutial for this to work, so a dependency system is also added. Fix #1482 --- docs/config.ld | 2 + lib/awful/rules.lua | 178 +++++++++++++++++++++++++++++++++++++++++--- lib/awful/spawn.lua | 6 +- 3 files changed, 174 insertions(+), 12 deletions(-) diff --git a/docs/config.ld b/docs/config.ld index f553e669..a0e0a9fb 100644 --- a/docs/config.ld +++ b/docs/config.ld @@ -66,6 +66,8 @@ new_type("legacylayout", "Layout related functions", false, "param") new_type("clientlayout", "Client layouts", false, "param") -- Document some callback prototypes new_type("callback", "Callback functions prototype", false, "Parameters") +-- awful.rules sources +new_type("rulesources", "Rule sources", false, "param") -- More fitting section names kind_names={topic='Documentation', module='Libraries', script='Sample files'} diff --git a/lib/awful/rules.lua b/lib/awful/rules.lua index a3c68925..12ca789d 100644 --- a/lib/awful/rules.lua +++ b/lib/awful/rules.lua @@ -37,6 +37,10 @@ local atag = require("awful.tag") local gtable = require("gears.table") local a_place = require("awful.placement") local protected_call = require("gears.protected_call") +local aspawn = require("awful.spawn") +local gsort = require("gears.sort") +local gdebug = require("gears.debug") +local unpack = unpack or table.unpack -- luacheck: globals unpack (compatibility with Lua 5.1) local rules = {} @@ -207,23 +211,176 @@ function rules.matches_list(c, _rules) return false end ---- Apply awful.rules.rules to a client. --- @client c The client. -function rules.apply(c) - local props = {} +-- Contains the sources. +-- The elements are ordered "first in, first executed". Thus, the higher the +-- index, the higher the priority. Each entry is a table with a `name` and a +-- `callback` field. This table is exposed for debugging purpose. The API +-- is private and should be modified using the public accessors. +local rule_sources = {} +local rule_source_sort = gsort.topological() + +--- Add a new rule source. +-- +-- A rule source is a provider called when a client is managed (started). It +-- allows to configure the client by providing properties that should be applied. +-- By default, Awesome provides 2 sources: +-- +-- * `awful.rules`: A declarative matcher +-- * `awful.spawn`: Launch clients with pre-defined properties +-- +-- It is possible to register new callbacks to modify the properties table +-- before it is applied. Each provider is executed sequentially and modifies the +-- same table. If the first provider set a property, then the second can +-- override it, then the third, etc. Once the providers are exhausted, the +-- properties are applied on the client. +-- +-- It is important to note that properties themselves have their own +-- dependencies. For example, a `tag` property implies a `screen`. Therefor, if +-- a `screen` is already specified, then it will be ignored when the rule is +-- executed. Properties also have their own priorities. For example, the +-- `titlebar` and `border_width` need to be applied before the `x` and `y` +-- positions are set. Otherwise, it will be off or the client will shift +-- upward everytime Awesome is restarted. A rule source *cannot* change this. +-- It is up to the callback to be aware of the dependencies and avoid to +-- introduce issues. For example, if the source wants to set a `screen`, it has +-- to check if the `tag`, `tags` or `new_tag` are on that `screen` or remove +-- those properties. Otherwise, they will be ignored once the rule is applied. +-- +-- @tparam string name The provider name. It must be unique. +-- @tparam function callback The callback that is called to produce properties. +-- @tparam client callback.c The client +-- @tparam table callback.properties The current properties. The callback should +-- add to and overwrite properties in this table +-- @tparam table callback.callbacks A table of all callbacks scheduled to be +-- executed after the main properties are applied. +-- @tparam[opt={}] table depends_on A list of names of sources this source depends on +-- (sources that must be executed *before* `name`. +-- @tparam[opt={}] table precede A list of names of sources this source have a +-- priority over. +-- @treturn boolean Returns false if a dependency conflict was found. +function rules.add_rule_source(name, callback, depends_on, precede) + depends_on = depends_on or {} + precede = precede or {} + assert(type( depends_on ) == "table") + assert(type( precede ) == "table") + + for _, v in ipairs(rule_sources) do + -- Names must be unique + assert( + v.name ~= name, + "Name must be unique, but '" .. name .. "' was already registered." + ) + end + + local new_sources = rule_source_sort:clone() + + new_sources:prepend(name, precede ) + new_sources:append (name, depends_on ) + + local res, err = new_sources:sort() + + if err then + gdebug.print_warning("Failed to add the rule source: "..err) + return false + end + + -- Only replace the source once the additions have been proven safe + rule_source_sort = new_sources + local callbacks = {} - for _, entry in ipairs(rules.matching_rules(c, rules.rules)) do - if entry.properties then - for property, value in pairs(entry.properties) do - props[property] = value - end + -- Get all callbacks for *existing* sources. + -- It is important to remember that names can be used in the sorting even + -- if the source itself doesn't (yet) exists. + for _, v in ipairs(rule_sources) do + callbacks[v.name] = v.callback + end + + rule_sources = {} + callbacks[name] = callback + + for _, v in ipairs(res) do + if callbacks[v] then + table.insert(rule_sources, 1, { + callback = callbacks[v], + name = v + }) end + end + + return true +end + +--- Remove a source. +-- @tparam string name The source name. +-- @treturn boolean If the source was removed +function rules.remove_rule_source(name) + rule_source_sort:remove(name) + + for k, v in ipairs(rule_sources) do + if v.name == name then + table.remove(rule_sources, k) + return true + end + end + + return false +end + +-- Add the rules properties +local function apply_awful_rules(c, props, callbacks) + for _, entry in ipairs(rules.matching_rules(c, rules.rules)) do + gtable.crush(props,entry.properties or {}) + if entry.callback then table.insert(callbacks, entry.callback) end end +end + +--- The default `awful.rules` source. +-- +-- **Has priority over:** +-- +-- *nothing* +-- +-- @rulesources awful.rules + +rules.add_rule_source("awful.rules", apply_awful_rules, {"awful.spawn"}, {}) + +-- Add startup_id overridden properties +local function apply_spawn_rules(c, props, callbacks) + if c.startup_id and aspawn.snid_buffer[c.startup_id] then + local snprops, sncb = unpack(aspawn.snid_buffer[c.startup_id]) + + -- The SNID tag(s) always have precedence over the rules one(s) + if snprops.tag or snprops.tags or snprops.new_tag then + props.tag, props.tags, props.new_tag = nil, nil, nil + end + + gtable.crush(props, snprops) + gtable.merge(callbacks, sncb) + end +end + +--- The rule source for clients spawned by `awful.spawn`. +-- +-- **Has priority over:** +-- +-- * `awful.rules` +-- +-- @rulesources awful.spawn + +rules.add_rule_source("awful.spawn", apply_spawn_rules, {}, {"awful.rules"}) + +--- Apply awful.rules.rules to a client. +-- @client c The client. +function rules.apply(c) + local callbacks, props = {}, {} + for _, v in ipairs(rule_sources) do + v.callback(c, props, callbacks) + end rules.execute(c, props, callbacks) end @@ -531,12 +688,11 @@ function rules.execute(c, props, callbacks) end end +-- TODO v5 deprecate this function rules.completed_with_payload_callback(c, props, callbacks) rules.execute(c, props, callbacks) end -client.connect_signal("spawn::completed_with_payload", rules.completed_with_payload_callback) - client.connect_signal("manage", rules.apply) return rules diff --git a/lib/awful/spawn.lua b/lib/awful/spawn.lua index 069e0ec9..029b33e5 100644 --- a/lib/awful/spawn.lua +++ b/lib/awful/spawn.lua @@ -165,6 +165,7 @@ local lgi = require("lgi") local Gio = lgi.Gio local GLib = lgi.GLib local util = require("awful.util") +local timer = require("gears.timer") local protected_call = require("gears.protected_call") local spawn = {} @@ -217,7 +218,10 @@ function spawn.on_snid_callback(c) local props = entry[1] local callback = entry[2] c:emit_signal("spawn::completed_with_payload", props, callback) - spawn.snid_buffer[c.startup_id] = nil + + timer.delayed_call(function() + spawn.snid_buffer[c.startup_id] = nil + end) end end From 23d9727590871d18290d8d0d14f16b3909c0ce6c Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sat, 28 Jan 2017 17:03:36 -0500 Subject: [PATCH 4/6] tests: Make sure clients really end up on the right screen Prevent #1482 from regressing --- tests/test-awful-client.lua | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/test-awful-client.lua b/tests/test-awful-client.lua index dd0b494e..c2b6fed6 100644 --- a/tests/test-awful-client.lua +++ b/tests/test-awful-client.lua @@ -296,6 +296,12 @@ table.insert(multi_screen_steps, function() screen = screen[1], }) + -- Same as previous, but switched + test_client("test_tag2", nil, { + tag = screen[1].tags[5], + screen = screen[2], + }) + -- Add a client with multiple tags on the same screen, but not c.screen test_client("test_tags1", nil, { tags = { screen[1].tags[3], screen[1].tags[4] }, @@ -326,7 +332,7 @@ end) table.insert(multi_screen_steps, function() if screen.count() < 2 then return true end - if #client.get() ~= 5 then return end + if #client.get() ~= 6 then return end local c_by_class = {} @@ -336,6 +342,11 @@ table.insert(multi_screen_steps, function() assert(c_by_class["test_tag1"].screen == screen[2]) assert(#c_by_class["test_tag1"]:tags() == 1) + assert(c_by_class["test_tag1"]:tags()[1] == screen[2].tags[2]) + + assert(c_by_class["test_tag2"].screen == screen[1]) + assert(#c_by_class["test_tag2"]:tags() == 1) + assert(c_by_class["test_tag2"]:tags()[1] == screen[1].tags[5]) assert(c_by_class["test_tags1"].screen == screen[1]) assert(#c_by_class["test_tags1"]:tags() == 2) From aae9655e41e5de9c6a6bb83abd4acf65e70c4642 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sat, 28 Jan 2017 21:39:25 -0500 Subject: [PATCH 5/6] tests: Test the custom rule sources --- tests/test-awful-rules.lua | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/test-awful-rules.lua b/tests/test-awful-rules.lua index 59b1b43f..d99fccdb 100644 --- a/tests/test-awful-rules.lua +++ b/tests/test-awful-rules.lua @@ -264,6 +264,34 @@ test_rule { end } +-- Test the custom sources +assert(awful.rules.add_rule_source("high_priority", function(c, props, _) + assert(type(c) == "client") + assert(props.random2) + + props.random1 = true +end, {"awful.spawn"}, {"awful.rules", "low_priority"})) + +assert(awful.rules.add_rule_source("before2", function() + error("This function should not be called") +end, {"awful.spawn"}, {"awful.rules"})) + +assert(awful.rules.remove_rule_source("before2")) + +assert(awful.rules.add_rule_source("low_priority", function(c, props, _) + assert(type(c) == "client") + assert(not props.random1) + + props.random2 = true +end, {"awful.spawn", "high_priority"}, {"void", "awful.rules"})) + +local temp = gears.debug.print_warning +gears.debug.print_warning = function() end +assert(not awful.rules.add_rule_source("invalid_source", function() + assert(false, "This cannot happen") +end, {"awful.rules"}, {"awful.spawn"})) +gears.debug.print_warning = temp + -- Test tag and switchtotag test_rule { properties = { @@ -295,10 +323,17 @@ test_rule { assert(c.screen.selected_tag.name ~= "8") + -- Test the custom sources + assert(c.random1) + assert(c.random2) + assert(not c.random3) + return true end } + + -- Wait until all the auto-generated clients are ready local function spawn_clients() if #client.get() >= #tests then From 017077a33a79994dcd2c7c5611b752945b683727 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Tue, 14 Mar 2017 17:36:02 -0400 Subject: [PATCH 6/6] spawn: Add a TODO to remove an useless/broken signal from v5 It has a race condition causing it to be useless. --- lib/awful/spawn.lua | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/awful/spawn.lua b/lib/awful/spawn.lua index 029b33e5..a51ba36a 100644 --- a/lib/awful/spawn.lua +++ b/lib/awful/spawn.lua @@ -217,6 +217,7 @@ function spawn.on_snid_callback(c) if entry then local props = entry[1] local callback = entry[2] + --TODO v5: Remove this signal c:emit_signal("spawn::completed_with_payload", props, callback) timer.delayed_call(function()