Merge pull request #1267 from Elv13/fix_rules_screen_mismatch

Fix more rules screen mismatch

docs/90-FAQ.md

@@ -371,6 +371,16 @@ xkb keyboard description xkb_symbols line.
     };
     EOF
 
+### I upgraded from Awesome 3 to Awesome 4 and multiscreen broke. Why is that?
+
+Awesome 4.0+ support dynamic screen plugging and unplugging without restarting.
+
+This avoids losing your tags, layout and focus history. Olders `rc.lua` were not
+designed to support such changes and assumed Awesome would restart. To add
+multi-screen support to existing configs, see how
+`awful.screen.connect_for_each_screen` is used in the new `rc.lua` or rebuild
+your config on a newer revision of `rc.lua`.
+
 ## Development
 
 ### How to report bugs?

lib/awful/rules.lua

@@ -267,12 +267,18 @@ local force_ignore = {
     border_width=true,floating=true,size_hints_honor=true
 }
 
-function rules.high_priority_properties.tag(c, value)
+function rules.high_priority_properties.tag(c, value, props)
     if value then
         if type(value) == "string" then
             value = atag.find_by_name(c.screen, value)
         end
 
+        -- In case the tag has been forced to another screen, move the client
+        if c.screen ~= value.screen then
+            c.screen = value.screen
+            props.screen = value.screen -- In case another rule query it
+        end
+
         c:tags{ value }
     end
 end
@@ -370,9 +376,32 @@ function rules.extra_properties.placement(c, value)
     end
 end
 
-function rules.extra_properties.tags(c, value)
+function rules.extra_properties.tags(c, value, props)
     local current = c:tags()
-    c:tags(util.table.merge(current, value))
+
+    local tags, s = {}, nil
+
+    for _, t in ipairs(value) do
+        if type(t) == "string" then
+            t = atag.find_by_name(c.screen, t)
+        end
+
+        if t and ((not s) or t.screen == s) then
+            table.insert(tags, t)
+            s = s or t.screen
+        end
+    end
+
+    if s and s ~= c.screen then
+        c.screen = s
+        props.screen = s -- In case another rule query it
+    end
+
+    if #current == 0 or (value[1] and value[1].screen ~= current[1].screen) then
+        c:tags(tags)
+    else
+        c:tags(util.table.merge(current, tags))
+    end
 end
 
 --- Apply properties and callbacks to a client.
@@ -466,7 +495,7 @@ function rules.execute(c, props, callbacks)
 
         if not ignore then
             if rules.extra_properties[property] then
-                rules.extra_properties[property](c, value)
+                rules.extra_properties[property](c, value, props)
             elseif type(c[property]) == "function" then
                 c[property](c, value)
             else

tests/_multi_screen.lua

