It seems unclear (unlikely?) that this feature is definitely wanted, but on the chance that it is, responses to comments below:
On Wed, May 13, 2009 at 10:23 PM, Christopher Faylor wrote: > Thanks for the patch but since you asked, I don't understand why you > chose to add so many new files and so many new classes. It doesn't seem > like what you are doing calls for new classes. My rational for adding the PropSheetGeometry class is that there are a number of values that need to be available both to PropSheet and to WinGeometrySetting, so I packaged them together. They could all be made statics or maybe a static struct within PropSheet if that is better. With respect to the WinGeometrySetting class, all of the other saved settings are implemented as separate classes (by my reading, I think they have to be...) using libgetopt++. There is a mild non-standardization as to whether a class inheriting from UserSetting goes in it's own file or not: ConnectionSetting, KeysSetting, and SourceSetting are all in their own h/cc files, but LocalDirSetting and SiteSetting are combined in files with the classes that use them. The README says "Place all methods for a single class in a single file," and "Name the source file for classes and their headers after the class." I understood that to mean that new classes should have their own files, but I realize it doesn't explicitly say that, so I can put them elsewhere if that is preferred. > Also you use varying white space styles. You should stick with whatever > is in the surrounding code. It is a shame that setup has so many > different styles. Someone (maybe I) was asleep at the switch when the > varying styles made it into the code base but there is no need to > fragment things further. I just took another look at the GNU coding standards and think I see better what is required now. The mix of spaces and tabs in propsheet.cc confused me a little, but after figuring out the correct tab widths, I can see that most of it is consistent (there are a few lines that look off). To be sure, am I correct that the desired indenting is 4 spaces, with the open brace of a function lined up with the first column of the function declaration, and the opening braces of of other blocks of code indented 2 spaces from the first line of the block? I'll also change the added files to match if they stick around. I also notice I put some open-braces on the same line as their corresponding if's - I'll fix that in my next attempt if there is interest in the patch. > I know that Dave asked for this but I really don't see the need to add > this much machinery. I think this is a lot of work to go to for > something that is run infrequently and which is usually just clicked > through. For other installers, people just seem to live with whatever > they do rather than kvetching the geometry of the windows. I don't think that this patch interferes with anyone running things from the command line, though I admit that whether it is useful is extremely subjective - some people won't need or ever notice it while just clicking through, but some may find it useful. I decided to give it a shot since I fall into the "maximized package chooser is too big on my screen, but the default size is too small" camp, and a customizable size seemed to me like it might be an acceptable compromise. In terms of reducing the changes required, if the new files for the PropSheetGeometry and even the WinGeometrySetting class are eliminated, that would also remove the changes to Makefile.am, and leave only changes in propsheet.cc and new additions to propsheet.h. That doesn't really reduce the quantity of code though, just the number of files touched. Jonathon Merz