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

From: Christoph Lohmann <20h_AT_r-36.net>
Date: Mon, 19 Oct 2015 21:38:53 +0200

Greetings.

On Mon, 19 Oct 2015 21:38:53 +0200 Quentin Rameau <quinq+hackers_AT_fifth.space> wrote:
> #define SETPROP(p, q) { \
> - .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 \
> - } \
> + .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.)

> 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?

> diff --git a/surf.c b/surf.c
> index a83c5ea..d0adcd6 100644
> --- a/surf.c
> +++ b/surf.c
> _AT_@ -113,8 +113,8 @@ static SoupCache *diskcache = NULL;
>
> static void addaccelgroup(Client *c);
> static void beforerequest(WebKitWebView *w, WebKitWebFrame *f,
> - 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.

> -static void
> -addaccelgroup(Client *c) {
> +void
> +addaccelgroup(Client *c)

You are mixing style changes with programmatic changes.

> +{
> int i;
> GtkAccelGroup *group = gtk_accel_group_new();
> GClosure *closure;
> _AT_@ -225,15 +226,15 @@ addaccelgroup(Client *c) {
> for (i = 0; i < LENGTH(keys); i++) {
> closure = g_cclosure_new(G_CALLBACK(keypress), c, NULL);
> gtk_accel_group_connect(group, keys[i].keyval, keys[i].mod,
> - 0, closure);
> + 0, closure);

This is so ugly and looks like GNU style. Why do you want to move the
parameters too far left?

> if (!g_str_has_prefix(uri, "http://") \
> - && !g_str_has_prefix(uri, "https://") \
> - && !g_str_has_prefix(uri, "about:") \
> - && !g_str_has_prefix(uri, "file://") \
> - && !g_str_has_prefix(uri, "data:") \
> - && !g_str_has_prefix(uri, "blob:") \
> - && strlen(uri) > 0) {
> -
> + && !g_str_has_prefix(uri, "https://") \
> + && !g_str_has_prefix(uri, "about:") \
> + && !g_str_has_prefix(uri, "file://") \
> + && !g_str_has_prefix(uri, "data:") \
> + && !g_str_has_prefix(uri, "blob:") \
> + && strlen(uri) > 0) {

No.

The other changes look ok.


Sincerely,

Christoph Lohmann
Received on Mon Oct 19 2015 - 21:38:53 CEST