Universal escaping in KConfigINI: Summary and patch
ahartmetz at gmail.com
Fri May 25 10:28:23 BST 2007
On Friday 25 May 2007 09:02:26 Oswald Buddenhagen wrote:
> On Fri, May 25, 2007 at 01:46:00AM +0200, Andreas Hartmetz wrote:
> > You will find attached the patch that I consider *final*.
> On Fri, May 25, 2007 at 01:51:10AM +0200, Andreas Hartmetz wrote:
> > Of course "TODO: warning" is not final :)
> > I like the control flow in in stringToPrintable exactly the way it is,
> yes, my idea wouldn't work anyway (reading patches is hard, you know :).
I noticed when I played around with it.
> buuut ... having the default last is *still* the right thing to do. ;)
OK, braindump: IMO "default:" is usually last not because "default:" has to be
last, but because it usually handles the error case, which isn't as important
to the understanding of the code as the normal case. In this particular
case, "default:" handles the normal case so it's first. What-ever.
> > we cannot discuss minor stuff forever
> oh, c'mon ... are you in a hurry? :-D
There was this thing... I think it was called release plan :o
Yeah, who reads release plans anyway? xD
Still, I would like to start using KDE 4 before 2008. That would be cool.
> your patches are made with -b, right?
Uh, I had to look up what -b even means ("ignore whitespace changes"). My
patches are made with "svn diff".
> > + escape:
> > + *r++ = '\\';
> i unindent goto labels only half an indent step. maybe you find it more
> appealing, too. ;)
I don't like half-indentions. Goto labels just don't fit in well in C/C++ and
I see no good way to change that.
> > + *r++ = nibbleLookup[((unsigned char)s[i]) >> 4];
> > + *r = nibbleLookup[((unsigned char)s[i]) & 0x0f];
> you overdid the casting, but readability-wise it is better your way.
> it might get even better if you line up the equal sign and change to 15
> again. :)
Except for very mathematical code, I tend to think that lining up statements
makes them look silly. And for bit manipulations I *think* in bits, so I
write in bits/hex.
More information about the kde-core-devel