Re: [PATCH] rewrite of maximize functions
On Thu, 18 Apr 2013 at 0:10:12 -0300, Renan Traba wrote: > simplified logic of handleMaximize function You should write a separate patch for this item alone. > added maximize on top and bottom half and added maximize at 4 corners: > left top, right top, left bottom, right bottom ditto. > removed unneeded old_maximize flags ditto. > added to WPrefs options to configure new maximize modes ditto. > --- > WPrefs.app/KeyboardShortcuts.c | 12 ++ > src/actions.c | 253 > - > src/actions.h | 8 +- > src/defaults.c | 12 ++ > src/event.c| 42 +++ > src/keybind.h | 8 +- > src/window.h | 3 +- > 7 files changed, 175 insertions(+), 163 deletions(-) Since otherwise the patch is big and if a regression is found it is much harder to revert it. And even more importantly, did you address the comments by Iain? If so, that should be mentioned too. If not, please explain the reason. Is it really necessary to rewrite the handleMaximize() logic in order to add the left/right top/bottom maximize? If not, I'd suggest that your first patches should simply add the new functionality and only the later patches should rewrite the logic. -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
[PATCH] rewrite of maximize functions
simplified logic of handleMaximize function
added maximize on top and bottom half and added maximize at 4 corners:
left top, right top, left bottom, right bottom
removed unneeded old_maximize flags
added to WPrefs options to configure new maximize modes
---
WPrefs.app/KeyboardShortcuts.c | 12 ++
src/actions.c | 253 -
src/actions.h | 8 +-
src/defaults.c | 12 ++
src/event.c| 42 +++
src/keybind.h | 8 +-
src/window.h | 3 +-
7 files changed, 175 insertions(+), 163 deletions(-)
diff --git a/WPrefs.app/KeyboardShortcuts.c b/WPrefs.app/KeyboardShortcuts.c
index 61a49a1..98bd6cc 100644
--- a/WPrefs.app/KeyboardShortcuts.c
+++ b/WPrefs.app/KeyboardShortcuts.c
@@ -78,6 +78,12 @@ static char *keyOptions[] = {
"HMaximizeKey",
"LHMaximizeKey",
"RHMaximizeKey",
+ "THMaximizeKey",
+ "BHMaximizeKey",
+ "LTCMaximizeKey",
+ "RTCMaximizeKey",
+ "LBCMaximizeKey",
+ "RBCMaximizeKey",
"MaximusKey",
"RaiseKey",
"LowerKey",
@@ -482,6 +488,12 @@ static void createPanel(Panel * p)
WMAddListItem(panel->actLs, _("Maximize active window horizontally"));
WMAddListItem(panel->actLs, _("Maximize active window left half"));
WMAddListItem(panel->actLs, _("Maximize active window right half"));
+ WMAddListItem(panel->actLs, _("Maximize active window top half"));
+ WMAddListItem(panel->actLs, _("Maximize active window bottom half"));
+ WMAddListItem(panel->actLs, _("Maximize active window left top
corner"));
+ WMAddListItem(panel->actLs, _("Maximize active window right top
corner"));
+ WMAddListItem(panel->actLs, _("Maximize active window left bottom
corner"));
+ WMAddListItem(panel->actLs, _("Maximize active window right bottom
corner"));
WMAddListItem(panel->actLs, _("Maximus: Tiled maximization "));
WMAddListItem(panel->actLs, _("Raise active window"));
WMAddListItem(panel->actLs, _("Lower active window"));
diff --git a/src/actions.c b/src/actions.c
index 594b464..6e6dbf2 100644
--- a/src/actions.c
+++ b/src/actions.c
@@ -342,50 +342,37 @@ void update_saved_geometry(WWindow *wwin)
save_old_geometry(wwin, SAVE_GEOMETRY_X);
}
-#define IS_MAX_HORIZONTALLY(directions) ((directions & MAX_HORIZONTAL) |
(directions & MAX_LEFTHALF) | (directions & MAX_RIGHTHALF))
void wMaximizeWindow(WWindow *wwin, int directions)
{
- int new_x, new_y;
- unsigned int new_width, new_height, half_scr_width;
+ unsigned int half_scr_width, half_scr_height;
+ int new_x = 0;
+ int new_y = 0;
+ unsigned int new_width = 0;
+ unsigned int new_height = 0;
int maximus_x = 0;
int maximus_y = 0;
unsigned int maximus_width = 0;
unsigned int maximus_height = 0;
+ int adj_size;
WArea usableArea, totalArea;
Bool has_border = 1;
- int save_directions = 0;
- int adj_size;
if (!IS_RESIZABLE(wwin))
return;
- if (!HAS_BORDER(wwin))
- has_border = 0;
+ /* save old coordinates before we change the current values */
+ if (!wwin->flags.maximized)
+ save_old_geometry(wwin, SAVE_GEOMETRY_ALL);
/* the size to adjust the geometry */
+ if (!HAS_BORDER(wwin))
+ has_border = 0;
adj_size = wwin->screen_ptr->frame_border_width * 2 * has_border;
- /* save old coordinates before we change the current values
-* always if the window is not currently maximized at all
-* but never if the window has been Maximusized */
- if (!wwin->flags.maximized)
- save_directions |= SAVE_GEOMETRY_ALL;
- else if (!(wwin->flags.old_maximized & MAX_MAXIMUS)) {
- if ((directions & MAX_VERTICAL) &&
- !(wwin->flags.maximized & MAX_VERTICAL))
- save_directions |= SAVE_GEOMETRY_Y |
SAVE_GEOMETRY_HEIGHT;
- if (IS_MAX_HORIZONTALLY(directions) &&
- !IS_MAX_HORIZONTALLY(wwin->flags.maximized))
- save_directions |= SAVE_GEOMETRY_X |
SAVE_GEOMETRY_WIDTH;
- }
- if ((directions & MAX_MAXIMUS) && !wwin->flags.maximized)
- save_directions |= SAVE_GEOMETRY_ALL;
- save_old_geometry(wwin, save_directions);
-
- totalArea.x1 = 0;
- totalArea.y1 = 0;
totalArea.x2 = wwin->screen_ptr->scr_width;
totalArea.y2 = wwin->screen_ptr->scr_height;
+ totalArea.x1 = 0;
+ totalArea.y1 = 0;
usableArea = totalArea;
if (!(directions & MAX_IGNORE_XINERAMA)) {
@@ -400,89 +387,76 @@ void wMaximizeWindow(WWindow *wwin, int directions)
usableArea = wGetUsableAreaForHead(scr, head, &totalArea, True);
}
- /* remember Ma
[repo.or.cz] wmaker-crm.git branch next updated: wmaker-0.95.4-89-g29a5267
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project wmaker-crm.git.
The branch, next has been updated
via 29a526748553e11a1bcd7400e4cfc16c1f5351ed (commit)
via 41da1b30dbfe09aef2658f4feac9021bde0d31a9 (commit)
from 25b5ca256614020150ecf5f13637d1e931ead666 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -
http://repo.or.cz/w/wmaker-crm.git/commit/29a526748553e11a1bcd7400e4cfc16c1f5351ed
commit 29a526748553e11a1bcd7400e4cfc16c1f5351ed
Author: Rodolfo GarcÃa Peñas (kix)
Date: Sun Apr 14 23:13:27 2013 +0200
icon_update_pixmap default moved to bottom
The default case is moved to the bottom of the switch case.
The default case should be removed, because the icon has always
a right value, because the icon creation always uses a real value:
kix@debian:~/src/wmaker/wmaker-crm/src$ grep wAppIconCreateForDock *c
appicon.c:WAppIcon *wAppIconCreateForDock(WScreen *scr, char *command, char
*wm_instance, char *wm_class, int tile)
dock.c: btn = wAppIconCreateForDock(scr, NULL, "Logo", "WMClip",
TILE_CLIP);
dock.c: btn = wAppIconCreateForDock(scr, NULL, "Logo", "WMDock",
TILE_NORMAL);
dock.c: btn = wAppIconCreateForDock(scr, NULL, name, "WMDrawer",
TILE_DRAWER);
dock.c: aicon = wAppIconCreateForDock(scr, command, winstance, wclass,
TILE_NORMAL);
(1)dock.c: aicon =
wAppIconCreateForDock(dock->screen_ptr, NULL,
kix@debian:~/src/wmaker/wmaker-crm/src$
kix@debian:~/src/wmaker/wmaker-crm/src$ grep TILE_ *c | grep -v
ICON_TILE_SIZE
***[2]appicon.c: tile = TILE_CLIP;
dock.c: btn = wAppIconCreateForDock(scr, NULL, "Logo", "WMClip",
TILE_CLIP);
dock.c: btn = wAppIconCreateForDock(scr, NULL, "Logo", "WMDock",
TILE_NORMAL);
dock.c: btn = wAppIconCreateForDock(scr, NULL, name, "WMDrawer",
TILE_DRAWER);
dock.c: aicon = wAppIconCreateForDock(scr, command, winstance, wclass,
TILE_NORMAL);
(2)dock.c:
wm_instance, wm_class, TILE_NORMAL);
***[3]icon.c: icon->tile_type = TILE_NORMAL;
icon.c: case TILE_NORMAL:
icon.c: case TILE_CLIP:
icon.c: case TILE_DRAWER:
kix@debian:~/src/wmaker/wmaker-crm/src$ grep tile_type *c
icon.c: icon->tile_type = TILE_NORMAL;
***[1]icon.c: icon->tile_type = tile;
icon.c: switch (icon->tile_type) {
icon.c: wwarning("Unknown tile type: %d.n", icon->tile_type);
kix@debian:~/src/wmaker/wmaker-crm/src$
There are only three cases without value (asterisk in the line start) set
as preprocessor variable. (1) and (2) is the same call. These are the three
cases:
Case [1]:
-8<--
WIcon *icon_create_for_dock(WScreen *scr, char *command, char *wm_instance,
char *wm_class, int tile)
{
WIcon *icon;
icon = icon_create_core(scr, 0, 0);
icon->tile_type = tile;
-8<--
Calls to icon_create_for_dock, is only call in appicon.c:
-8<--
kix@debian:~/src/wmaker/wmaker-crm/src$ grep icon_create_for_dock *c
appicon.c: aicon->icon = icon_create_for_dock(scr, command,
wm_instance, wm_class, tile);
icon.c:WIcon *icon_create_for_dock(WScreen *scr, char *command, char
*wm_instance, char *wm_class, int tile)
kix@debian:~/src/wmaker/wmaker-crm/src$
-8<--
The call:
-8<--
WAppIcon *wAppIconCreateForDock(WScreen *scr, char *command, char
*wm_instance, char *wm_class, int tile)
{
[snip]
if (strcmp(wm_class, "WMDock") == 0 &&
wPreferences.flags.clip_merged_in_dock)
tile = TILE_CLIP;
aicon->icon = icon_create_for_dock(scr, command, wm_instance,
wm_class, tile);
-8<--
And the calls to wAppIconCreateForDock() are checked before.
The case [2] is just the line:
-8<--
WAppIcon *wAppIconCreateForDock(WScreen *scr, char *command, char
*wm_instance, char *wm_class, int tile)
{
[snip]
if (strcmp(wm_class, "WMDock") == 0 &&
wPreferences.flags.clip_merged_in_dock)
*** tile = TILE_CLIP;
aicon->icon = icon_create_for_dock(scr, command, wm_instance,
wm_class, tile);
-8<--
Then, is sure too.
The case [3] is:
-8<--
WIcon *icon_create_for_wwindow(WWindow *wwin)
{
[snip]
icon->tile_type = TILE_NORMAL;
-
Re: [PATCH 3/8] Add the application to the wapp_list
Quoting "Carlos R. Mafra" :
On Sun, 14 Apr 2013 at 23:13:28 +0200, Rodolfo García Peñas (kix) wrote:
From: "Rodolfo García Peñas (kix)"
The screen has a list of applications:
8<
kix@debian:~/src/wmaker/wmaker-crm/src$ grep wapp_list screen.h
struct WApplication *wapp_list;/* list of all aplications */
kix@debian:~/src/wmaker/wmaker-crm/src$
8<
But this variable was not set before this patch:
- Was removed in application.c
- Local variable in session.c
8<
kix@debian:~/src/wmaker/wmaker-crm/src$ grep wapp_list *.c
application.c: if (wapp == scr->wapp_list) {
application.c: scr->wapp_list = wapp->next;
session.c: WMArray *wapp_list = NULL;
session.c: wapp_list = WMCreateArray(16);
session.c:&& (WMGetFirstInArray(wapp_list, (void *)appId)
== WANotFound
session.c: WMAddToArray(wapp_list, (void *)appId);
session.c: WMFreeArray(wapp_list);
kix@debian:~/src/wmaker/wmaker-crm/src$
8<
This variable is needed to restore the screen without re-create the
applications structs.
---
src/application.c |4
1 file changed, 4 insertions(+)
diff --git a/src/application.c b/src/application.c
index abe1d2d..6a03775 100644
--- a/src/application.c
+++ b/src/application.c
@@ -143,6 +143,10 @@ WApplication *wApplicationCreate(WWindow * wwin)
create_appicon_for_application(wapp, wwin);
+ /* Save the application in the application list */
+ wapp->next = scr->wapp_list;
+ scr->wapp_list = wapp;
If wapp_list was not set before then wapp->next will contain junk
at this point, no?
Yes. Good point. We should initialize the wapp_list when wmaker starts.
This list is needed to re-position the windows, for example with
XRandR screen change.
We have a different list for appicons. See src->app_icon_list. Is the
same idea.
With XRandR, we should follow the app_icon_list to re-position the
appicons and then paint it (move them). And we should follow the
wapp_list to to the same with the windows.
I didn't found where the app_icon_list is initialized.
Cheers,
kix
Rodolfo García Peñas (kix)
http://www.kix.es/
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [PATCH 3/8] Add the application to the wapp_list
On Sun, 14 Apr 2013 at 23:13:28 +0200, Rodolfo García Peñas (kix) wrote:
> From: "Rodolfo García Peñas (kix)"
>
> The screen has a list of applications:
>
> 8<
> kix@debian:~/src/wmaker/wmaker-crm/src$ grep wapp_list screen.h
> struct WApplication *wapp_list;/* list of all aplications */
> kix@debian:~/src/wmaker/wmaker-crm/src$
> 8<
>
> But this variable was not set before this patch:
> - Was removed in application.c
> - Local variable in session.c
>
> 8<
> kix@debian:~/src/wmaker/wmaker-crm/src$ grep wapp_list *.c
> application.c: if (wapp == scr->wapp_list) {
> application.c: scr->wapp_list = wapp->next;
> session.c: WMArray *wapp_list = NULL;
> session.c: wapp_list = WMCreateArray(16);
> session.c:&& (WMGetFirstInArray(wapp_list, (void *)appId) ==
> WANotFound
> session.c: WMAddToArray(wapp_list, (void *)appId);
> session.c: WMFreeArray(wapp_list);
> kix@debian:~/src/wmaker/wmaker-crm/src$
> 8<
>
> This variable is needed to restore the screen without re-create the
> applications structs.
> ---
> src/application.c |4
> 1 file changed, 4 insertions(+)
>
> diff --git a/src/application.c b/src/application.c
> index abe1d2d..6a03775 100644
> --- a/src/application.c
> +++ b/src/application.c
> @@ -143,6 +143,10 @@ WApplication *wApplicationCreate(WWindow * wwin)
>
> create_appicon_for_application(wapp, wwin);
>
> + /* Save the application in the application list */
> + wapp->next = scr->wapp_list;
> + scr->wapp_list = wapp;
If wapp_list was not set before then wapp->next will contain junk
at this point, no?
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [PATCH 4/8] Split dock menu creation
Quoting "Carlos R. Mafra" : On Wed, 17 Apr 2013 at 6:37:57 +, Rodolfo García Peñas (kix) wrote: Hi Carlos, something is wrong in this patch. Please, don't apply it yet. I was not planning to apply it. I don't think that all that code movement is worth the trouble. We will need some parts of this code to the XRandR too. The XRandR needs split the item creation and the item painting. Cheers, kix Rodolfo García Peñas (kix) http://www.kix.es/ -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Move the "Add drawer" option down
IMO, the option "Add drawer" should be moved down in the dock's menu. IMO, the right list sort should be: - Launch - Bring here - Hide - Settings - Dock position - Add drawer - Kill I use sometimes the "Launch" option and now I am adding drawers every time. kix Rodolfo García Peñas (kix) http://www.kix.es/ -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [PATCH 6/8] Create clip only if needed
On Wed, 17 Apr 2013 at 8:56:02 +, Rodolfo García Peñas (kix) wrote: > > >>When we set the with_clip flag is only for that function flow, but not > >>for all the wmaker session. > > > >I don't see why these things have to be different. If noclip is set in > >WPrefs the user already told wmaker he doesn't want a clip, so the > >functions should not create it. > > And if the user want the clip? He select the "noclip" to false, then > the clip is created when the workspace is restored, then removed, then > created again... I see now, thanks. -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [PATCH 6/8] Create clip only if needed
Quoting "Carlos R. Mafra" :
On Tue, 16 Apr 2013 at 23:25:10 +0200, "Rodolfo García Peñas (kix)" wrote:
On 16/04/2013 22:06, Carlos R. Mafra wrote:
>> +int wWorkspaceNew(WScreen *scr, Bool with_clip)
>> {
>>WWorkspace *wspace, **list;
>>int i;
>> @@ -102,7 +102,7 @@ int wWorkspaceNew(WScreen *scr)
>>sprintf(wspace->name, _("Workspace %i"),
scr->workspace_count);
>>}
>>
>> - if (!wPreferences.flags.noclip)
>> + if (!wPreferences.flags.noclip && with_clip)
>
>
> This seems redundant.
>
> Why can't you use the noclip flag directly instead of adding the new
> with_clip argument? Ie, if the noclip flag is set then that should be
> equivalent to your !with_clip case.
>
Hi Carlos,
these flags are different.
wPreferences.flag.noclip is used when the user don't want to use the clip.
Then, if the flag is set, the Clip is never used.
So if the user doesn't want the clip you should not create it in the first
place too. So instead of adding an extra parameter to control the creation,
simply use the existing flag to make the decisions. I don't see the need
to add the with_clip argument when you already have a noclip flag.
Brrr, I need improve my english.
This is the current flow:
1. The workspace creates the clip when is restored
2. The clip is removed
3. The Clip is created again if the flag "noclip" is not set.
With the patch, the flow is:
1. The workspace is created, but not the clip, when is restored -> The
with_clip flag is used for that
2. The clip is not removed, because is not created in the step 1
3. The Clip is created if the flag "noclip" is not set. -> We use the
noclip flag for that
Then, only the step 3 creates the Clip.
If the flag noclip is set, without the patch, then, the Clip is never
created in the step 3 (I am not sure if is created in the step 1, I
need check the code).
If we use the flag noclip with the funcionality of with_clip flag, the
flow should be:
1. Set the noclip flag to false
2. The workspace is created, but not the clip, when is restored
(again, I need check if this function test the noclip flag)
3. The clip is not removed, because is not created in the step 1
4. *Restore* the initial value of "noclip", because the user might see
the clip
5. The Clip is created if the flag "noclip" is not set.
When we set the with_clip flag is only for that function flow, but not
for all the wmaker session.
I don't see why these things have to be different. If noclip is set in
WPrefs the user already told wmaker he doesn't want a clip, so the
functions should not create it.
And if the user want the clip? He select the "noclip" to false, then
the clip is created when the workspace is restored, then removed, then
created again...
Rodolfo García Peñas (kix)
http://www.kix.es/
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [PATCH 4/8] Split dock menu creation
On Wed, 17 Apr 2013 at 6:37:57 +, Rodolfo García Peñas (kix) wrote: > Hi Carlos, > > something is wrong in this patch. Please, don't apply it yet. I was not planning to apply it. I don't think that all that code movement is worth the trouble. -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [PATCH 6/8] Create clip only if needed
On Tue, 16 Apr 2013 at 23:25:10 +0200, "Rodolfo García Peñas (kix)" wrote:
> On 16/04/2013 22:06, Carlos R. Mafra wrote:
>
> >> +int wWorkspaceNew(WScreen *scr, Bool with_clip)
> >> {
> >>WWorkspace *wspace, **list;
> >>int i;
> >> @@ -102,7 +102,7 @@ int wWorkspaceNew(WScreen *scr)
> >>sprintf(wspace->name, _("Workspace %i"),
> >> scr->workspace_count);
> >>}
> >>
> >> - if (!wPreferences.flags.noclip)
> >> + if (!wPreferences.flags.noclip && with_clip)
> >
> >
> > This seems redundant.
> >
> > Why can't you use the noclip flag directly instead of adding the new
> > with_clip argument? Ie, if the noclip flag is set then that should be
> > equivalent to your !with_clip case.
> >
>
> Hi Carlos,
>
> these flags are different.
>
> wPreferences.flag.noclip is used when the user don't want to use the clip.
> Then, if the flag is set, the Clip is never used.
So if the user doesn't want the clip you should not create it in the first
place too. So instead of adding an extra parameter to control the creation,
simply use the existing flag to make the decisions. I don't see the need
to add the with_clip argument when you already have a noclip flag.
> When we set the with_clip flag is only for that function flow, but not
> for all the wmaker session.
I don't see why these things have to be different. If noclip is set in
WPrefs the user already told wmaker he doesn't want a clip, so the
functions should not create it.
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
