Re: [PATCH 15/20] Add drawers to wmaker!

2013-04-12 Thread Rodolfo García Peñas (kix)
On 12/04/2013 1:42, yo@free.fr wrote:
 From: Daniel Déchelotte yo@free.fr

Hi Daniel,

first, your patches are impressive. Congrats. I have some comments about
them:

[snip]


   switch (which) {
   case WM_DOCK:
   wPreferences.flags.nodock = wPreferences.flags.nodock || *flag;
 + // Drawers require the dock
 + wPreferences.flags.nodrawer = wPreferences.flags.nodrawer || 
 wPreferences.flags.nodock;
   break;

First, you are using // for comments, C++ comments... But the main
question is why drawers require the dock?

Reading the main.c file I saw that if you use the dock and clip as one
thing:

   } else if (strcmp(argv[i], -nodock) == 0 || 
 strcmp(argv[i],
--no-dock) == 0) {
   wPreferences.flags.nodock = 1;
 + wPreferences.flags.nodrawer = 1;

Why? If the user set the nodock flag, then cannot use drawers?

IMO that is wrong.

 diff --git a/src/main.c b/src/main.c
 index f63b1a6..4559f86 100644
 --- a/src/main.c
 +++ b/src/main.c
 @@ -686,8 +686,11 @@ static int real_main(int argc, char **argv)
   wPreferences.flags.norestore = 1;
   } else if (strcmp(argv[i], -nodock) == 0 || 
 strcmp(argv[i], --no-dock) == 0) {
   wPreferences.flags.nodock = 1;
 + wPreferences.flags.nodrawer = 1;
   } else if (strcmp(argv[i], -noclip) == 0 || 
 strcmp(argv[i], --no-clip) == 0) {
   wPreferences.flags.noclip = 1;
 + } else if (strcmp(argv[i], -nodrawer) == 0 || 
 strcmp(argv[i], --no-drawer) == 0) {
 + wPreferences.flags.nodrawer = 1;
   } else if (strcmp(argv[i], -version) == 0 || 
 strcmp(argv[i], --version) == 0) {
   printf(Window Maker %s\n, VERSION);
   exit(0);
 diff --git a/src/screen.c b/src/screen.c
 index bc8f3fa..8d1c46c 100644

[snip]

 +/* Drawers, which are docks, really */
 +typedef struct WDrawerChain {
 +struct WDock *adrawer;
 +struct WDrawerChain *next;
 +} WDrawerChain;

Really, IMO the right way is use one internal struct WDock (or any
name), with the full behaviour (clip, dock and drawers), then, select
the type.

About the if brackets... I did a lot of clean patches to use the KR
style (I wrote about that in my previous comments to your patches).

IMO your patches are very very big. Some changes are not related to add
drawers. Probably the patches should be sent with more time between
them. There is no way to understand all (with standard human time)

Only this patch:
 19 files changed, 1560 insertions(+), 220 deletions(-)

Your patches might use the last patch in #next.

I will need more time for xrandr patches.

kix
-- 
||// //\\// Rodolfo kix Garcia
||\\// //\\ http://www.kix.es/


-- 
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.


Re: [PATCH 15/20] Add drawers to wmaker!

2013-04-12 Thread Rodolfo García Peñas (kix)
On 12/04/2013 1:42, yo@free.fr wrote:
 From: Daniel Déchelotte yo@free.fr
 
 Just discovered this bug: the auto-attract icon functionality will not
 work (to be precise, it crashes WM!) if the clip is disabled
 (NoClip=YES). Will fix shortly, of course.

You should release an specific patch for this bug.




-- 
||// //\\// Rodolfo kix Garcia
||\\// //\\ http://www.kix.es/


-- 
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.