[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Themeing
From: |
Fred Kiefer |
Subject: |
Re: Themeing |
Date: |
Sun, 29 Apr 2007 23:55:31 +0200 |
User-agent: |
Thunderbird 1.5.0.10 (X11/20060911) |
Christopher Armstrong wrote:
> 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?
>
No, at least not now, I will be away for the next ten days.
Fred
- Themeing, Christopher Armstrong, 2007/04/29