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.
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?
> - 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.
> - } 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?
>
> 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?
> 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 :-)
> + 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.
> 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.
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.
> 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?
> + 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]
--
To unsubscribe, send mail to [EMAIL PROTECTED]
