Re: [Preliminary Patch] setup.exe size/position restore on startup

[Jonathon Merz <jonathon dot merz at gmail dot com>]

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