Re: [hackers] [PATCH] Continue style fixing: function declarations, code alignement

From: Quentin Rameau <quinq+hackers_AT_fifth.space>
Date: Mon, 19 Oct 2015 22:57:55 +0200

> > + .v = (char *[]){ "/bin/sh", "-c", \
> > + "prop=\"`xprop -id $2 $0 " \
> > + "| sed \"s/^$0(STRING) = \\(\\\\\"\\?\\)\\(.*\\)\\1$/\\2/\" " \
> > + "| xargs -0 printf %b | dmenu`\" &&" \
> > + "xprop -id $2 -f $1 8s -set $1 \"$prop\"", \
> > + p, q, winid, NULL \
> > +} \
>
> Why this change? (Including the other ones doing the same.)
Quoting the style page (which I agree with on that point at least),
“Use spaces not tabs for multiline macros as the indentation level is 0, where the #define began”

> > static Key keys[] = {
> > - /* modifier keyval function arg Focus */
> > + /* modifier keyval function arg Focus */
> > { MODKEY|GDK_SHIFT_MASK,GDK_r, reload, { .b = TRUE } },
> > { MODKEY, GDK_r, reload, { .b = FALSE } },
> > { MODKEY|GDK_SHIFT_MASK,GDK_p, print, { 0 } },
>
> Why don’t you add a tab in front of the array elements instead and move
> everything to tabs?
I didn't want to change the config file lines and make them too long so I didn't
touch those, but I will do that if you think it's better.
I wouldn't move everything to tabs though as this is alignement, not indentation.

> > - WebKitWebResource *r, WebKitNetworkRequest *req,
> > - WebKitNetworkResponse *resp, Client *c);
> > + WebKitWebResource *r, WebKitNetworkRequest *req,
> > + WebKitNetworkResponse *resp, Client *c);
> This looks ugly. The extra parameters should be more indented to the right.
I like the 8 spaces tab width, 4 real spaces for line continuing alignement.

> > -static void
> > -addaccelgroup(Client *c) {
> > +void
> > +addaccelgroup(Client *c)
> You are mixing style changes with programmatic changes.
Not really, the prototypes already states that the function is static, putting it
in the function definition is redundant so that's more a matter of style.

> > - 0, closure);
> > + 0, closure);
> This is so ugly and looks like GNU style. Why do you want to move the
> parameters too far left?
To be consistant with the rest of the indentation/alignement style. And I wanted
to keep the code as it is when possible.

> > - && strlen(uri) > 0) {
> > + && strlen(uri) > 0) {
> No.
Same answer.

> The other changes look ok.
ok!
Received on Mon Oct 19 2015 - 22:57:55 CEST