Re: Themeing

[Christopher Armstrong]

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