> On maj 19, 2015, 3:12 e.m., Vishesh Handa wrote: > > autotests/test_settings.cpp, line 43 > > <https://git.reviewboard.kde.org/r/123341/diff/4/?file=362129#file362129line43> > > > > Minor thing, which is up to you. > > > > In this test you're allocating the `Speller` off the heap and having to > > delete it. In the other test you're allocating it off the stack.
In the first test I allocate the Speller on the heap to be able delete it before the end of test case. I delete it to test that it does not save the settings automatically on destruction. I'll add comments :) - Kåre ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123341/#review80631 ----------------------------------------------------------- On maj 12, 2015, 6:03 e.m., Kåre Särs wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123341/ > ----------------------------------------------------------- > > (Updated maj 12, 2015, 6:03 e.m.) > > > Review request for KDE Frameworks, Laurent Montel and Martin Tobias Holmedahl > Sandsmark. > > > Repository: sonnet > > > Description > ------- > > The curent implementation can causes ~25 cals to Settings::save() for every > created Speller instance. > > The Settings::restore() function ends with setQuietIgnoreList(...) which > would call save() for evey ignore-word. > > This patch removes the save() cals in the setFoo() functions of Settings and > replace it by a boolean that indicates if the settign is changed or not. Then > in the Speller class save() is called when needed. > > The reason for this patch is that it took Kate ~2 minutes to open a kate > session with ~340 files. The implementation in Kate called reload() for every > file. There is a RR for Kate to remove that unneeded reload. > > This patch also prepares Sonnet::Settings for being used in the public API to > set aplication specific Sonnet settings without saving them in the global > settings. > > The Settings class is exported but the header is private. This change is BIC, > but since the class is private, my interpretation is that it should not mater. > > If accepted I will also add a Review Request to make the Settings class > public to enable application specific settings. > > > Diffs > ----- > > autotests/CMakeLists.txt 8ade413 > autotests/test_settings.h PRE-CREATION > autotests/test_settings.cpp PRE-CREATION > src/core/settings.cpp b5c4198 > src/core/settings_p.h 0d48889 > src/core/speller.cpp 3903b42 > src/ui/configwidget.cpp 02f2a26 > > Diff: https://git.reviewboard.kde.org/r/123341/diff/ > > > Testing > ------- > > Tested with Kate and with the patch the startup time was back to "normal". > Two unit tests added. > > > Thanks, > > Kåre Särs > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
