On Sat, Nov 22, 2008 at 5:25 PM, Carlos R. Mafra <[EMAIL PROTECTED]> wrote:
> On Sat 22.Nov'08 at 13:09:11 +0100, Samir SAADA wrote:
>> I prefer the patch too, but didn't know about the --git option of hg
>> export command :)
>> here is the patch.
>
> 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).
> Can you resend it trying to avoid this problem?
>
> [ there are a set of tips in Documentation/email-clients.txt
> in the linux kernel source code for dealing with this situation ]
>
> Ok, I still don't know exactly how your patch works so I have a few
> questions and very simple and naive comments, just from a brief
> look at the patch.
>
>> We can handle windows while swithing windows (ie alt tabbing) in the
>> switch pannel.
>> ex: alt+tab + [ minimize | hide | close | shade ... ]
>>
>> these operations should be initialized with shortcuts
>> containing the 'alt' (or equivalent) key of the switching shortcut.
>>
>> ex: switch window --> mod1+tab
>> close window --> mod1+k
>
> So the idea is that you press mod1+tab till you select the desired
> window and then you press 'k' to kill it while keeping the mod1 key
> pressed?
>
> Why can't you simply mod1+tab to select the window and _then_ kill
> it with whatever shortcut you have for doing so?
>
you can close the window with the shortcut of your choise, with an
anoying constraint, is that you keep the persistant key (in my example
mode1) pressed to still have the "switch panel" visible otherwise the
panel will be closed.
so if you want to close a window an still have your panel visible, you
have to choose a 'close window' shortcut with a mode1 (the persistant
key to keep the panel visible) in it.
This is a constraint and i should find a solution for it.
If this explenation is not clear I'll put a video on the web ;)
>
>> - XEvent ev;
>> + XEvent ev;
>
> To the cleanup patch separatedly, as it distracts reviewing the new code.
> There were other cases like this in your patch.
didn't note this one will look at them.
>
>> - } else if (ev.xkey.keycode != shiftLKey &&
>> ev.xkey.keycode != shiftRKey) {
>> + } else if (wKeyBindings[WKBD_CLOSE].keycode == ev.xkey.keycode)
>> {
>> +#ifdef DEBUG
>> + printf("Got closing Action\n");
>> +#endif
>> + if (swpanel) {
>> + closingAction = True;
>> + int winIndex = WMFindInArray(windowsToClose,
>> NULL, newFocused);
>> + if (winIndex != WANotFound) {
>> + WMDeleteFromArray(windowsToClose, winIndex);
>> + wSwitchPanelMarkCurrent(swpanel, 0);
>> + } else {
>> + WMAddToArray(windowsToClose, newFocused);
>> + closingEvent = ev;//>>>>>>>:( ugly I know
>> + wSwitchPanelMarkCurrent(swpanel, 1);
>> + }
>> + }
>> + } else if (ev.xkey.keycode != shiftLKey && ev.xkey.keycode !=
>> shiftRKey) {
>> #ifdef DEBUG
>> printf("Got something else\n");
>> #endif
>> - somethingElse = True;
>> - done = True;
>> - }
>> + if(WMFindInArray(windowsToClose, NULL, newFocused) ==
>> WANotFound){
>> + wSetFocusTo(scr, newFocused);
>> + WMHandleEvent(&ev);
>> + wSwitchPanelRefreshImages(swpanel, newFocused);
>> + }
>> + }
>> break;
>> case KeyRelease:
>> #ifdef DEBUG
>> @@ -249,19 +270,27 @@
>>
>> if (swpanel)
>> wSwitchPanelDestroy(swpanel);
>> -
>> - if (newFocused) {
>> - wRaiseFrame(newFocused->frame->core);
>> - CommitStacking(scr);
>> - if (!newFocused->flags.mapped)
>> - wMakeWindowVisible(newFocused);
>> - wSetFocusTo(scr, newFocused);
>> +
>> + if (closingAction){
>> + WWindow *aWindow;
>> + int i;
>> + WM_ITERATE_ARRAY(windowsToClose, aWindow , i){
>> + wSetFocusTo(scr, aWindow);
>> + WMHandleEvent(&closingEvent);
>> + }
>> + WMEmptyArray(windowsToClose);
>> }
>> + else
>> + if (newFocused) {
>
> else if
>
>> + wRaiseFrame(newFocused->frame->core);
>> + CommitStacking(scr);
>> + if (!newFocused->flags.mapped)
>> + wMakeWindowVisible(newFocused);
>> + wSetFocusTo(scr, newFocused);
>> + }
>>
>> scr->flags.doing_alt_tab = 0;
>
>
>> - 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"
>>
>> diff --git a/src/defaults.c b/src/defaults.c
>> --- a/src/defaults.c
>> +++ b/src/defaults.c
>> @@ -659,7 +659,7 @@
>> {"IconTitleBack", "black", NULL,
>> NULL, getColor, setIconTitleBack
>> },
>> - {"SwitchPanelImages", "(swtile.png, swback.png, 30, 40)",
>> &wPreferences,
>> + {"SwitchPanelImages", "(swtile.png, swback.png, 30, 40,
>> swkill.png)", &wPreferences,
>> NULL, getPropList, setSwPOptions
>> },
>> /* keybindings */
>> @@ -3585,14 +3585,18 @@
>> if (!WMIsPLArray(array) || WMGetPropListItemCount(array)==0) {
>> if (prefs->swtileImage) RReleaseImage(prefs->swtileImage);
>> prefs->swtileImage= NULL;
>> -
>> +
>> + if (prefs->swkillImage) RReleaseImage(prefs->swkillImage);
>> + prefs->swkillImage= NULL;
>> +
>> +
>> WMReleasePropList(array);
>> return 0;
>> }
>>
>> switch (WMGetPropListItemCount(array))
>> {
>> - case 4:
>> + case 5:
>
> what happened to case 4? Is it no longer necessary?
>
>
I don't think so. I'll be pleased to keep it if its necessity is proven.
>> if (!WMIsPLString(WMGetFromPLArray(array, 1))) {
>> wwarning(_("Invalid arguments for option \"%s\""),
>> entry->key);
>> @@ -3687,8 +3691,34 @@
>> wwarning(_("Could not load image \"%s\" for option \"%s\""),
>> path, entry->key);
>> }
>> +
>> wfree(path);
>> }
>> +
>> + //-----------------------------------------------------------
>
> don't uglify the source :-)
matter of taste :p
>
>> + if (!WMIsPLString(WMGetFromPLArray(array, 4))) {
>> + wwarning(_("Invalid arguments for option \"%s\""),
>> + entry->key);
>> + break;
>> + } else
>> + path= FindImage(wPreferences.pixmap_path,
>> WMGetFromPLString(WMGetFromPLArray(array, 4)));
>> +
>> + if (!path) {
>> + wwarning(_("Could not find image \"%s\" for option \"%s\""),
>> + WMGetFromPLString(WMGetFromPLArray(array, 4)),
>> + entry->key);
>> + } else {
>> + if (prefs->swkillImage) RReleaseImage(prefs->swkillImage);
>> +
>> + prefs->swkillImage= RLoadImage(scr->rcontext, path, 4);
>> + if (!prefs->swkillImage) {
>> + wwarning(_("Could not load image \"%s\" for option \"%s\""),
>> + path, entry->key);
>> + }
>> +
>> + wfree(path);
>> + }
>> + //-------------------------------------------------------------
>
> ditto.
:p
>
>> break;
>>
>> default:
>> diff --git a/src/event.c b/src/event.c
>> --- a/src/event.c
>> +++ b/src/event.c
>> @@ -1359,8 +1359,7 @@
>> break;
>> }
>> }
>> -
>> -
>> +
>> if (command < 0) {
>> #ifdef LITE
>> {
>> @@ -1523,7 +1522,12 @@
>> break;
>> case WKBD_CLOSE:
>> if (ISMAPPED(wwin) && ISFOCUSED(wwin) && !WFLAGP(wwin,
>> no_closable)) {
>> - CloseWindowMenu(scr);
>> + CloseWindowMenu(scr);
>> + if (wwin->protocols.DELETE_WINDOW)
>> + wClientSendProtocol(wwin, _XA_WM_DELETE_WINDOW,
>> + event->xkey.time);
>> + }
>> + else if(scr->flags.doing_alt_tab){
>> if (wwin->protocols.DELETE_WINDOW)
>> wClientSendProtocol(wwin, _XA_WM_DELETE_WINDOW,
>> event->xkey.time);
>> diff --git a/src/switchpanel.c b/src/switchpanel.c
>> --- a/src/switchpanel.c
>> +++ b/src/switchpanel.c
>> @@ -50,6 +50,7 @@
>> WMArray *icons;
>> WMArray *images;
>> WMArray *windows;
>> + WMArray *markedWindows;
>> RImage *bg;
>> int current;
>> int firstVisible;
>> @@ -61,6 +62,7 @@
>>
>> RImage *tileTmp;
>> RImage *tile;
>> + RImage *kill;
>>
>> WMFont *font;
>> WMColor *white;
>> @@ -75,6 +77,7 @@
>> #define BORDER_SPACE 10
>> #define ICON_SIZE 48
>> #define ICON_TILE_SIZE 64
>> +#define ICON_KILL_SIZE 64
>> #define LABEL_HEIGHT 25
>> #define SCREEN_BORDER_SPACING 2*20
>> #define SCROLL_STEPS (ICON_TILE_SIZE/2)
>> @@ -110,11 +113,15 @@
>> RImage *back;
>> int opaq= 255;
>> RImage *tile;
>> + RImage *kill;
>> WMPoint pos;
>> +
>> Pixmap p;
>> -
>> - 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?
> 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.
>> opaq= 50;
>> + }
>
> There is no need to introduce { and } here.
>
>>
>> pos= WMGetViewPosition(WMWidgetView(icon));
>> back= panel->tileTmp;
>> @@ -138,10 +145,18 @@
>> tile= panel->tile;
>> RCombineArea(back, tile, 0, 0, tile->width, tile->height,
>> (back->width - tile->width)/2, (back->height
>> - tile->height)/2);
>> - }
>> + }
>> +
>> +
>> RCombineAreaWithOpaqueness(back, image, 0, 0, image->width,
>> image->height,
>> (back->width - image->width)/2,
>> (back->height - image->height)/2,
>> opaq);
>> + if (selected == -1 ||
>> + WMFindInArray(panel->markedWindows, NULL, theWindow) !=
>> WANotFound) {
>> + kill= panel->kill;
>> + RCombineArea(back, kill, 0, 0, kill->width, kill->height,
>> + (back->width - kill->width)/2, (back->height
>> - kill->height)/2);
>> + }
>>
>> RConvertImage(panel->scr->rcontext, back, &p);
>> XSetWindowBackgroundPixmap(dpy, WMWidgetXID(icon), p);
>> @@ -353,6 +368,20 @@
>> return stile;
>> }
>>
>> +static RImage *getKill(WSwitchPanel *panel)
>> +{
>> + RImage *sKill;
>> +
>> + if (!wPreferences.swkillImage)
>> + return NULL;
>> +
>> + sKill = RScaleImage(wPreferences.swkillImage, ICON_KILL_SIZE,
>> ICON_KILL_SIZE);
>> + if (!sKill)
>> + return wPreferences.swkillImage;
>> +
>> + return sKill;
>> +}
>> +
>>
>> static void
>> drawTitle(WSwitchPanel *panel, int idecks, char *title)
>> @@ -452,6 +481,9 @@
>>
>> panel->windows= makeWindowListArray(scr, curwin, workspace,
>> wPreferences.swtileImage!=0);
>> +
>> + panel->markedWindows= WMCreateArray(1);
>> +
>> count= WMGetArrayItemCount(panel->windows);
>>
>> if (count == 0) {
>> @@ -477,6 +509,7 @@
>>
>> panel->tileTmp= RCreateImage(ICON_TILE_SIZE, ICON_TILE_SIZE, 1);
>> panel->tile= getTile(panel);
>> + panel->kill= getKill(panel);
>> if (panel->tile && wPreferences.swbackImage[8]) {
>> panel->bg= createBackImage(scr, width+2*BORDER_SPACE,
>> height+2*BORDER_SPACE);
>> }
>> @@ -490,6 +523,9 @@
>> if (panel->tileTmp)
>> RReleaseImage(panel->tileTmp);
>> panel->tileTmp= NULL;
>> + if (panel->kill)
>> + RReleaseImage(panel->kill);
>> + panel->kill= NULL;
>> }
>>
>> panel->white= WMWhiteColor(scr->wmscreen);
>> @@ -602,6 +638,8 @@
>> RReleaseImage(panel->tile);
>> if (panel->tileTmp)
>> RReleaseImage(panel->tileTmp);
>> + if (panel->kill)
>> + RReleaseImage(panel->kill);
>> if (panel->bg)
>> RReleaseImage(panel->bg);
>> if (panel->font)
>> @@ -609,6 +647,37 @@
>> if (panel->white)
>> WMReleaseColor(panel->white);
>> wfree(panel);
>> +}
>> +
>> +
>> +void wSwitchPanelRefreshImages(WSwitchPanel *panel, WWindow *focus){
>> + WWindow *wwin;
>> + int i;
>> + //if it is faster to check application than rendering all windows
>> + Window *win = (Window*)wApplicationOf(focus->main_window);
>> + WM_ITERATE_ARRAY(panel->windows, wwin, i) {
>
> Isn't "i" used unitialized here?
Look at how WM_ITERATE_ARRAY is used in the code.
If there is a security problem I should change it of course. and some
occurences of this macro elsewhere
>
>
>> + if(win == (Window*)wApplicationOf(wwin->main_window))
>> + changeImage(panel, i, 0);
>> + }
>> +
>> + if (panel->current >= 0)
>> + changeImage(panel, panel->current, 1);
>> +
>> +}
>> +
>> +
>> +void wSwitchPanelMarkCurrent(WSwitchPanel *panel, int mark){
>> + if (panel->current >= 0){
>> + if(mark){
>
> Please be consistent with the spacings around if().
>
>> + WMAddToArray(panel->markedWindows,
>> + WMGetFromArray(panel->windows, panel->current));
>> + changeImage(panel, panel->current, -1);
>> + }else{
>
> ditto.
>
>> + WMRemoveFromArray(panel->markedWindows,
>> + WMGetFromArray(panel->windows,
>> panel->current));
>> + changeImage(panel, panel->current, 1);
>> + }
>> + }
>> }
>>
>>
>> diff --git a/src/switchpanel.h b/src/switchpanel.h
>> --- a/src/switchpanel.h
>> +++ b/src/switchpanel.h
>> @@ -35,4 +35,8 @@
>>
>> Window wSwitchPanelGetWindow(WSwitchPanel *swpanel);
>>
>> +void wSwitchPanelRefreshImages(WSwitchPanel *panel, WWindow *wwin);
>> +
>> +void wSwitchPanelMarkCurrent(WSwitchPanel *panel, int mark);
>> +
>> #endif /* _SWITCHPANEL_H_ */
>>
>>
>> --
>> To unsubscribe, send mail to [EMAIL PROTECTED]
>
You are a patchy guy Carlos :)
thx for contructive notes.
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 :)
Regards.
--
To unsubscribe, send mail to [EMAIL PROTECTED]
