> > 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

[EMAIL PROTECTED]:Download]$ file swp2.hg
swp2.hg: data

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

> 
> > 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?

> 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.


-- 
To unsubscribe, send mail to [EMAIL PROTECTED]

Reply via email to