GNOME Bugzilla – Bug 342899
Add wnck_window_set_geometry()
Last modified: 2006-07-20 17:45:58 UTC
The current version of libwnck (latest release, and in CVS as well) lacks a way of moving and resizing a window.
Created attachment 66176 [details] [review] Adding wnck_window_set_geometry() This patch adds wnck_window_set_geometry(). The implementation uses _NET_MOVERESIZE_WINDOW.
The patch doesn't set the source indication as required in the spec, so that needs to be fixed. I also dislike the use of -1 to mean not specifed, as that should be a perfectly valid x or y position (puts the window partially offscreen). This probably makes sense for completeness, but out of curiosity, what's the particular use case?
Use case -------- Well, the general use case basically is that Metacity sucks ;-) It doesn't really, but it's very limited in what window manipulations it can be configured to perform. I agree with and respect the design decision to keep Metacity lean and mean. At the same time I miss the configurability (or programmability) that other window managers offer. libwnck offers a nice interface for window manipulations, I think it's a better course than directly sending XEvents like wmctrl (http://sweb.cz/tripie/utils/wmctrl/) does. Also, libwnck has python bindings :-) (If libwnck is made more complete it might even be a possibility to remove some functionality from Metacity into an external tool. That might just be me being silly though...) Specifically, I've started writing a small commandline tool in python that can be used together with something like xbindkeys to manipulate windows from the keyboard. I need to be able to move and resize windows for it to realise its full potential. Source indication ----------------- I'll upload a modified patch as soon as I have it. -1 meaning "not specified" -------------------------- Yes, it's icky. Any suggestion on how to do it? All I can think of is a bitmask, would that do?
> Specifically, I've started writing a small commandline tool in python that can > be used together with something like xbindkeys to manipulate windows from the > keyboard. I need to be able to move and resize windows for it to realise its > full potential. I'm curious how this differs from e.g. Alt+F7 and Alt+F8. (I'm also curious about any new cool twists you have different from other window manipulation tools like devilspie, brightside, iswitchwin, and Superswitcher; and I guess wmctrl too) But then again, I'm just curious about these kinds of things in general. :) > (If libwnck is made more complete it might even be a possibility > to remove some functionality from Metacity into an external tool. That might > just be me being silly though...) Actually, the README file of metacity has an "HOW TO ADD EXTERNAL FEATURES" section specifically mentioning libwnck, so not so silly at all. ;-) Also, compiz is apparently using libwnck to do part of its window management tasks (in particular, in the gnome-window-decorator process) -- or at least so I was told in a separate libwnck feature request yesterday. > -1 meaning "not specified" > -------------------------- > Yes, it's icky. Any suggestion on how to do it? > All I can think of is a bitmask, would that do? Yeah, a bitmask is all I can think of too. That should be fine.
Created attachment 66231 [details] [review] Second attempt at a patch. Source, a geometry mask, and gravity added to the API.
(In reply to comment #4) > > Specifically, I've started writing a small commandline tool in python that can > > be used together with something like xbindkeys to manipulate windows from the > > keyboard. I need to be able to move and resize windows for it to realise its > > full potential. > > I'm curious how this differs from e.g. Alt+F7 and Alt+F8. (I'm also curious > about any new cool twists you have different from other window manipulation > tools like devilspie, brightside, iswitchwin, and Superswitcher; and I guess > wmctrl too) But then again, I'm just curious about these kinds of things in > general. :) Ah, cool. I had seen devilspie and superswitcher before, but brightside and iswitchwin are new to me. I'll have to take them for a ride :-) Basically my target is no-mouse.jl (http://sawfish.endorphin.org/SawfishWiki/NoMouse). That's the thing I miss most from my sawfish days. > > -1 meaning "not specified" > > -------------------------- > > Yes, it's icky. Any suggestion on how to do it? > > All I can think of is a bitmask, would that do? > > Yeah, a bitmask is all I can think of too. That should be fine. I've uploaded a patch already. Your comments are most welcome!
Comment on attachment 66231 [details] [review] Second attempt at a patch. >+ * @x: new X coordinate of window, -1 means don't modify >+ * @y: new Y coordinate of window, -1 means don't modify >+ * @width: new width of window, -1 means don't modify >+ * @height: new height of window, -1 means don't modify -1 stuff is history, right? >+ WnckWindowSource source, This shouldn't be part of the API; instead, xutils.c should call _wnck_get_client_type() and handle fixing up the gravity_and_flags stuff at that level. >+ gravity_flags |= source << 8; >+ gravity_flags |= source << 12; That doesn't look right. Did you mean to or with the shift geometry_mask the first time? >+typedef enum >+{ >+ WNCK_WINDOW_GRAVITY_NORTHWEST = 1, >+ WNCK_WINDOW_GRAVITY_NORTH, >+ WNCK_WINDOW_GRAVITY_NORTHEAST, >+ WNCK_WINDOW_GRAVITY_WEST, >+ WNCK_WINDOW_GRAVITY_CENTER, >+ WNCK_WINDOW_GRAVITY_EAST, >+ WNCK_WINDOW_GRAVITY_SOUTHWEST, >+ WNCK_WINDOW_GRAVITY_SOUTH, >+ WNCK_WINDOW_GRAVITY_SOUTHEAST >+} WnckWindowGravity; This doesn't allow for using the window's existing gravity, also, I'd prefer if each value were explicitly given (=2, =3, etc.) since it is precisely defined in the EWMH. (Prevent errors from others thinking they can just add a new value to the middle of the enum) >+typedef enum >+{ >+ WNCK_WINDOW_RESIZE_X = 1 << 0, >+ WNCK_WINDOW_RESIZE_Y = 1 << 1, >+ WNCK_WINDOW_RESIZE_WIDTH = 1 << 2, >+ WNCK_WINDOW_RESIZE_HEIGHT = 1 << 3, >+} WnckWindowResizePresence; I'd prefer a name like WnckWindowMoveResizeMask. Also, WNCK_WINDOW_RESIZE_X doesn't seem to make sense -- it's whether to _move_ in the x direction. I'd prefer something more like WNCK_WINDOW_CHANGE_X, WNCK_WINDOW_CHANGE_WIDTH, etc as that would be more consistent with ConfigureRequest events (see e.g. http://tronche.com/gui/x/xlib/window/configure.html#XWindowChanges), upon which _NET_MOVERESIZE_WINDOW is based. >+typedef enum >+{ >+ WNCK_WINDOW_SOURCE_APPLICATION = 1, >+ WNCK_WINDOW_SOURCE_PAGER = 2, >+ WNCK_WINDOW_SOURCE_TASKBAR = 2 >+} WnckWindowSource; WnckClientType is already defined in util.h. No need to define this thing twice. :) >+void _wnck_set_window_geometry (Screen *screen, >+ Window xwindow, No. Tabs. They're Evil(TM). ;-) You did this in a couple other places too now that I check. >+ int grflags, I know I'm being really nitpicky as this patch really is fine other than really minor things like this, but I'm a nit-picky kind of guy I guess. So, just one more: I'd prefer this were something like gravity_and_flags just to make the code a little clearer.
(In reply to comment #6) > Basically my target is no-mouse.jl > (http://sawfish.endorphin.org/SawfishWiki/NoMouse). That's the thing I miss > most from my sawfish days. Interesting. Metacity already allows you to bind keys to vertical/horizontal maximization. There was another bug open about adding (unbound by default) keybindings for moving windows to the corners (bug 317884) and another for "packing" windows (bug 118598). I could probably close one or both if you write this tool. :) Seems kind of lame though that the sawfish-nomouse thing assumed a panel width of 50 -- the tool ought to be able to figure out what panels exist and then avoid them. (However, if you want to be lazy, you can just make sure your requests are treated like an application's instead of a pager's -- application configure-requests and by extension _NET_WM_MOVERESIZE events are overridden by metacity in order to ensure the window stays "on screen", meaning not covered up by panels)
Created attachment 66251 [details] [review] Third attempt (addressing comment #7)
(In reply to comment #7) [..] > -1 stuff is history, right? I really ought to know beeter than create a patch that close to midnight :-) > >+ WnckWindowSource source, > > This shouldn't be part of the API; instead, xutils.c should call > _wnck_get_client_type() and handle fixing up the gravity_and_flags stuff at > that level. Oups, I should have thought about this a bit more carefully before uploading the third patch. I suspect you're saying that EWMH details shouldn't be in windows.c. I'll move the bit shifting to xutils.c at the same time as I address any other issues you find in that patch. > >+ gravity_flags |= source << 8; > >+ gravity_flags |= source << 12; > > That doesn't look right. Did you mean to or with the shift geometry_mask the > first time? See comment about creating patches close to midnight above. > >+typedef enum > >+{ [...] > >+} WnckWindowGravity; > > This doesn't allow for using the window's existing gravity, also, I'd prefer if > each value were explicitly given (=2, =3, etc.) since it is precisely defined > in the EWMH. (Prevent errors from others thinking they can just add a new > value to the middle of the enum) Good point. See what you think of the name of the constant I put in the patch. I've always found naming a bit tricky. > >+void _wnck_set_window_geometry (Screen *screen, > >+ Window xwindow, > > No. Tabs. They're Evil(TM). ;-) You did this in a couple other places too > now that I check. Yes, I know tabs are Evil(TM). They really shouldn't have been there. The only defense I have is that they appear in lines I copied from other functions in the files. I can put together a tab-removal patch for you later if you want ;-) > I know I'm being really nitpicky as this patch really is fine other than really > minor things like this, but I'm a nit-picky kind of guy I guess. No need to make excuses. You're the one who'll have to live with the code once the patch is in, so you have every right to be nit-picky. :-)
(In reply to comment #8) > (In reply to comment #6) > > Basically my target is no-mouse.jl > > (http://sawfish.endorphin.org/SawfishWiki/NoMouse). That's the thing I miss > > most from my sawfish days. > > Interesting. Metacity already allows you to bind keys to vertical/horizontal > maximization. There was another bug open about adding (unbound by default) > keybindings for moving windows to the corners (bug 317884) and another for > "packing" windows (bug 118598). I could probably close one or both if you > write this tool. :) Here comes more of my opinion again :-) I'm surprised the Metacity developers have entertained the bugs for this long. I know that Metacity has gotten a lot of "bad press" for being too limited and for having know-it-all developers that reject "usability enhancements" with comments along the lines of "Metacity is a minimal WM, that feature will NEVER be in it". At first that's what kept me away from Metacity (I'm still using Openbox but am planning to switch as soon as my tool is usable). Now it's actually what attracts me to it. I blame wmctrl for my renewed interest in Metacity, it opened up my eyes to what was really going on. AFAICS there are two ways of being minimal, either stick an entire programming language in the tool (Sawfish's approach), or implement standards (sometimes they might need to be created first) to let external applications influence the behaviour of the tool. I think Metacity is closer to the latter camp, and it could move even more firmly into it. I think that means Metacity implements some of the UNIX ideal: many small tools, each doing one thing really well. Some window manipulation might be difficult to do from an external application (interactive moving of the window, holding shift and left mouse button would be one such example, I think) but I don't see any point in Metacity having keybindings for shading, maximising, etc. xbindkeys and wmctrl would do that just as well. (I realise the configuring of those two tools isn't really up to what's expected of GNOME, but I think you get the idea.) > Seems kind of lame though that the sawfish-nomouse thing assumed a panel width > of 50 -- the tool ought to be able to figure out what panels exist and then > avoid them. (However, if you want to be lazy, you can just make sure your > requests are treated like an application's instead of a pager's -- application > configure-requests and by extension _NET_WM_MOVERESIZE events are overridden by > metacity in order to ensure the window stays "on screen", meaning not covered > up by panels) Yes, I remember having to edit the script slightly since my panel was 24 points. Later on it seemed the panel changed to 25 points (without my changing icon size), it took me a little while to figure out why my windows all of a sudden were covered just a little bit by the panel... Thanks for the tip about "application" versus "pager" behaviour. :-)
Comment on attachment 66251 [details] [review] Third attempt (addressing comment #7) >+ source = _wnck_get_client_type(); I had been thinking of putting this into xutils.c, but reading over your patch it looks just fine here. >+ WNCK_WINDOW_GRAVITY_EXISTING = 0, I'd prefer something like WNCK_WINDOW_GRAVITY_CURRENT. Vincent: do you have any alternative suggestions here? Other than renaming this one entry in the enum, patch looks great.
Created attachment 66316 [details] [review] Addressing comment #12
(In reply to comment #12) > (From update of attachment 66251 [details] [review] [edit]) > >+ WNCK_WINDOW_GRAVITY_EXISTING = 0, > > I'd prefer something like WNCK_WINDOW_GRAVITY_CURRENT. Vincent: do you have > any alternative suggestions here? CURRENT is better, yes. Elijah is the king here and he says the patch is okay. So I'll say it's okay to commit.
Committed.