[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Themeing

From: Christopher Armstrong
Subject: Re: Themeing
Date: Sun, 29 Apr 2007 21:43:26 +1000
User-agent: Thunderbird (Windows/20070221)

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
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?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]