[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Sun, 29 Apr 2007 23:55:31 +0200
Thunderbird 188.8.131.52 (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
>> 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
>> 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.
- Themeing, Christopher Armstrong, 2007/04/29