@@ -348,53 +348,58 @@ local colors = {
 
 -- 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
+--     for s in screen do --FIXME very, very slow on Travis
+--         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
+    --FIXME very, very slow on Travis
+--     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
 
-    -- 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
+    --TODO remove this line, all it does it saving some time in the tests until
+    -- the bug is fixed.
+    wallpaper.maximized = function() end
 end
 
 local function add_steps(real_steps, new_steps)

tests/test-awful-client.lua

@@ -236,6 +236,73 @@ table.insert(multi_screen_steps, function()
         assert(c.first_tag.name == "NEW_AT_"..c.screen.index)
     end
 
+    -- Kill the client
+    for _, c in ipairs(client.get()) do
+        c:kill()
+    end
+
+    if screen.count() < 2 then return true end
+
+    -- Now, add client where the target tag and screen don't match
+    test_client("test_tag1", nil, {
+        tag    = screen[2].tags[2],
+        screen = screen[1],
+    })
+
+    -- 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] },
+        screen = screen[2],
+    })
+
+    -- Identical, but using the tag names
+    test_client("test_tags2", nil, {
+        tags   = { "3", "4" },
+        screen = screen[2],
+    })
+
+    -- Also test tags, but with an invalid screen array
+    test_client("test_tags3", nil, {
+        tags   = { screen[2].tags[3], screen[1].tags[4] },
+        screen = screen[1],
+    })
+
+    -- Another test for tags, but with no matching names
+    test_client("test_tags4", nil, {
+        tags   = { "foobar", "bobcat" },
+        screen = screen[1],
+    })
+
+    return true
+end)
+
+
+table.insert(multi_screen_steps, function()
+    if screen.count() < 2 then return true end
+    if #client.get() ~= 5 then return end
+
+    local c_by_class = {}
+
+    for _, c in ipairs(client.get()) do
+        c_by_class[c.class] = c
+    end
+
+    assert(c_by_class["test_tag1"].screen == screen[2])
+    assert(#c_by_class["test_tag1"]:tags() == 1)
+
+    assert(c_by_class["test_tags1"].screen == screen[1])
+    assert(#c_by_class["test_tags1"]:tags() == 2)
+
+    assert(c_by_class["test_tags2"].screen == screen[2])
+    assert(#c_by_class["test_tags2"]:tags() == 2)
+
+    assert(c_by_class["test_tags3"].screen == screen[2])
+    assert(#c_by_class["test_tags3"]:tags() == 1)
+
+    assert(c_by_class["test_tags4"].screen == screen[1])
+    assert(#c_by_class["test_tags4"]:tags() == 1)
+    assert(c_by_class["test_tags4"]:tags()[1] == screen[1].selected_tag)
+
     return true
 end)
 

tests/test-wallpaper.lua

@@ -0,0 +1,139 @@
+--- Test if the wallpaper function do not cause errors
+
+local runner = require("_runner")
+local wp = require("gears.wallpaper")
+local color = require("gears.color")
+local cairo = require( "lgi" ).cairo
+local surface = require("gears.surface")
+
+local steps = {}
+
+local colors = {
+    "#000030",
+    {
+        type  = "linear" ,
+        from  = { 0, 0  },
+        to    = { 0, 100 },
+        stops = {
+            { 0, "#ff0000" },
+            { 1, "#0000ff" }
+        }
+    },
+    color("#043000"),
+    "#302E00",
+    "#002C30",
+    color("#300030AA"),
+    "#301C00",
+    "#140030",
+}
+
+-- Dummy wallpaper
+local img = cairo.ImageSurface.create(cairo.Format.ARGB32, 100, 100)
+local cr = cairo.Context(img)
+cr:set_source(color(colors[2]))
+cr:rectangle(0,0,100,100)
+cr:fill()
+cr:paint()
+
+table.insert(steps, function()
+    assert(img)
+    assert(surface.load_uncached(img))
+
+    local w, h = surface.get_size(surface.load_uncached(img))
+
+    assert(w == 100)
+    assert(h == 100)
+
+    wp.fit(img, nil, nil)
+
+    wp.fit(img, screen[1], nil)
+
+    -- There is a delayed call for the last call, let it be processed before
+    -- adding more
+    return true
+end)
+
+table.insert(steps, function()
+    wp.fit(img, screen[1], "#00ff00")
+
+    -- There is a delayed call for the last call, let it be processed before
+    -- adding more
+    return true
+end)
+
+table.insert(steps, function()
+    wp.centered(img, nil, nil)
+    wp.centered(img, screen[1], nil)
+
+    -- There is a delayed call for the last call, let it be processed before
+    -- adding more
+    return true
+end)
+
+table.insert(steps, function()
+    wp.centered(img, screen[1], "#00ff00")
+
+    return true
+end)
+
+table.insert(steps, function()
+    wp.maximized(img, nil, nil, nil)
+
+    -- There is a delayed call for the last call, let it be processed before
+    -- adding more
+    return true
+end)
+
+table.insert(steps, function()
+    wp.maximized(img, screen[1], nil, nil)
+
+    -- There is a delayed call for the last call, let it be processed before
+    -- adding more
+    return true
+end)
+
+table.insert(steps, function()
+
+    wp.maximized(img, screen[1], false, nil)
+    wp.maximized(img, screen[1], true, nil)
+
+    -- There is a delayed call for the last call, let it be processed before
+    -- adding more
+    return true
+end)
+
+table.insert(steps, function()
+
+    wp.maximized(img, screen[1], false, {x=10, y= 10})
+    wp.maximized(img, screen[1], true, {x=10, y= 10})
+
+    return true
+end)
+
+table.insert(steps, function()
+    wp.tiled(img, nil, nil)
+    wp.tiled(img, screen[1], nil)
+
+    -- There is a delayed call for the last call, let it be processed before
+    -- adding more
+    return true
+end)
+
+table.insert(steps, function()
+    wp.tiled(img, screen[1], {x=10, y= 10})
+
+    return true
+end)
+
+table.insert(steps, function()
+    for _, c in ipairs(colors) do
+        wp.set(c)
+    end
+
+    return true
+end)
+
+
+runner.run_steps(steps)
+
+-- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80