Various code cleanups (prototypes, style, etc)

[ThomasAdam]

Hi,

I'm giving quite a bit of thought to creating several patches for goomwwm, but before I do that, I've got a couple of pet-peeves I wanted to try and address. :)

The code looks quite good; I'm glad it's been split up in to separate files. However, some of the indentation and bracing style is most peculiar. I appreciate there's no accounting for taste, but I was wondering if cleaning this up would be worth it?

For example, changing:

if (foo) dosomething;

To:

if (foo) dosomething; else dosomethingelse;

If you're going to have a single proto.h file for prototypes, I might I suggest:

• Order the prototypes alphabetically.
• Or order them based on the .c file where the function resides.
• Don't put a variable name, just declare the type in the function. For example:

int foo(char *, int, int);

As opposed to:

int foo(char *f, int a, int b);

It means if things ever change, only the respective prototypes need to change, and not a lot else.

I'm happy to do the work on this -- just wanted to check it was going to be rejected. :)

Kindly,

-- Thomas Adam

[seanpringle]

Hi Thomas

I have no problem with:

if (foo) bar();

I don't mind if patches use extra braces or line breaks, but patches just for clean-up won't be pulled unless they have other worth too. After all, such clean-up by one person might simply end up offending someone else's taste.

My only layout requirements are:

1. Tabs for indentation, spaces for alignment.
2. Function definitions all on one line, not including the first opening brace.

Regarding proto.h, it should never be touched manually, imho. Observe how it is generated in the Makefile. If that auto-generation is working, there is no point making extra work by removing variable names or reordering stuff. If proto.h breaks, it just means the Makefile is wrong or someone hasn't obeyed rule #2.

Otoh, I have no problems with someone tweaking the Makefile to do it differently so long as I, personally, never have to look at or maintain proto.h!

No offence taken or intended by all this :) Feel free to argue. It takes all sorts!

cheers

[ThomasAdam]

OK. Good luck with that then. I won't be doing anything further on this. I might suggest some kind of style guide since it's certainly a unique style to put it politely.

One thing though -- you'll need to look at how you're generating the protoypes in proto.h -- currently, simply taking a function definition verbatim, will sometimes give you:

char *foo();

When technically, in the case where foo() accepts no arguments, should be:

char *foo(void);

Kindly,

Thomas Adam

[seanpringle]

Ok, np :)

Out of genuine interest, is it the use of one-line if-statements you're referring to as unique? I take it you're probably not that keen on the ternary operator either?

Re. the prototype with no args: Thanks for the heads-up. It would certainly be good to make that technically correct.

Besides the two simple style points I listed for indentation and function defs, my general opinion is that, so long clang -Wall is happy and builds without any warnings, I'm happy (to my knowledge both clang and gcc warn when braces are ambiguous). I can understand how others might like more rigidity.

cheers

[ThomasAdam]

Oh, I have no problem with the ternary operator, I use it a lot. But when you see things like this:

if (foo) dosoemthing;

Or:

for (; foo < bar; foo++) foo++;

all on one line, it's very difficult to read, especially if the line is long.

[seanpringle]

Interesting how the brain works :) I find the opposite. Possibly I'd be less happy if someone took away my syntax highlighting.

Anyway, as I mentioned, more line breaks in patches is fine with me, including adjustments to surrounding code. Not using tabs-for-indenting is my only real pet peeve.