diff --git a/lib/wibox/layout/align.lua b/lib/wibox/layout/align.lua index 069f4674b..258acb892 100644 --- a/lib/wibox/layout/align.lua +++ b/lib/wibox/layout/align.lua @@ -175,11 +175,9 @@ end -- This layout only accept three children, all others will be ignored -- @tparam table children A table composed of valid widgets function align:set_children(children) - if not children then return self:reset() end - self.first = children[1] - self.second = children[2] - self.third = children[3] - self:emit_signal("widget::layout_changed") + self:set_first(children[1]) + self:set_second(children[2]) + self:set_third(children[3]) end --- Fit the align layout into the given space. The align layout will diff --git a/lib/wibox/layout/constraint.lua b/lib/wibox/layout/constraint.lua index 99177c1bd..e355e5e4b 100644 --- a/lib/wibox/layout/constraint.lua +++ b/lib/wibox/layout/constraint.lua @@ -54,8 +54,7 @@ end -- This layout only accept one children, all others will be ignored -- @tparam table children A table composed of valid widgets function constraint:set_children(children) - self.widget = children and children[1] - self:emit_signal("widget::layout_changed") + self:set_widget(children[1]) end --- Set the strategy to use for the constraining. Valid values are 'max', diff --git a/lib/wibox/layout/fixed.lua b/lib/wibox/layout/fixed.lua index f2de84b8f..ecc704dc6 100644 --- a/lib/wibox/layout/fixed.lua +++ b/lib/wibox/layout/fixed.lua @@ -5,6 +5,7 @@ -- @classmod wibox.layout.fixed --------------------------------------------------------------------------- +local unpack = unpack or table.unpack -- luacheck: globals unpack (compatibility with Lua 5.1) local base = require("wibox.widget.base") local table = table local pairs = pairs @@ -110,9 +111,10 @@ end --- Replace the layout children -- @tparam table children A table composed of valid widgets function fixed:set_children(children) - if not children then return self:reset() end - self.widgets = children - self:emit_signal("widget::layout_changed") + self:reset() + if #children > 0 then + self:add(unpack(children)) + end end --- Replace the first instance of `widget` in the layout with `widget2` diff --git a/lib/wibox/layout/margin.lua b/lib/wibox/layout/margin.lua index e7629a106..9a8a8b2cd 100644 --- a/lib/wibox/layout/margin.lua +++ b/lib/wibox/layout/margin.lua @@ -82,8 +82,7 @@ end -- This layout only accept one children, all others will be ignored -- @tparam table children A table composed of valid widgets function margin:set_children(children) - self.widget = children and children[1] - self:emit_signal("widget::layout_changed") + self:set_widget(children[1]) end --- Set all the margins to val. diff --git a/lib/wibox/layout/mirror.lua b/lib/wibox/layout/mirror.lua index cac2936e6..2d19385f3 100644 --- a/lib/wibox/layout/mirror.lua +++ b/lib/wibox/layout/mirror.lua @@ -64,8 +64,7 @@ end -- This layout only accept one children, all others will be ignored -- @tparam table children A table composed of valid widgets function mirror:set_children(children) - self.widget = children and children[1] - self:emit_signal("widget::layout_changed") + self:set_widget(children[1]) end --- Reset this layout. The widget will be removed and the axes reset. diff --git a/lib/wibox/layout/rotate.lua b/lib/wibox/layout/rotate.lua index 8de9ef7bf..46e60b8e0 100644 --- a/lib/wibox/layout/rotate.lua +++ b/lib/wibox/layout/rotate.lua @@ -76,8 +76,7 @@ end -- This layout only accept one children, all others will be ignored -- @tparam table children A table composed of valid widgets function rotate:set_children(children) - self.widget = children and children[1] - self:emit_signal("widget::layout_changed") + self:set_widget(children[1]) end --- Reset this layout. The widget will be removed and the rotation reset. diff --git a/lib/wibox/layout/scroll.lua b/lib/wibox/layout/scroll.lua index 1af8f94eb..850fee4b0 100644 --- a/lib/wibox/layout/scroll.lua +++ b/lib/wibox/layout/scroll.lua @@ -267,8 +267,7 @@ end -- This layout only accept one children, all others will be ignored -- @tparam table children A table composed of valid widgets function scroll:set_children(children) - self.widget = children and children[1] - self:emit_signal("widget::layout_changed") + self:set_widget(children[1]) end --- Specify the expand mode that is used for extra space. diff --git a/lib/wibox/widget/background.lua b/lib/wibox/widget/background.lua index 274d80243..7d547124a 100644 --- a/lib/wibox/widget/background.lua +++ b/lib/wibox/widget/background.lua @@ -109,8 +109,7 @@ end -- This layout only accept one children, all others will be ignored -- @tparam table children A table composed of valid widgets function background:set_children(children) - self.widget = children and children[1] - self:emit_signal("widget::layout_changed") + self:set_widget(children[1]) end --- Set the background to use diff --git a/lib/wibox/widget/base.lua b/lib/wibox/widget/base.lua index d347a43ce..d9bf92af2 100644 --- a/lib/wibox/widget/base.lua +++ b/lib/wibox/widget/base.lua @@ -338,13 +338,15 @@ local function parse_table(t, leave_empty) local attributes, widgets = {}, {} for k,v in pairs(t) do if type(k) == "number" then - -- As `ipairs` doesn't always work on sparse tables, update the - -- maximum - if k > max then - max = k - end + if v then + -- As `ipairs` doesn't always work on sparse tables, update the + -- maximum + if k > max then + max = k + end - widgets[k] = v + widgets[k] = v + end else attributes[k] = v end @@ -391,6 +393,7 @@ local function drill(ids, content) e, id2 = drill(ids, v) widgets[k] = e end + base.check_widget(widgets[k]) -- Place the widget in the access table if id2 then @@ -559,7 +562,8 @@ end --- Do some sanity checking on widget. This function raises a lua error if -- widget is not a valid widget. function base.check_widget(widget) - assert(type(widget) == "table") + assert(type(widget) == "table", "Type should be table, but is " .. tostring(type(widget))) + assert(widget.is_widget, "Argument is not a widget!") for _, func in pairs({ "add_signal", "connect_signal", "disconnect_signal" }) do assert(type(widget[func]) == "function", func .. " is not a function") end diff --git a/spec/wibox/test_utils.lua b/spec/wibox/test_utils.lua index a557df76c..04e6e0f13 100644 --- a/spec/wibox/test_utils.lua +++ b/spec/wibox/test_utils.lua @@ -84,6 +84,7 @@ return { local w = object() w:add_signal("widget::redraw_needed") w:add_signal("widget::layout_changed") + w.is_widget = true w.visible = true w.opacity = 1 if width or height then diff --git a/spec/wibox/widget/base_spec.lua b/spec/wibox/widget/base_spec.lua new file mode 100644 index 000000000..91be59182 --- /dev/null +++ b/spec/wibox/widget/base_spec.lua @@ -0,0 +1,92 @@ +--------------------------------------------------------------------------- +-- @author Uli Schlachter +-- @copyright 2016 Uli Schlachter +--------------------------------------------------------------------------- + +local base = require("wibox.widget.base") +local no_parent = base.no_parent_I_know_what_I_am_doing + +describe("wibox.widget.base", function() + local widget1, widget2 + before_each(function() + widget1 = base.make_widget() + widget2 = base.make_widget() + widget1.layout = function() + return { base.place_widget_at(widget2, 0, 0, 1, 1) } + end + end) + + describe("caches", function() + it("garbage collectable", function() + local alive = setmetatable({ widget1, widget2 }, { __mode = "kv" }) + assert.is.equal(2, #alive) + + widget1, widget2 = nil, nil + collectgarbage("collect") + assert.is.equal(0, #alive) + end) + + it("simple cache clear", function() + local alive = setmetatable({ widget1, widget2 }, { __mode = "kv" }) + base.layout_widget(no_parent, { "fake context" }, widget1, 20, 20) + assert.is.equal(2, #alive) + + widget1, widget2 = nil, nil + collectgarbage("collect") + assert.is.equal(0, #alive) + end) + + it("self-reference cache clear", function() + widget2.widget = widget1 + + local alive = setmetatable({ widget1, widget2 }, { __mode = "kv" }) + base.layout_widget(no_parent, { "fake context" }, widget1, 20, 20) + assert.is.equal(2, #alive) + + widget1, widget2 = nil, nil + collectgarbage("collect") + assert.is.equal(0, #alive) + end) + end) + + describe("setup", function() + it("Filters out 'false'", function() + -- Regression test: There was a bug where "nil"s where correctly + -- skipped, but "false" entries survived + local layout1, layout2 = base.make_widget(), base.make_widget() + local called = false + function layout1:set_widget(w) + called = true + assert.equals(w, layout2) + end + function layout2:set_children(children) + assert.is_same({nil, widget1, nil, widget2}, children) + end + layout2.allow_empty_widget = true + layout1:setup{ layout = layout2, false, widget1, nil, widget2 } + assert.is_true(called) + end) + + it("Attribute 'false' works", function() + -- Regression test: I introduced a bug with the above fix + local layout1, layout2 = base.make_widget(), base.make_widget() + local called1, called2 = false, false + function layout1:set_widget(w) + called1 = true + assert.equals(w, layout2) + end + function layout2:set_children(children) + assert.is_same({}, children) + end + function layout2:set_foo(foo) + called2 = true + assert.is_false(foo) + end + layout1:setup{ layout = layout2, foo = false } + assert.is_true(called1) + assert.is_true(called2) + end) + end) +end) + +-- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80