Open jrdietrick opened 9 years ago
Hi,
thanks for your patches, I have no time to review them not but I will try to take a look during the weekend. Just one quick note: the gdk_screen_get_monitor_workarea
function is kind of broken and only works for the primary screen. I have been pushing for a fix but that requires an extension of the EWMH specification, you may track that here and here.
Those patches look good in general, but I have some issues with it.
First I don't agree with the <Primary>Right
default hotkey, that is far too likely to be in use by other applications in which case we either end up with a broken hotkey or we block the hotkey for other applications. How about using <Super><Shift>Right
, anything with super is usually not likely to be taken by some other application.
Second, In the first patch it you need to also change the size of the tilda window, look at combo_monitor_selection_changed_cb
how I compare current_rectangle
and last_rectangle
and then resize the size. This is not prefect, because as I wrote in my last comment the gdk_screen_get_monitor_workarea
function somethings reports wrong work area sizes, but in general this works. Without this you might be moving the tilda window from your desktop monitor to a smaller notebook monitor and then the size is way too big there.
Third, your code duplicates part of the code in combo_monitor_selection_changed_cb
, please move that into a common function which is called from both callbacks, so we do not end up with duplicate code.
I am planning to release 1.3 soon (probably end of October), so if you want this to end up in 1.3 then please send me a rebased version with the the fixes soon.
Appreciate the review and feedback! Agreed on all counts. I'll work up a revised set of patches and get it back to you.
Thanks!
Just a quick heads up, I pushed some commits from the wip/monitornames (but not all of them), so Tilda now makes use of the monitor name instead of the monitor number, please consider that when you rebase your patches
Awesome! Will rework on top of those, hopefully within the next day or two. Thanks.
I was gazing longingly at https://github.com/lanoxx/tilda/tree/wip/monitornames. That branch is ambitious, and I would love to see it come all the way through at some point, but this is a simple solution that gets me 95% of the way there.
Between home and the office, I have at least three different multi-monitor workstations I plug into on a weekly basis, each with a different monitor type and physical geometry. Sometimes the external monitor is on the left (and thus is monitor #0), and sometimes it's on the right. Sometimes my terminal is front and center, and I want it on my tiny 1366x768 laptop display, and sometimes I want it on the secondary display. Etc.
All this adds up to a lot of diving into the wizard to jam the "Select monitor" combo box. So I wanted a keybinding to just move tilda to the next monitor.
As described in e5805e706, I made a call about how the window should be positioned on the destination monitor in the likely event that the workarea sizes are different. I think all three options (the implemented one, plus the two alternatives listed in that commit msg) are pretty reasonable, and would be happy to modify this patch to implement any of them.