From d2988d1aab78bc5ff338b5a8409a9062baa89110 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 28 Aug 2016 16:35:47 -0400 Subject: [PATCH 01/12] tag: Fix index when screen count change. There was still a problem that caused the "old" tags to be inserted in the wrong position when "saved" from a screen being removed. Also, this use a :get_tags(true) to save an uneeded sorting pass. --- lib/awful/tag.lua | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/awful/tag.lua b/lib/awful/tag.lua index 7be30a82..b65c3f41 100644 --- a/lib/awful/tag.lua +++ b/lib/awful/tag.lua @@ -465,6 +465,7 @@ function tag.object.set_screen(t, s) -- Change the screen tag.setproperty(t, "screen", s) + tag.setproperty(t, "index", #s:get_tags(true)) -- Make sure the client's screen matches its tags for _,c in ipairs(t:clients()) do @@ -473,10 +474,8 @@ function tag.object.set_screen(t, s) end -- Update all indexes - for _,screen in ipairs {old_screen, s} do - for i,t2 in ipairs(screen.tags) do - tag.setproperty(t2, "index", i) - end + for i,t2 in ipairs(old_screen.tags) do + tag.setproperty(t2, "index", i) end -- Restore the old screen history if the tag was selected From 2af0553b67a79a49bd388acc07fbf6f4edcd9ec9 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 28 Aug 2016 19:00:54 -0400 Subject: [PATCH 02/12] tag: Fix :delete() when there is no clients The logic was broken. It prevented tags from being deleted. This also make sure the boolean return value is always set. --- lib/awful/tag.lua | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/awful/tag.lua b/lib/awful/tag.lua index b65c3f41..063d0440 100644 --- a/lib/awful/tag.lua +++ b/lib/awful/tag.lua @@ -259,15 +259,14 @@ end -- stickied tags to. -- @tparam[opt=false] boolean force Move even non-sticky clients to the fallback -- tag. --- @return Returns true if the tag is successfully deleted, nil otherwise. +-- @return Returns true if the tag is successfully deleted. -- If there are no clients exclusively on this tag then delete it. Any -- stickied clients are assigned to the optional 'fallback_tag'. -- If after deleting the tag there is no selected tag, try and restore from -- history or select the first tag on the screen. function tag.object.delete(self, fallback_tag, force) - -- abort if the taf isn't currently activated - if not self.activated then return end + if not self.activated then return false end local target_scr = get_screen(tag.getproperty(self, "screen")) local tags = target_scr.tags @@ -275,7 +274,7 @@ function tag.object.delete(self, fallback_tag, force) local ntags = #tags -- We can't use the target tag as a fallback. - if fallback_tag == self then return end + if fallback_tag == self then return false end -- No fallback_tag provided, try and get one. if fallback_tag == nil then @@ -284,7 +283,7 @@ function tag.object.delete(self, fallback_tag, force) -- Abort if we would have un-tagged clients. local clients = self:clients() - if ( #clients > 0 and ntags <= 1 ) or fallback_tag == nil then return end + if #clients > 0 and fallback_tag == nil then return false end -- Move the clients we can off of this tag. for _, c in pairs(clients) do From 180e70f6d682391a8ffae7ec1300f740d1981cfd Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 28 Aug 2016 19:02:39 -0400 Subject: [PATCH 03/12] tag: Fix off by one when deleting. If a tag has been deleted, then the number of tags is decremented, the code still used the old count. --- lib/awful/tag.lua | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/awful/tag.lua b/lib/awful/tag.lua index 063d0440..5a4e1810 100644 --- a/lib/awful/tag.lua +++ b/lib/awful/tag.lua @@ -308,8 +308,10 @@ function tag.object.delete(self, fallback_tag, force) tag.setproperty(tags[i], "index", i-1) end - -- If no tags are visible, try and view one. - if target_scr.selected_tag == nil and ntags > 0 then + -- If no tags are visible (and we did not delete the lasttag), try and + -- view one. The > 1 is because ntags is no longer synchronized with the + -- current count. + if target_scr.selected_tag == nil and ntags > 1 then tag.history.restore(nil, 1) if target_scr.selected_tag == nil then local other_tag = tags[tags[1] == self and 2 or 1] From abb36c369755d718ef7d74eada7c6e62aad73797 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 28 Aug 2016 19:04:09 -0400 Subject: [PATCH 04/12] tag: Restore the history on the right screen when deleting. --- lib/awful/tag.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/awful/tag.lua b/lib/awful/tag.lua index 5a4e1810..560e6bab 100644 --- a/lib/awful/tag.lua +++ b/lib/awful/tag.lua @@ -312,7 +312,7 @@ function tag.object.delete(self, fallback_tag, force) -- view one. The > 1 is because ntags is no longer synchronized with the -- current count. if target_scr.selected_tag == nil and ntags > 1 then - tag.history.restore(nil, 1) + tag.history.restore(target_scr, 1) if target_scr.selected_tag == nil then local other_tag = tags[tags[1] == self and 2 or 1] if other_tag then From 21787444e4f12441735cd4080337469e33ab417a Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Mon, 22 Aug 2016 23:18:07 -0400 Subject: [PATCH 05/12] client: Simplify screen selection decision tree. This commit remove the `awful.tag` "manage" hook. The relevant code has been moved to ewmh.lua request::tag handler. The handler is called either by a volontary screen change or by a forced one. It also require the awful.rules to be executed. This is done by default and the user would have to explicitly disable that behavior. From now on, disabling the rules require the user to handle tag selection. Fixes #1028 #1052 --- lib/awful/ewmh.lua | 27 ++++++++++++++++++++++++--- lib/awful/tag.lua | 31 ++++++++++++++++++------------- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/lib/awful/ewmh.lua b/lib/awful/ewmh.lua index d110cd31..c1f2d058 100644 --- a/lib/awful/ewmh.lua +++ b/lib/awful/ewmh.lua @@ -122,20 +122,41 @@ function ewmh.activate(c, context, hints) -- luacheck: no unused args end end +-- Get tags that are on the same screen as the client. This should _almost_ +-- always return the same content as c:tags(). +local function get_valid_tags(c, s) + local tags, new_tags = c:tags(), {} + + for _, t in ipairs(tags) do + if screen[s] == t.screen then + table.insert(new_tags, t) + end + end + + return new_tags +end + --- Tag a window with its requested tag. -- -- It is the default signal handler for `request::tag` on a `client`. -- -- @signalhandler awful.ewmh.tag -- @client c A client to tag --- @tag[opt] t A tag to use. If omitted, then the client is made sticky. +-- @tparam[opt] tag|boolean t A tag to use. If true, then the client is made sticky. -- @tparam[opt={}] table hints Extra information function ewmh.tag(c, t, hints) --luacheck: no unused -- There is nothing to do - if not t and #c:tags() > 0 then return end + if not t and #get_valid_tags(c, c.screen) > 0 then return end if not t then - c:to_selected_tags() + if c.transient_for then + c.screen = c.transient_for.screen + if not c.sticky then + c:tags(c.transient_for:tags()) + end + else + c:to_selected_tags() + end elseif type(t) == "boolean" and t then c.sticky = true else diff --git a/lib/awful/tag.lua b/lib/awful/tag.lua index 560e6bab..a0dd00a3 100644 --- a/lib/awful/tag.lua +++ b/lib/awful/tag.lua @@ -12,6 +12,7 @@ local util = require("awful.util") local ascreen = require("awful.screen") local beautiful = require("beautiful") local object = require("gears.object") +local timer = require("gears.timer") local pairs = pairs local ipairs = ipairs local table = table @@ -1277,21 +1278,25 @@ function tag.attached_connect_signal(screen, ...) end -- Register standard signals. -capi.client.connect_signal("manage", function(c) - -- If we are not managing this application at startup, - -- move it to the screen where the mouse is. - -- We only do it for "normal" windows (i.e. no dock, etc). - if not awesome.startup and c.type ~= "desktop" and c.type ~= "dock" then - if c.transient_for then - c.screen = c.transient_for.screen - if not c.sticky then - c:tags(c.transient_for:tags()) +capi.client.connect_signal("property::screen", function(c) + -- First, the delayed timer is necessary to avoid a race condition with + -- awful.rules. It is also messing up the tags before the user have a chance + -- to set them manually. + timer.delayed_call(function() + local tags, new_tags = c:tags(), {} + + for _, t in ipairs(tags) do + if t.screen == c.screen then + table.insert(new_tags, t) end - else - c.screen = ascreen.focused() end - end - c:connect_signal("property::screen", function() c:to_selected_tags() end) + + if #new_tags == 0 then + c:emit_signal("request::tag", nil, {reason="screen"}) + elseif #new_tags < #tags then + c:tags(new_tags) + end + end) end) -- Keep track of the number of urgent clients. From e29a2d3ac9c55290ed74673caf73cddd35954c66 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 28 Aug 2016 22:18:53 -0400 Subject: [PATCH 06/12] client: Add property::tags It is useful for rules debugging. --- objects/client.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/objects/client.c b/objects/client.c index 838112d1..7a6cfa67 100644 --- a/objects/client.c +++ b/objects/client.c @@ -2304,6 +2304,10 @@ luaA_client_swap(lua_State *L) * * Use the `first_tag` field to access the first tag of a client directly. * + * **Signal:** + * + * * *property::tags* + * * @tparam table tags_table A table with tags to set, or `nil` to get the * current tags. * @treturn table A table with all tags. @@ -2342,7 +2346,10 @@ luaA_client_tags(lua_State *L) lua_pushnil(L); while(lua_next(L, 2)) tag_client(L, c); + lua_pop(L, 1); + + luaA_object_emit_signal(L, -1, "property::tags", 0); } lua_newtable(L); From a0b40bfd8f9cb79af68215e4a911442f5d1155c5 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 28 Aug 2016 22:27:14 -0400 Subject: [PATCH 07/12] tests: Try to detect client "manage" signal race conditions. See #1028 --- tests/test-awful-rules.lua | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test-awful-rules.lua b/tests/test-awful-rules.lua index 7dc871a2..dd136d16 100644 --- a/tests/test-awful-rules.lua +++ b/tests/test-awful-rules.lua @@ -11,6 +11,13 @@ local tests = {} local tb_height = awful.util.round(beautiful.get_font_height() * 1.5) -- local border_width = beautiful.border_width +-- Detect "manage" race conditions +local real_apply = awful.rules.apply +function awful.rules.apply(c) + assert(#c:tags() == 0) + return real_apply(c) +end + local function test_rule(rule) rule.rule = rule.rule or {} From db73887b9f263cd55f43b2697314584777cf1196 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 28 Aug 2016 23:31:24 -0400 Subject: [PATCH 08/12] tests: Add a framework to make it easier to test multiple screens. While testing using "the real deal" and with all the tests would be better, it would add a lot of complexity to the testing framework. This module generate multiple multi-screen scenarios and some obvious issues that they can cause. Over time, as more steps are added, it will provide "good enough" testing for multiple screens. Individual test suits can require() this utility to replicate their steps for each multi-screen scenarios. --- tests/_multi_screen.lua | 512 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 512 insertions(+) create mode 100644 tests/_multi_screen.lua diff --git a/tests/_multi_screen.lua b/tests/_multi_screen.lua new file mode 100644 index 00000000..fab8f367 --- /dev/null +++ b/tests/_multi_screen.lua @@ -0,0 +1,512 @@ +-- Helper module to replicate a set of steps across multiple screen scenarios +-- Ideas for improvements: +-- * Export an iterator so the suits can call the screen change step by step +-- themselves instead of the current all or nothing +-- * Offer multiple scenarios such as screen add, remove, resize/swap instead +-- of having to test all corner cases all the time. +local wibox = require("wibox") +local surface = require("gears.surface") +local wallpaper = require("gears.wallpaper") +local color = require("gears.color") +local shape = require("gears.shape") + +local module = {} + +-- Get the original canvas size before it is "lost" +local canvas_w, canvas_h = root.size() + +local half_w, half_h = math.floor(canvas_w/2), math.floor(canvas_h/2) +local quarter_w = math.floor(canvas_w/4) + +-- Create virtual screens. +-- The steps will be executed on each set of screens. +local dispositions = { + -- A simple classic, 2 identical screens side by side + { + function() + return { + x = 0, + y = 0, + width = half_w, + height = canvas_h, + } + end, + function() + return { + x = half_w, + y = 0, + width = half_w, + height = canvas_h, + } + end, + }, + + -- Take the previous disposition and swap the screens + { + function() + return { + x = half_w, + y = 0, + width = half_w, + height = canvas_h, + } + end, + function() + return { + x = 0, + y = 0, + width = half_w, + height = canvas_h, + } + end, + keep = true, + }, + + -- Take the previous disposition and resize the leftmost one + { + function() + return { + x = quarter_w, + y = 0, + width = 3*quarter_w, + height = canvas_h, + } + end, + function() + return { + x = 0, + y = 0, + width = quarter_w, + height = half_h, + } + end, + keep = true, + }, + + -- Take the previous disposition and remove the leftmost screen + { + function() + return { + x = quarter_w, + y = 0, + width = 3*quarter_w, + height = canvas_h, + } + end, + function() + return nil + end, + keep = true, + }, + + -- Less used, but still quite common vertical setup + { + function() + return { + x = 0, + y = 0, + width = canvas_w, + height = half_h, + } + end, + function() + return { + x = 0, + y = half_h, + width = canvas_w, + height = half_h, + } + end + }, + + -- Another popular setup, 22" screens with a better 31" in the middle. + -- So, 2 smaller vertical screen with a larger central horizontal one. + { + -- Left + function() + return { + x = 0, + y = 0, + width = quarter_w, + height = canvas_h, + } + end, + -- Right, this test non-continous screen index on purpose, as this setup + -- Often requires dual-GPU, it _will_ happen. + function() + return { + x = canvas_w-quarter_w, + y = 0, + width = quarter_w, + height = canvas_h, + } + end, + -- Center + function() + return { + x = quarter_w, + y = 0, + width = half_w, + height = math.floor(canvas_h*(9/16)), + } + end, + }, + + -- Same as the previous one, but with the gap centered + { + -- Left + function() + return { + x = 0, + y = 0, + width = quarter_w, + height = canvas_h, + } + end, + -- Right, this test non-continous screen index on purpose, as this setup + -- Often requires dual-GPU, it _will_ happen. + function() + return { + x = canvas_w-quarter_w, + y = 0, + width = quarter_w, + height = canvas_h, + } + end, + -- Center + function() + return { + x = quarter_w, + y = math.ceil((canvas_h-(canvas_h*(9/16)))/2), + width = half_w, + height = math.floor(canvas_h*(9/16)), + } + end, + + -- Keep the same screens as the last test, just move them + keep = true, + }, + + -- AMD Eyefinity / (old) NVIDIA MOSAIC style config (symmetric grid) + { + function() + return { + x = 0, + y = 0, + width = half_w, + height = half_h, + } + end, + function() + return { + x = half_w, + y = 0, + width = half_w, + height = half_h, + } + end, + function() + return { + x = 0, + y = half_h, + width = half_w, + height = half_h, + } + end, + function() + return { + x = half_w, + y = half_h, + width = half_w, + height = half_h, + } + end, + }, + + -- Corner case 1: Non-continuous screens + -- If there is nothing and client is drag&dropped into that area, some + -- geometry callbacks may break in nil index. + { + function() + return { + x = 0, + y = 0, + width = quarter_w, + height = canvas_h, + } + end, + function() + return { + x = canvas_w-quarter_w, + y = 0, + width = quarter_w, + height = canvas_h, + } + end, + }, + + -- Corner case 2: Nothing at 0x0. + -- As some position may fallback to 0x0 this need to be tested often. It + -- also caused issues such as #154 + { + function() + return { + x = 0, + y = half_w, + width = half_w, + height = half_w, + } + end, + function() + return { + x = half_w, + y = 0, + width = half_w, + height = canvas_h, + } + end + }, + + -- Corner case 3: Many very small screens. + -- On the embedded side of the compuverse, it is possible + -- to buy 32x32 RGB OLED screens. They are usually used to display single + -- status icons, but why not use them as a desktop! This is a critical + -- market for AwesomeWM. Here's a 256px wide strip of tiny screens. + -- This may cause wibars to move to the wrong screen accidentally + { + function() return { x = 0 , y = 0, width = 32, height = 32, } end, + function() return { x = 32 , y = 0, width = 32, height = 32, } end, + function() return { x = 64 , y = 0, width = 32, height = 32, } end, + function() return { x = 96 , y = 0, width = 32, height = 32, } end, + function() return { x = 128, y = 0, width = 32, height = 32, } end, + function() return { x = 160, y = 0, width = 32, height = 32, } end, + function() return { x = 192, y = 0, width = 32, height = 32, } end, + function() return { x = 224, y = 0, width = 32, height = 32, } end, + }, + + -- Corner case 4: A screen taller than more than 1 other screen. + -- this may cause some issues with client resize and drag&drop move + { + function() + return { + x = 0, + y = 0, + width = half_w, + height = canvas_h, + } + end, + function() + return { + x = half_w, + y = 0, + width = half_w, + height = math.floor(canvas_h/3), + } + end, + function() + return { + x = half_w, + y = math.floor(canvas_h/3), + width = half_w, + height = math.floor(canvas_h/3), + } + end, + function() + return { + x = half_w, + y = 2*math.floor(canvas_h/3), + width = half_w, + height = math.floor(canvas_h/3), + } + end + }, + + -- The overlapping corner case isn't supported. There is valid use case, + -- such as a projector with a smaller resolution than the laptop screen + -- in non-scaling mirror mode, but it isn't worth the complexity it brings. +} + +local function check_tag_indexes() + for s in screen do + for i, t in ipairs(s.tags) do + assert(t.index == i) + assert(t.screen == s) + end + end +end + +local colors = { + "#000030", + "#300000", + "#043000", + "#302E00", + "#002C30", + "#300030", + "#301C00", + "#140030", +} + +-- Paint it black +local function clear_screen() + for s in screen do + local sur = surface.widget_to_surface( + wibox.widget { + bg = "#000000", + widget = wibox.container.background + }, + s.geometry.width, + s.geometry.height + ) + wallpaper.fit(sur, s, "#000000") + end +end + +-- Make it easier to debug the tests by showing the screen geometry when the +-- tests are executed. +local function show_screens() + wallpaper.set(color("#000000")) -- Should this clear the wallpaper? It doesn't + + -- Add a wallpaper on each screen + for i=1, screen.count() do + local s = screen[i] + + local w = wibox.widget { + { + text = table.concat{ + "Screen: ",i,"\n", + s.geometry.width,"x",s.geometry.height, + "+",s.geometry.x,",",s.geometry.y + }, + valign = "center", + align = "center", + widget = wibox.widget.textbox, + }, + bg = colors[i], + fg = "#ffffff", + shape_border_color = "#ff0000", + shape_border_width = 1, + shape = shape.rectangle, + widget = wibox.container.background + } + local sur = surface.widget_to_surface( + w, + s.geometry.width, + s.geometry.height + ) + wallpaper.fit(sur, s) + end +end + +local function add_steps(real_steps, new_steps) + for _, dispo in ipairs(dispositions) do + -- Cleanup + table.insert(real_steps, function() + if #client.get() == 0 then return true end + + for _, c in ipairs(client.get()) do + c:kill() + end + end) + + table.insert(real_steps, function() + clear_screen() + local keep = dispo.keep or false + local old = {} + local geos = {} + + check_tag_indexes() + + if keep then + for _, sf in ipairs(dispo) do + local geo = sf and sf() or nil + + -- If the function return nothing, assume the screen need to + -- be destroyed. + table.insert(geos, geo or false) + end + + -- Removed screens need to be explicit. + assert(#geos >= screen.count()) + + -- Keep a cache to avoid working with invalid data + local old_screens = {} + for s in screen do + table.insert(old_screens, s) + end + + for i, s in ipairs(old_screens) do + -- Remove the screen (if no new geometry is given + if not geos[i] then + s:fake_remove() + else + local cur_geo = s.geometry + for _, v in ipairs {"x", "y", "width", "height" } do + cur_geo[v] = geos[i][v] or cur_geo[v] + end + s:fake_resize( + cur_geo.x, + cur_geo.y, + cur_geo.width, + cur_geo.height + ) + end + end + + -- Add the remaining screens + for i=#old_screens + 1, #geos do + local geo = geos[i] + screen.fake_add(geo.x, geo.y, geo.width, geo.height) + end + + check_tag_indexes() + else + -- Move all existing screens out of the way (to avoid temporary overlapping) + for s in screen do + s:fake_resize(canvas_w, canvas_h, 1, 1) + table.insert(old, s) + end + + -- Add the new screens + for _, sf in ipairs(dispo) do + local geo = sf() + screen.fake_add(geo.x, geo.y, geo.width, geo.height) + table.insert(geos, geo) + end + + -- Remove old screens + for _, s in ipairs(old) do + s:fake_remove() + end + end + + show_screens() + + -- Check the result is correct + local expected_count = 0 + for _,v in ipairs(geos) do + expected_count = expected_count + (v and 1 or 0) + end + + assert(expected_count == screen.count()) + for k, geo in ipairs(geos) do + if geo then + local sgeo = screen[k].geometry + assert(geo.x == sgeo.x) + assert(geo.y == sgeo.y) + assert(geo.width == sgeo.width ) + assert(geo.height == sgeo.height) + end + end + + return true + end) + + for _, step in ipairs(new_steps) do + table.insert(real_steps, step) + end + end +end + +return setmetatable(module, { + __call = function(_,...) return add_steps(...) end +}) From 760e72f94a481a5f2894f0bab3b7b5cd7b26e87f Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 28 Aug 2016 23:48:28 -0400 Subject: [PATCH 09/12] tests: Test spawning client on a specific screen. --- tests/test-awful-client.lua | 87 +++++++++++++++++++++++++++++++++++-- 1 file changed, 84 insertions(+), 3 deletions(-) diff --git a/tests/test-awful-client.lua b/tests/test-awful-client.lua index ca36318c..93155bcc 100644 --- a/tests/test-awful-client.lua +++ b/tests/test-awful-client.lua @@ -1,4 +1,5 @@ local awful = require("awful") +local test_client = require("_client") awful.util.deprecate = function() end @@ -64,9 +65,6 @@ assert(c:tags()[1] == t2 or c:tags()[2] == t2) awful.client.toggletag(t2, c) assert(c:tags()[1] == t and c:tags()[1] ~= t2 and c:tags()[2] == nil) --- Test movetoscreen ---FIXME I don't have the hardware to test - -- Test floating assert(c.floating ~= nil and type(c.floating) == "boolean") c.floating = true @@ -79,4 +77,87 @@ end end } +local has_error + +table.insert(steps, function() + -- Make sure there is no extra callbacks that causes double screen changes + -- (regress #1028 #1052) + client.connect_signal("property::screen",function(c) + if c.class and c.class ~= "screen"..c.screen.index then + has_error = table.concat { + "An unexpected screen change did occur\n", + debug.traceback(),"\n", + c.class, " Screen: ",c.screen.index + } + end + end) + + return true +end) + +local multi_screen_steps = {} + +-- Add a test client on each screen. +table.insert(multi_screen_steps, function() + has_error = false + for i=1, screen.count() do + local s = screen[i] + test_client("screen"..i, nil, { + screen = s, + }) + end + + return true +end) + +-- Test if the clients have been placed on the right screen +table.insert(multi_screen_steps, function() + if #client.get() ~= screen.count() then return end + + if has_error then print(has_error) end + assert(not has_error) + + for i=1, screen.count() do + local s = screen[i] + + -- Just in case + for _, t in ipairs(s.selected_tags) do + assert(t.screen == s) + end + + local t = s.selected_tag + assert(t and t.screen == s) + + -- In case the next step has failed, find what went wrong + for _, c in ipairs(t:clients()) do + -- This is _supposed_ to be impossible, but can be forced by buggy + -- lua code. + if c.screen ~= t.screen then + local tags = c:tags() + for _, t2 in ipairs(tags) do + assert(t2.screen == c.screen) + end + + assert(false) + end + assert(c.screen.index == i) + end + + -- Check that the right client is placed, the loop above will trigger + -- asserts already, but in case the tag has more than 1 client and + -- everything is messed up, those asserts are still necessary. + assert(#t:clients() == 1) + assert(t:clients()[1].class == "screen"..i) + assert(t:clients()[1].screen == s) + + -- Make sure it stays where it belongs + awful.placement.maximize(t:clients()[1]) + assert(t:clients()[1] and t:clients()[1].screen == t.screen) + end + + return true +end) + +require("_multi_screen")(steps, multi_screen_steps) + require("_runner").run_steps(steps) From edf7fb40b3a73183fbd938c901f231be35abf6ac Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 28 Aug 2016 23:49:01 -0400 Subject: [PATCH 10/12] tests: Test moving tags between screens. --- tests/test-awful-tag.lua | 113 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/tests/test-awful-tag.lua b/tests/test-awful-tag.lua index 3f684ad6..d58a9898 100644 --- a/tests/test-awful-tag.lua +++ b/tests/test-awful-tag.lua @@ -128,4 +128,117 @@ end end } +local multi_screen_steps = {} + +-- Clear the state (this step will be called once per disposition) +table.insert(multi_screen_steps, function() + if #client.get() == 0 then + -- Remove all existing tags created by the rc.lua callback + for s in screen do + while #s.tags > 0 do + local t = s.tags[1] + -- Test #1051 against regressions + assert(t.index == 1) + + assert(t.screen == s) + + t:delete() + assert(not t.activated) + assert(t.screen == nil) + end + end + + return true + end + + for _, c in ipairs(client.get()) do + c:kill() + end +end) + +-- Add a bunch of tags on each screens. +table.insert(multi_screen_steps, function() + for i=1, screen.count() do + local s = screen[i] + -- Add 5 tags per screen + for idx=1, 5 do + awful.tag.add("S"..i..":"..idx,{screen = s}) + end + end + + -- Check if everything adds up + for i=1, screen.count() do + local tags = screen[i].tags + assert(#tags == 5) + + for k, t in ipairs(tags) do + assert(t.index == k) + assert(t.name == "S"..i..":"..k) + end + end + + -- Select a tag on each screen + for i=1, screen.count() do + screen[i].tags[1].selected = true + end + + return true +end) + +local function check_tag_indexes() + for s in screen do + for i, t in ipairs(s.tags) do + assert(t.index == i) + assert(t.screen == s) + end + end +end + +-- Move tags across screens (without clients) +table.insert(multi_screen_steps, function() + -- This screen disposition cannot be used for this test, move on + if screen.count() < 2 then return true end + + local s1, s2 = screen[1], screen[2] + + local old_count1, old_count2 = #s1.tags, #s2.tags + + local t = awful.tag.add("TEST", {screen=s1}) + + -- Check the current state + assert( t.index == old_count1+1 ) + assert( t.screen == s1 ) + assert( #s1.tags == old_count1+1 ) + assert( s1.tags[old_count1+1] == t ) + assert( #s2.tags == old_count2 ) + + -- Move to another index + local new_index = 3 + t.index = new_index + assert(t.index == new_index) + check_tag_indexes() + + -- Move the tag to screen 2 + t.screen = s2 + assert(t.screen == s2 ) + assert(#s1.tags == old_count1 ) + assert(#s2.tags == old_count2+1 ) + assert( s2.tags[old_count2+1] == t ) + assert( t.index == old_count2+1 ) + check_tag_indexes() + + -- Move to another index + t.index = new_index + assert(t.index == new_index) + check_tag_indexes() + + -- Delete it + t:delete() + check_tag_indexes() + + return true +end) + +require("_multi_screen")(steps, multi_screen_steps) + require("_runner").run_steps(steps) From 35c0476c86a6913208e84733d8f339f0c886c96c Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Wed, 7 Sep 2016 00:40:30 -0400 Subject: [PATCH 11/12] tests: Increase time limit to 120 seconds The "test launching client on a specific screen" suit is very slow. However, it is also necessary to avoid issues such as #1069 or #154 from regressing again. This is a temporary fix until a faster test client "daemon" is developped. --- tests/run.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/run.sh b/tests/run.sh index 51702184..b04d62c6 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -194,7 +194,7 @@ start_awesome() { # Count errors. errors=0 # Seconds after when awesome gets killed. -timeout_stale=60 +timeout_stale=120 # FIXME This should be no more than 60s for f in $tests; do echo "== Running $f ==" From 12b72d6164a75319b3f8200bbbc02c5c6b9a9343 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Wed, 7 Sep 2016 00:41:10 -0400 Subject: [PATCH 12/12] drawable: Prevent collected drawables from being redrawn. --- lib/wibox/drawable.lua | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/wibox/drawable.lua b/lib/wibox/drawable.lua index ddeba52c..0212efae 100644 --- a/lib/wibox/drawable.lua +++ b/lib/wibox/drawable.lua @@ -59,6 +59,8 @@ local function get_widget_context(self) end local function do_redraw(self) + if type(self.drawable) ~= "drawable" then return end --FIXME See #1070 + local surf = surface.load_silently(self.drawable.surface, false) -- The surface can be nil if the drawable's parent was already finalized if not surf then return end