[Libreoffice-commits] core.git: include/vcl

Chris Sherlock chris.sherlock79 at gmail.com
Thu Jan 23 02:14:47 PST 2014


 include/vcl/svapp.hxx |   50 +++-----------------------------------------------
 1 file changed, 3 insertions(+), 47 deletions(-)

New commits:
commit 826502c7496d1fc4aef46f65f6d2dc8966245ad3
Author: Chris Sherlock <chris.sherlock79 at gmail.com>
Date:   Thu Jan 23 21:04:31 2014 +1100

    Remove unnecessary verbiage.
    
    Change-Id: Icf0230577a5458425f22579e8d17f69c53310812
    Reviewed-on: https://gerrit.libreoffice.org/7608
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
    Tested-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/include/vcl/svapp.hxx b/include/vcl/svapp.hxx
index 05d982a..058f6ad 100644
--- a/include/vcl/svapp.hxx
+++ b/include/vcl/svapp.hxx
@@ -292,8 +292,7 @@ public:
 
     /** @} */ // end of changes
 
-    /** @defgroup Initialization
-        Initialization functions
+    /** @defgroup Initialization Initialization functions
         @{
     */
 
@@ -607,30 +606,16 @@ public:
         @{
     */
 
-    /** Overrides system settings with user settings.
+    /** Sets user settings in settings object to override system settings
 
      The system settings that can be overridden are:
         - window dragging options (on or off, including live scrolling!)
-        - style settings (e.g. cehckbox color, border color, 3D colors,
+        - style settings (e.g. checkbox color, border color, 3D colors,
           button rollover colors, etc.)
         - mouse settings
         - menu options, including the mouse follows the menu and whether menu
           icons are used
 
-     @remark One wonders why this is a virtual function when all the other
-        settings options are static functions. What is it about overridding
-        an AllSettings object that requires it to have it's own Application
-        instance, when LibreOffice only keeps one Application instance around
-        at any time?
-
-        Another thing about this function is that it is a remarkably awkward
-        name that doesn't \em really describe what it does.
-
-     @todo Recommend that rSettings be made immutable and
-        we return a new AllSettings because as it stands it would be easy to
-        believe that this function changes system settings, which it does not
-        as this is the job of @SetSettings.
-
      @param      rSettings      Reference to the settings object to change.
 
      @see MergeSystemSettings, SetSettings, GetSettings
@@ -640,21 +625,6 @@ public:
     /** Set the settings object to the platform/desktop environment system
      settings.
 
-     @todo Incredibly, this gets the default window, then it uses whatever
-        the system settings of this window are. This seems entirely unnecessary.
-        Furthermore, to do this trick it then calls on @ImplUpdateGlobalSettings
-        (a function of the @Window class, which in turn calls on a platform
-        specific SalFrame UpdateSettings to update the settings object to the
-        environment's defaults.
-
-        This would be better refactored to a function that gets the environment's
-        defaults without having to get a Window instance.
-
-     @todo Recommend that rSettings be made immutable and
-        we return a new AllSettings because as it stands it would be easy to
-        believe that this function changes system settings, which it does not
-        as this is the job of @SetSettings.
-
      @param     rSettings       Reference to the settings object to change.
 
      @see SystemSettingsChanging, SetSettings, GetSettings
@@ -664,11 +634,6 @@ public:
     /** Sets the application's settings and notifies all windows of the
      change.
 
-     @todo If the application hasn't initialized its settings yet, then
-        currently it calls on GetSettings to initialize the settings. Recommend
-        moving the initialization to a private function and call on this
-        instead.
-
      @param     rSettings       const reference to settings object used to
                                 change the application's settings.
 
@@ -680,15 +645,6 @@ public:
     /** Gets the application's settings. If the application hasn't initialized
      it's settings, then it does so (lazy initialization).
 
-     @remark This is a const function. However, it can update the
-        application's settings object in pSVData. Be warned!
-
-     @todo  We need to call on @GetSettings to initialize the application's
-        settings! This makes absolutely no sense. See @SetSettings, which does
-        exactly this and doesn't care about the return value. Recommend
-        moving this initialization code to a private function and call on
-        this seperately.
-
      @returns AllSettings instance that contains the current settings of the
         application.
 


More information about the Libreoffice-commits mailing list