Review Request: Use a qml based screen locker in place of the screensaver

Sat Aug 25 10:02:25 BST 2012

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106124/#review17977
-----------------------------------------------------------

it would be good if you could present a "process model", so i could sanity-check the architecture again without reverse engineering it from scratch. maybe even a strategically placed readme.

not many comments on the qml, as i have no proper clue about it.

kcontrol/screensaver/scrnsave.cpp
<http://git.reviewboard.kde.org/r/106124/#comment14201>

    it wasn't the original plan to make these mutually exclusive. but whatever.

ksmserver/screenlocker/greeter/greeter.h
<http://git.reviewboard.kde.org/r/106124/#comment14205>

    looks more like my code. (almost) unmodified.

ksmserver/screenlocker/greeter/greeterapp.cpp
<http://git.reviewboard.kde.org/r/106124/#comment14206>

    you could just write KUser().property(...)

ksmserver/screenlocker/greeter/greeterapp.cpp
<http://git.reviewboard.kde.org/r/106124/#comment14207>

    looks anything between pointless and harmful. care to explain?

ksmserver/screenlocker/greeter/greeterapp.cpp
<http://git.reviewboard.kde.org/r/106124/#comment14208>

    ditto

ksmserver/screenlocker/greeter/screensaverwindow.cpp
<http://git.reviewboard.kde.org/r/106124/#comment14209>

    i bet something is missing here

ksmserver/screenlocker/greeter/themes/org.kde.passworddialog/contents/ui/SessionSwitching.qml
<http://git.reviewboard.kde.org/r/106124/#comment14210>

    i think a space is missing before the opening paren

ksmserver/screenlocker/ksldapp.cpp
<http://git.reviewboard.kde.org/r/106124/#comment14204>

    this code clearly borrows from the old implementation, so the copyrights should be restored accordingly.

ksmserver/server.cpp
<http://git.reviewboard.kde.org/r/106124/#comment14202>

    #else
      Q_UNUSED(lockscreen)

ksmserver/shutdown.cpp
<http://git.reviewboard.kde.org/r/106124/#comment14203>

    unrelated style change. don't do that

plasma/screensaver/shell/qml/lockscreen.qml
<http://git.reviewboard.kde.org/r/106124/#comment14211>

    should use the new header with the url. check other files as well.

startkde.cmake
<http://git.reviewboard.kde.org/r/106124/#comment14200>

    don't leave dead code in

you could/should fix at least the most blatant coding style violations (like wholly wrong indentation) while you are re-importing the code.

- Oswald Buddenhagen

