Re: [dwm] more consistent codestyle patch
On Tue, May 13, 2008 at 06:08:59PM +0200, Szabolcs Nagy wrote: On 5/13/08, Diego Biurrun [EMAIL PROTECTED] wrote: XGrabButton(dpy, AnyButton, AnyModifier, c-win, False, - BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); + BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); Well, before it was indented to align with the XGrabButton call. This used spaces to achieve this which is wrong A lot of people seem to argue over tab vs. spaces but somehow don't realize that the best (at least in my opinion) is to combine them in a smart way. That is, use tabs for indention and spaces for further alignment: ---XGrabButton(dpy, AnyButton, AnyModifier, c-win, False, ---BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); This way the user can set the tabwidth to whatever he likes and the code will still look decent. Regards, Marc -- Marc Andre Tanner http://www.brain-dump.org/ GPG key: CF7D56C0
Re: [dwm] more consistent codestyle patch
On Wed, May 14, 2008 at 03:54:10PM +0200, Marc Andre Tanner wrote: On Tue, May 13, 2008 at 06:08:59PM +0200, Szabolcs Nagy wrote: On 5/13/08, Diego Biurrun [EMAIL PROTECTED] wrote: XGrabButton(dpy, AnyButton, AnyModifier, c-win, False, - BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); + BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); Well, before it was indented to align with the XGrabButton call. This used spaces to achieve this which is wrong A lot of people seem to argue over tab vs. spaces but somehow don't realize that the best (at least in my opinion) is to combine them in a smart way. That is, use tabs for indention and spaces for further alignment: ---XGrabButton(dpy, AnyButton, AnyModifier, c-win, False, ---BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); This way the user can set the tabwidth to whatever he likes and the code will still look decent. Actually that's the way I intend to have the source code -- imho I used this already quite a lot. I didn't bother that much so far, because I'm going to polish the code anyways for the upcoming dwm-5.0 release. Kind regards, -- Anselm R. Garbe http://www.suckless.org/ GPG key: 0D73F361
Re: [dwm] more consistent codestyle patch
Marc Andre Tanner wrote: [...] That is, use tabs for indention and spaces for further alignment: ---XGrabButton(dpy, AnyButton, AnyModifier, c-win, False, ---BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); This way the user can set the tabwidth to whatever he likes and the code will still look decent. *drool* Is there a tool that does this? Kai
Re: [dwm] more consistent codestyle patch
On (14/05/08 16:24), Kai Grossjohann wrote: To: dynamic window manager dwm@suckless.org From: Kai Grossjohann [EMAIL PROTECTED] Subject: Re: [dwm] more consistent codestyle patch User-Agent: Thunderbird 2.0.0.14 (X11/20080505) Reply-To: dynamic window manager dwm@suckless.org List-Id: dynamic window manager dwm.suckless.org Marc Andre Tanner wrote: [...] That is, use tabs for indention and spaces for further alignment: ---XGrabButton(dpy, AnyButton, AnyModifier, c-win, False, ---BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); This way the user can set the tabwidth to whatever he likes and the code will still look decent. *drool* Is there a tool that does this? Kai indent(1) and there are other programs that this do. -- Premysl Anydot Hruby, http://www.redrum.cz/
Re: [dwm] more consistent codestyle patch
Premysl Hruby [EMAIL PROTECTED] wrote: On (14/05/08 16:24), Kai Grossjohann wrote: To: dynamic window manager dwm@suckless.org From: Kai Grossjohann [EMAIL PROTECTED] Subject: Re: [dwm] more consistent codestyle patch User-Agent: Thunderbird 2.0.0.14 (X11/20080505) Reply-To: dynamic window manager dwm@suckless.org List-Id: dynamic window manager dwm.suckless.org Marc Andre Tanner wrote: [...] That is, use tabs for indention and spaces for further alignment: ---XGrabButton(dpy, AnyButton, AnyModifier, c-win, False, ---BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); This way the user can set the tabwidth to whatever he likes and the code will still look decent. *drool* Is there a tool that does this? Kai indent(1) and there are other programs that this do. I can confirm (I tried it) that indent doesn't supports some options which would be necessary to imitate dwm style and therefore enable automatic style testing. So indent might be a useful tool if you use GNU style or KNF, but otherwise it's simply useless. Regards Matthias-Christian
Re: [dwm] more consistent codestyle patch
On Tue, May 13, 2008 at 06:08:59PM +0200, Szabolcs Nagy wrote: On 5/13/08, Diego Biurrun [EMAIL PROTECTED] wrote: XGrabButton(dpy, AnyButton, AnyModifier, c-win, False, - BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); + BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); Well, before it was indented to align with the XGrabButton call. This used spaces to achieve this which is wrong You have an interesting concept of wrong. It was certainly intentional. was consistently done in at least a few places and makes the code more readable. no (it was consistently tab only indenting in most places) It was consistently aligned with the help of tabs and spaces. You have undone that in your patch. Diego
Re: [dwm] more consistent codestyle patch
On 5/14/08, Diego Biurrun [EMAIL PROTECTED] wrote: used spaces to achieve this which is wrong You have an interesting concept of wrong. It was certainly intentional. no (it was consistently tab only indenting in most places) It was consistently aligned with the help of tabs and spaces. You have undone that in your patch. no and yes ok it was a misunderstanding i asked arg if it's right to make style cleanup and also asked him if i should **use spaces everywhere in case of indented wraps because imho that's the right way to do it (in most places the code uses tabs for this)** he said that he prefered tabs everywhere so i cleaned up the code with this in mind (== spaces are wrong there) now i see he misunderstood my intentions, but at least now everyone agrees on the style ;)
[dwm] more consistent codestyle patch
simple modifications (whitespce, line wrapping) diff -r f939086fa41d dwm.c --- a/dwm.c Tue May 13 11:27:20 2008 +0100 +++ b/dwm.c Tue May 13 14:03:20 2008 +0200 @@ -595,7 +595,7 @@ drawtext(const char *text, unsigned long x = dc.x + (h / 2); /* shorten text if necessary */ for(; len (w = textnw(buf, len)) dc.w - h; len--); - if (!len) + if(!len) return; if(len olen) { if(len 1) @@ -782,21 +782,21 @@ grabbuttons(Client *c, Bool focused) { int i, j; unsigned int buttons[] = { Button1, Button2, Button3 }; unsigned int modifiers[] = { MODKEY, MODKEY|LockMask, MODKEY|numlockmask, - MODKEY|numlockmask|LockMask} ; +MODKEY|numlockmask|LockMask} ; XUngrabButton(dpy, AnyButton, AnyModifier, c-win); if(focused) for(i = 0; i LENGTH(buttons); i++) for(j = 0; j LENGTH(modifiers); j++) XGrabButton(dpy, buttons[i], modifiers[j], c-win, False, -BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); + BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); else XGrabButton(dpy, AnyButton, AnyModifier, c-win, False, - BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); + BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); } void -grabkeys(void) { +grabkeys(void) { unsigned int i, j; KeyCode code; XModifierKeymap *modmap; @@ -1041,7 +1041,7 @@ monocle(void) { Client *c; for(c = clients; c; c = c-next) - if((lt-isfloating || !c-isfloating) isvisible(c, NULL)) + if((lt-isfloating || !c-isfloating) isvisible(c, NULL)) resize(c, wx, wy, ww - 2 * c-bw, wh - 2 * c-bw, RESIZEHINTS); } @@ -1055,7 +1055,7 @@ movemouse(Client *c) { ocx = nx = c-x; ocy = ny = c-y; if(XGrabPointer(dpy, root, False, MOUSEMASK, GrabModeAsync, GrabModeAsync, - None, cursor[CurMove], CurrentTime) != GrabSuccess) + None, cursor[CurMove], CurrentTime) != GrabSuccess) return; XQueryPointer(dpy, root, dummy, dummy, x1, y1, di, di, dui); for(;;) { @@ -1221,7 +1221,7 @@ resizemouse(Client *c) { ocx = c-x; ocy = c-y; if(XGrabPointer(dpy, root, False, MOUSEMASK, GrabModeAsync, GrabModeAsync, - None, cursor[CurResize], CurrentTime) != GrabSuccess) + None, cursor[CurResize], CurrentTime) != GrabSuccess) return; XWarpPointer(dpy, None, c-win, 0, 0, 0, 0, c-w + c-bw - 1, c-h + c-bw - 1); for(;;) { @@ -1351,7 +1351,7 @@ scan(void) { if(XQueryTree(dpy, root, d1, d2, wins, num)) { for(i = 0; i num; i++) { if(!XGetWindowAttributes(dpy, wins[i], wa) - || wa.override_redirect || XGetTransientForHint(dpy, wins[i], d1)) + || wa.override_redirect || XGetTransientForHint(dpy, wins[i], d1)) continue; if(wa.map_state == IsViewable || getstate(wins[i]) == IconicState) manage(wins[i], wa); @@ -1360,7 +1360,7 @@ scan(void) { if(!XGetWindowAttributes(dpy, wins[i], wa)) continue; if(XGetTransientForHint(dpy, wins[i], d1) - (wa.map_state == IsViewable || getstate(wins[i]) == IconicState)) + (wa.map_state == IsViewable || getstate(wins[i]) == IconicState)) manage(wins[i], wa); } } @@ -1479,8 +1479,8 @@ setup(void) { wa.event_mask = ButtonPressMask|ExposureMask; barwin = XCreateWindow(dpy, root, bx, by, bw, bh, 0, DefaultDepth(dpy, screen), -CopyFromParent, DefaultVisual(dpy, screen), -CWOverrideRedirect|CWBackPixmap|CWEventMask, wa); + CopyFromParent, DefaultVisual(dpy, screen), + CWOverrideRedirect|CWBackPixmap|CWEventMask, wa); XDefineCursor(dpy, barwin, cursor[CurNormal]); XMapRaised(dpy, barwin); strcpy(stext, dwm-VERSION); @@ -1704,7 +1704,6 @@ unmapnotify(XEvent *e) { void updatebar(void) { - if(dc.drawable != 0) XFreePixmap(dpy, dc.drawable); dc.drawable = XCreatePixmap(dpy, root, bw, bh, DefaultDepth(dpy, screen)); @@ -1839,7 +1838,7 @@ xerror(Display *dpy, XErrorEvent *ee) { || (ee-request_code == X_CopyArea ee-error_code == BadDrawable)) return 0; fprintf(stderr, dwm: fatal error: request code=%d, error code=%d\n, - ee-request_code, ee-error_code); + ee-request_code, ee-error_code); return xerrorxlib(dpy, ee); /* may call exit */ }
Re: [dwm] more consistent codestyle patch
On Tue, May 13, 2008 at 02:05:27PM +0200, Szabolcs Nagy wrote: simple modifications (whitespce, line wrapping) U... --- a/dwm.c Tue May 13 11:27:20 2008 +0100 +++ b/dwm.c Tue May 13 14:03:20 2008 +0200 @@ -782,21 +782,21 @@ grabbuttons(Client *c, Bool focused) { unsigned int modifiers[] = { MODKEY, MODKEY|LockMask, MODKEY|numlockmask, - MODKEY|numlockmask|LockMask} ; + MODKEY|numlockmask|LockMask} ; - BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); + BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); XGrabButton(dpy, AnyButton, AnyModifier, c-win, False, - BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); + BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); } @@ -1055,7 +1055,7 @@ movemouse(Client *c) { if(XGrabPointer(dpy, root, False, MOUSEMASK, GrabModeAsync, GrabModeAsync, - None, cursor[CurMove], CurrentTime) != GrabSuccess) + None, cursor[CurMove], CurrentTime) != GrabSuccess) return; @@ -1221,7 +1221,7 @@ resizemouse(Client *c) { if(XGrabPointer(dpy, root, False, MOUSEMASK, GrabModeAsync, GrabModeAsync, - None, cursor[CurResize], CurrentTime) != GrabSuccess) + None, cursor[CurResize], CurrentTime) != GrabSuccess) return; @@ -1479,8 +1479,8 @@ setup(void) { barwin = XCreateWindow(dpy, root, bx, by, bw, bh, 0, DefaultDepth(dpy, screen), - CopyFromParent, DefaultVisual(dpy, screen), - CWOverrideRedirect|CWBackPixmap|CWEventMask, wa); + CopyFromParent, DefaultVisual(dpy, screen), + CWOverrideRedirect|CWBackPixmap|CWEventMask, wa); XDefineCursor(dpy, barwin, cursor[CurNormal]); @@ -1839,7 +1838,7 @@ xerror(Display *dpy, XErrorEvent *ee) { fprintf(stderr, dwm: fatal error: request code=%d, error code=%d\n, - ee-request_code, ee-error_code); + ee-request_code, ee-error_code); return xerrorxlib(dpy, ee); /* may call exit */ All of this looks like code uglification to me... Diego
Re: [dwm] more consistent codestyle patch
On 5/13/08, Diego Biurrun [EMAIL PROTECTED] wrote: On Tue, May 13, 2008 at 02:05:27PM +0200, Szabolcs Nagy wrote: simple modifications (whitespce, line wrapping) U... All of this looks like code uglification to me... well imho consistent code style is better than inconsistent especially if the inconsistency does not serve any purpose i don't want you to worry about it too much so here is an explanation of the patch: 1) removed accidental double spaces eg.: -grabkeys(void) { +grabkeys(void) { 2) consistent whitespace in selection/iteration statements eg: - if (!len) + if(!len) 3) use tabs for identation (even for line wraps because arg prefers this way): XGrabButton(dpy, AnyButton, AnyModifier, c-win, False, - BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); + BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); 4) no extra indentation if the conditional expression of a statement wraps: if(XGrabPointer(dpy, root, False, MOUSEMASK, GrabModeAsync, GrabModeAsync, - None, cursor[CurResize], CurrentTime) != GrabSuccess) + None, cursor[CurResize], CurrentTime) != GrabSuccess) most of the code already followed these rules and i did not see any reason not to follow them in the specific cases. of course i am open to any comment about these style rules
Re: [dwm] more consistent codestyle patch
On Tue, May 13, 2008 at 05:41:30PM +0200, Szabolcs Nagy wrote: On 5/13/08, Diego Biurrun [EMAIL PROTECTED] wrote: On Tue, May 13, 2008 at 02:05:27PM +0200, Szabolcs Nagy wrote: simple modifications (whitespce, line wrapping) U... All of this looks like code uglification to me... well imho consistent code style is better than inconsistent especially if the inconsistency does not serve any purpose i don't want you to worry about it too much so here is an explanation of the patch: 3) use tabs for identation (even for line wraps because arg prefers this way): XGrabButton(dpy, AnyButton, AnyModifier, c-win, False, - BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); + BUTTONMASK, GrabModeAsync, GrabModeSync, None, None); Well, before it was indented to align with the XGrabButton call. This was consistently done in at least a few places and makes the code more readable. 4) no extra indentation if the conditional expression of a statement wraps: if(XGrabPointer(dpy, root, False, MOUSEMASK, GrabModeAsync, GrabModeAsync, - None, cursor[CurResize], CurrentTime) != GrabSuccess) + None, cursor[CurResize], CurrentTime) != GrabSuccess) Same here, it was aligned to make it clear what expression the next line belongs to. You kill this alignment and make this line less readable. Putting this at the same indentation depth as the if indicates that it is a new block after the if. Contrary to what your indentation indicates, this is not the case. most of the code already followed these rules and i did not see any reason not to follow them in the specific cases. I think you have misunderstood the rules the code was following. Diego
Re: [dwm] more consistent codestyle patch
one more little patch please review diff -r 33ba827ee84e dwm.c --- a/dwm.c Tue May 13 14:33:02 2008 +0100 +++ b/dwm.c Tue May 13 19:01:59 2008 +0200 @@ -504,7 +504,7 @@ detachstack(Client *c) { Client **tc; - for(tc=stack; *tc *tc != c; tc=(*tc)-snext); + for(tc = stack; *tc *tc != c; tc = (*tc)-snext); *tc = c-snext; } @@ -1162,8 +1162,7 @@ /* adjust for aspect limits */ if(c-minax != c-maxax c-minay != c-maxay - c-minax 0 c-maxax 0 c-minay 0 c-maxay 0) - { + c-minax 0 c-maxax 0 c-minay 0 c-maxay 0) { if(w * c-maxay h * c-maxax) w = h * c-maxax / c-maxay; else if(w * c-minay h * c-minax) @@ -1532,7 +1531,7 @@ if(!sel) return; for(i = 0; i LENGTH(tags); i++) - sel-tags[i] = (NULL == arg); + sel-tags[i] = (arg == NULL); sel-tags[idxoftag(arg)] = True; arrange(); }
Re: [dwm] more consistent codestyle patch
Szabolcs Nagy [EMAIL PROTECTED] wrote: one more little patch please review Looks good. Regards Matthias-Christian