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

From: Markus Teich <markus.teich_AT_stusta.mhn.de>
Date: Mon, 19 Oct 2015 23:19:43 +0200

Heyho,

since I seem to have caused these style changes, I will also throw my 0.02$ at
you.

Quentin Rameau wrote:
> > > 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 also think they should be indented by one tab.

> I wouldn't move everything to tabs though as this is alignement, not indentation.

Here I agree, we should not use tabs other than for indentation at the beginning
of line. This is also noted in the style guidelines.

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

Per the guidelines these lines should be indented by tabs to the same level as
the first line of the function call and then use spaces for alignment. We have
no guideline on how to align remaining function parameters, I would align it to
the same level as the first parameter to the function event though it sometimes
is a little ugly to have parameters far to the right.

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

Funnily the guide says „Functions not used outside translation unit should be
declared and defined static“, so functions should also include the static
keyword at the definition. I understand this is meant for verbosity and
expressiveness (if done consistently, you basically don't have to lookup
declarations if you're reading the definition and asking yourself if the
function is static or not).

--Markus
Received on Mon Oct 19 2015 - 23:19:43 CEST