Elv13 / tyrannical

Dynamic tagging configuration system for awesomeWM
210 stars 33 forks source link

Correctly fullscreen clients #53

Closed getzze closed 9 years ago

getzze commented 9 years ago

This corrects a problem with fullscreen and dual screen. The problem also happens with maximized clients. When the second screen is not the same size as the first screen, new fullscreen clients (for instance Libreoffice presentation) seem to be first fullscreened in the first screen, then moved to the second screen. However it is not really fullscreened in the second screen, but it keeps the size of the first screen instead. This should be corrected directly in awful I assume, but this is a temporary patch. It basically set fullscreen to false then to true again once the client is in the proper screen.

This commit also correct a bug from PR #39, by replacing one instance of c.class that was omitted by get_class(c).

Elv13 commented 9 years ago

Hello, can you split the PR in 2? I would like to merge the This commit also correct a bug from PR #39, by replacing one instance of c.class that was omitted by get_class(c). part.

For the maximized part, I need further investigation. While obviously working and fixing a real problem, it doesn't "look" right. Maybe we could fix this upstream in Awesome itself rather than use such a workaround.

Thanks for contributing this!

getzze commented 9 years ago

I split it in two. I first wrote a monkey patch for awful.client.movetoscreen to include the resizing, but it was not sufficient. It seems the resizing is needed when the client screen is set also, like c.screen = 2 is called. This seems to be part of the awesome core and not awful... This patch is temporary I agree.

Elv13 commented 9 years ago

Right, this will need to be fixed upstream. I try to avoid adding any more hacks to Tyrannical like I used to. A while back I just fixed them upstream and dropped the code from Tyrannical. Can you creating a separate PR with the cherry picked Change forgotten c.class to get_class(c) commit? The other commit really need to be fixed upstream (in the core), I wont merge it.

getzze commented 9 years ago

Just for information, this patch is not needed anymore. I fixed the problem by adding these lines to my rc.lua :

-- Patch change screen for fullscreen
local function client_reload_max(c)
    local c = c or client.focus
    if not c then return end
    if c.maximized then
        --naughty.notify({text="Maximized ! " .. c.name, screen=c.screen})
        c.maximized = false
        c.maximized = true
    else
        if c.maximized_horizontal then
            c.maximized_horizontal = false
            c.maximized_horizontal = true
        end
        if c.maximized_vertical then
            c.maximized_vertical = false
            c.maximized_vertical = true
        end
    end
    if c.fullscreen then
        --naughty.notify({text="Fullscreen ! " .. c.name, screen=c.screen})
        c.fullscreen = false
        c.fullscreen = true
    end
end
-- Connect change screen signal to a resize function
client.connect_signal("property::screen", client_reload_max)
Elv13 commented 9 years ago

Ok, thanks for writing the patch anyway