On Sat, Nov 22, 2008 at 11:49 PM, Carlos R. Mafra <[EMAIL PROTECTED]> wrote: >> > Your patch unfortunately was broken by your email client >> > and does not apply due to broken lines. I tried to fix >> > it manually but gave up on line 334. >> > >> >> 100 left only Carlos, only 100. :) >> Seriously, sorry for all developpers for this quirk. >> I'll try to set up a mail client as soon as possible (I have busy weekend). >> I used the gmail webmail editor which truncates 'long' lines. I didn't >> know that. And too much trust in it. >> >> Until that you can use the Hg bundle I put on the web. >> You can analyze changsets from the bundle too (hg in bundleName). > > What is this Hg bundle? I downloaded the file, but it seems corrupted
http://www.selenic.com/mercurial/wiki/index.cgi/Bundle > > [EMAIL PROTECTED]:Download]$ file swp2.hg > swp2.hg: data > data != corrupted > I think you can send the patch attached, that avoids the broken > line issue. > > I want to apply your patch in my 'test' git repo to see how > it goes, but I am lazy to hand apply it. > > >> >> - if (somethingElse) >> >> - WMHandleEvent(&ev); >> >> } >> > >> > Why are you removing this? Wasn't it some type of "ok, we don't understand >> > what the user wanted to do so let's proceed" thing? >> > >> >> Are you talking about the 'something else'? >> This default comportement is preserved, look above. >> >> Why is it removed from the end of the file? >> the old code is justified by closing the "switch panel" and proceeding >> the actions. In this order. >> >> Now actions are imediate (like shading) while in the panel. >> the exception to this are closing action and focus, which is the main >> interest of the "switch panel" existance. >> these two actions are proceeded in the end of life of the "switch panel" > > Ok, thanks for the explanation. > > >> >> - >> >> - if (canReceiveFocus(WMGetFromArray(panel->windows, idecks)) < 0) >> >> + >> >> + Window *theWindow = WMGetFromArray(panel->windows, idecks); >> >> + if (canReceiveFocus(theWindow) < 0){ >> > >> > I like this kind of cleanup in general, where you move the assignment out >> > of the if(), but leave it to another patch. Furthermore, I think it would >> > be better to declare *theWindow together with the other variables further >> > above. >> > >> >> can you be more precise? > > I think code is more readable when there is not too much going on inside > the if(). > > For example > > if (check_err(something)) { > do stuff > } > > looks better to me like this > > err = check_err(something); > if (err) { > do stuff > } > > and that is what you partially did in the above, by defining the > variable 'theWindow'. > > And it would be even nicer if you defined theWindow together with the > other variables in the beginning of the function, so it would be like > this > > WMFrame *icon = WMGetFromArray(panel->icons, idecks); > RImage *image= WMGetFromArray(panel->images, idecks); > WWindow *theWindow = WMGetFromArray(panel->windows, idecks); > > stuff > > if (canReceiveFocus(theWindow) < 0) > opaq= 50; > > other stuff > me too, but I like more this if (...) { ... } less problem maker :D >> >> > By the way, won't gcc warn you about the above modification? I tried it >> > here >> > and got >> > >> > switchpanel.c:117: warning: passing argument 1 of 'canReceiveFocus' from >> > incompatible pointer type >> > >> > but I did not check anything further. >> > >> >> >> hum, to be checked. I'll look at it. > > I looked briefly at it, and it appears you have a typo in > > + Window *theWindow = WMGetFromArray(panel->windows, idecks); > ^^^^^ > WWindow > > using WWindow silences gcc here. > > One thing I find strange is that the result of WMGetFromArray() is a > pointer to void, and canReceiveFocus() expects a pointer to WWindow as > its argument, so the old code was not being careful about this. > > But in your version of the code wouldn't it be necessary to add a cast like > this? > > + WWindow *theWindow = (WWindow *)WMGetFromArray(panel->windows, idecks); > > for correctedness? thx > >> I didn't share this code for a final code to put inside wmaker though. >> I wanted to present you the feature first I don't know if it takes >> place in wmaker style of WM, but I was too much exited about it :) > > Ok. > > I think that everything you can do with the keyboard has something to > do with wmaker style :-) > > But I guess your patch is a bit cumbersome in the sense that you have to > keep pressed the mod1 and then the kill shortcut. > > And in my opinion the patch is also a bit strange in its purpose. When I use > mod1+tab to switch windows, the selected windows come to the foreground. > Then I don't know why it is not better to simply select the window you > want and then kill it by the usual means. > > That said, I am still not convinced that I would want such a feature. > > But the patch looked ok to my untrained eyes, let's see what other people > have to say about it too. > > It depends on how you use wmaker. Let say zepard opens lot of windows (some of them heavy java apps by the way). Zepard wants to quit 6 of them. MR.Zepard will alt-tab close 6 times (and each time tab to set the focus on the right window) with this feature, zepard will alt-tab once and close/unclose 6 times regards -- To unsubscribe, send mail to [EMAIL PROTECTED]
