Re: [PATCH] : Inversion for options

On Wed, Dec 26, 2007 at 02:38:34PM +0100, Krzysztof Oledzki wrote:
> >That said, I've looked at your patch, and it looks good to me. I would just
> >have two very minor comments :
> > - in error messages reported to the user, it is said that option xxx does
> > not
> > support inversion. I'd prefer to say that "option xxx cannot be
> > negated".
>
> OK, but IMHO "support" is better as there are options that cannot be
> negated/disabled now but this may change in the future.

good point. I'll try to arrange the sentence with all this in mind.

> > - you'll find me very annoying on this one, but please do not forget to
> > add
> > a space after language keywords such as "for" :-) In fact, I find that
> > it
> > makes it really easier to read the code at certain moments in the night
> > when you know that isolated words are for the language and that words
> > immediately followed by a parenthesis are function names.
>
> OK. My coding style is little different but if it makes different for you
> I'll try to remember about this and even send a patch for one of my
> prevoius changes:

Hey, that's not something very important, so do not make too strong efforts for this! It's just general preference after having spent time reusing old code of mine which I found harder to read for such reasons.

> ./src/cfgparse.c: for(curproxy=proxy; curproxy;
> curproxy=curproxy->next) {
>
> BTW: there are more places requiring changing:
[...]

I know, that's what proves that I'm not a tyrant with coding style ;-)

Cheers,
Willy Received on 2007/12/26 15:05