On Aug. 24, 2012, 11:30 a.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106124/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2012, 11:30 a.m.)
> 
> 
> Review request for KDE Runtime.
> 
> 
> Description
> -------
> 
> this is the finalization of the old "screenlocker" branch in workspace:
> the screen saver goes away (discussed at the time, about one year ago) and the screen locker gets managed by ksmserver, with a greeter that has the ui dine in qml.
> The same qml ui gets loaded by the plasma based greeter when the "allow widgets on screen locker" is enabled.
> the screensaver kcm is now called "Screen locker" and is way simpler, the screen saver chooser is gone from it.
> 
> 
> Diffs
> -----
> 
>   kcontrol/screensaver/CMakeLists.txt e4dcc3a 
>   kcontrol/screensaver/screensaver.ui 0ad5cd8 
>   kcontrol/screensaver/scrnsave.h 7c8deba 
>   kcontrol/screensaver/scrnsave.cpp c0507d4 
>   krunner/CMakeLists.txt 21eac6f 
>   krunner/dbus/org.freedesktop.ScreenSaver.xml 5efd943 
>   krunner/dbus/org.kde.screensaver.xml e700b88 
>   krunner/kcfg/kscreensaversettings.kcfg c8f76f3 
>   krunner/kcfg/kscreensaversettings.kcfgc af9133d 
>   krunner/krunnerapp.h 040198d 
>   krunner/krunnerapp.cpp eea6220 
>   krunner/lock/CMakeLists.txt cf9a67e 
>   krunner/lock/autologout.h 0c444050 
>   krunner/lock/autologout.cc c86e29a 
>   krunner/lock/config-krunner-lock.h.cmake 7bfdfd6 
>   krunner/lock/kscreenlocker.notifyrc 14e37ec 
>   krunner/lock/lockdlg.h f25e55f 
>   krunner/lock/lockdlg.cc 14a9b34 
>   krunner/lock/lockprocess.h 8b6d9a8 
>   krunner/lock/lockprocess.cc 65c7f1d 
>   krunner/lock/main.h 8a60353 
>   krunner/lock/main.cc 7b41024 
>   krunner/main.cpp 84a547b 
>   krunner/screensaver/saverengine.h 3384d4a 
>   krunner/screensaver/saverengine.cpp 4d90faa 
>   krunner/screensaver/xautolock.h 3db3233 
>   krunner/screensaver/xautolock.cpp 7124215 
>   krunner/screensaver/xautolock_c.h 3b82f5c 
>   krunner/screensaver/xautolock_diy.c b9df2f8 
>   krunner/screensaver/xautolock_engine.c d6d0cf5 
>   ksmserver/CMakeLists.txt 5f0fd34 
>   ksmserver/config-ksmserver.h.cmake 933da35 
>   ksmserver/main.cpp 430a61a 
>   ksmserver/screenlocker/CMakeLists.txt PRE-CREATION 
>   ksmserver/screenlocker/Messages.sh PRE-CREATION 
>   ksmserver/screenlocker/autologout.h PRE-CREATION 
>   ksmserver/screenlocker/autologout.cpp PRE-CREATION 
>   ksmserver/screenlocker/data/CMakeLists.txt PRE-CREATION 
>   ksmserver/screenlocker/data/force_krunner_lock_shortcut_unreg.cpp PRE-CREATION 
>   ksmserver/screenlocker/data/kscreenlocker_locksession-shortcut.upd PRE-CREATION 
>   ksmserver/screenlocker/dbus/org.freedesktop.ScreenSaver.xml PRE-CREATION 
>   ksmserver/screenlocker/dbus/org.kde.screensaver.xml PRE-CREATION 
>   ksmserver/screenlocker/greeter/CMakeLists.txt PRE-CREATION 
>   ksmserver/screenlocker/greeter/Messages.sh PRE-CREATION 
>   ksmserver/screenlocker/greeter/greeter.h PRE-CREATION 
>   ksmserver/screenlocker/greeter/greeter.cpp PRE-CREATION 
>   ksmserver/screenlocker/greeter/greeterapp.h PRE-CREATION 
>   ksmserver/screenlocker/greeter/greeterapp.cpp PRE-CREATION 
>   ksmserver/screenlocker/greeter/main.cpp PRE-CREATION 
>   ksmserver/screenlocker/greeter/screensaverwindow.h PRE-CREATION 
>   ksmserver/screenlocker/greeter/screensaverwindow.cpp PRE-CREATION 
>   ksmserver/screenlocker/greeter/sessions.h PRE-CREATION 
>   ksmserver/screenlocker/greeter/sessions.cpp PRE-CREATION 
>   ksmserver/screenlocker/greeter/themes/org.kde.passworddialog/contents/ui/Greeter.qml PRE-CREATION 
>   ksmserver/screenlocker/greeter/themes/org.kde.passworddialog/contents/ui/SessionSwitching.qml PRE-CREATION 
>   ksmserver/screenlocker/greeter/themes/org.kde.passworddialog/contents/ui/main.qml PRE-CREATION 
>   ksmserver/screenlocker/greeter/themes/org.kde.passworddialog/metadata.desktop PRE-CREATION 
>   ksmserver/screenlocker/interface.h PRE-CREATION 
>   ksmserver/screenlocker/interface.cpp PRE-CREATION 
>   ksmserver/screenlocker/kcfg/kscreensaversettings.kcfg PRE-CREATION 
>   ksmserver/screenlocker/kcfg/kscreensaversettings.kcfgc PRE-CREATION 
>   ksmserver/screenlocker/kscreenlocker.notifyrc PRE-CREATION 
>   ksmserver/screenlocker/ksldapp.h PRE-CREATION 
>   ksmserver/screenlocker/ksldapp.cpp PRE-CREATION 
>   ksmserver/screenlocker/lockwindow.h PRE-CREATION 
>   ksmserver/screenlocker/lockwindow.cpp PRE-CREATION 
>   ksmserver/server.h 8e54221 
>   ksmserver/server.cpp 7073fc9 
>   ksmserver/shutdown.cpp 6f298ec 
>   plasma/desktop/applets/kickoff/CMakeLists.txt 4b0d32a 
>   plasma/generic/containmentactions/contextmenu/CMakeLists.txt 5433294 
>   plasma/generic/runners/sessions/CMakeLists.txt 1b8292c 
>   plasma/screensaver/shell/BackgroundDialog.ui d6d2df7 
>   plasma/screensaver/shell/CMakeLists.txt 8ea4504 
>   plasma/screensaver/shell/backgrounddialog.cpp 1357a50 
>   plasma/screensaver/shell/main.cpp a1ae939 
>   plasma/screensaver/shell/plasmaapp.cpp ade6c68 
>   plasma/screensaver/shell/qml/lockscreen.qml PRE-CREATION 
>   plasma/screensaver/shell/savercorona.h 794e50c 
>   plasma/screensaver/shell/savercorona.cpp 6f83859 
>   plasma/screensaver/shell/saverview.cpp b6a709e 
>   powerdevil/daemon/CMakeLists.txt 35a4fd4 
>   startkde.cmake 36f23f1 
> 
> Diff: http://git.reviewboard.kde.org/r/106124/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20120825/d57a9736/attachment.htm>