Fred Kiefer wrote:
Hi Christopher,
there is a small but very annoying problem with your patch for gui: You
added some clean up of he formatting to it. Now it is great to replace
the tab characters that went into the files with spaces. After noticing
that my editor used tabs instead of spaces for indentation I switched
that off and started to clean up files myself, each time I do a commit
on GNUstep. But having to proofread a huge patch with mostly whitespace
changes is rather boring. It is too easy to miss out on the important
bits, while skipping over the formatting changes.
Also your indentation does not match with the formatting rules for
GNUstep. You use
if (XXX)
{
[AAA bb];
}
whereas GNUstep has
if (XXX)
{
[AAA bb];
}
(There are other differences as well, but this is the most prominent)
Could you please switch over to the GNUstep style before doing a huge
commit?
For the changes to the comments, I also don't see why the reformatting
was necessary.
Sorry, the reformatting is an accidental part of me writing code or
possibly my editor (I'm using VIM with tabs set to be expanded to
spaces: sw=2, cin, expandtab, ai, sta - which one do I turn off?). I'll
definitely fix it up for next time. I didn't intend this patch to be
committed to SVN or even reviewed as such; I attached it as an example
of what I've done so that people could try it out. I know it is a mess
and thats why I suggest you apply it against a temporary SVN checkout to
save messing up your working copy.
As much as I like some of the stuff you did with the Windows themes, in
the current form this patch is not ready for a review.
Have your thought about moving your NSColor code into a separate
application that will just change the system colour list and write it
out? Your code is great for most cases, but when somebody tries to
access the system colour list the result will be totally different.
Yep, this is currently a hack. I need a way to map the GetSysColor()
function to the NSColor system APIs, but the theming API only accepts
static NSColorLists. I might try and expand something out to permit
themes to specify a color list on the fly.
The theming extensions for menu and scrollers are surely something we need.
To make your live easier in the future I will submit a whitespace change
to NSButtonCell and NSMenuItemCell removing all the tab characters.
Does this mean you are going to commit part of this patch, or would you
like me to split it out first so you can get part of it in?
Regards
Chris
_______________________________________________
Discuss-gnustep mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/discuss-gnustep
