D11522: [RFC] KCM Launch Feedback not saving cursor settings

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:8af970fd49f3: KCM Launch Feedback not saving cursor 
settings (authored by sharvey, committed by hein).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11522?vs=30042=30082

REVISION DETAIL
  https://phabricator.kde.org/D11522

AFFECTED FILES
  kcms/launch/package/contents/ui/main.qml

To: sharvey, #plasma, hein, davidedmundson, ngraham
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11522: [RFC] KCM Launch Feedback not saving cursor settings

[Scott Harvey]

sharvey added a comment.


  In D11522#230474 , @hein wrote:
  
  > Nah, it's to prevent an infinite loop between the different radio buttons 
and the backend object, since onCheckedChange will fire for both false and true.
  
  
  Makes sense, in a QML kind of way.
  
  > Thanks! Should I commit this for you? :)
  
  If you'd be so kind. Glad to help. :)

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kcm-launchfeedback-cursor (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11522

To: sharvey, #plasma, hein, davidedmundson, ngraham
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11522: [RFC] KCM Launch Feedback not saving cursor settings

[Eike Hein]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  Nah, it's to prevent an infinite loop between the different radio buttons and 
the backend object, since onCheckedChange will fire for both false and true.
  
  Thanks! Should I commit this for you? :)

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kcm-launchfeedback-cursor (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11522

To: sharvey, #plasma, hein, davidedmundson, ngraham
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11522: [RFC] KCM Launch Feedback not saving cursor settings

[Scott Harvey]

sharvey updated this revision to Diff 30042.
sharvey added a comment.


  - Revise code to use `Qt.callLater()` instead of invalid `Qt.later()`
  
  Per @hein, revised with correct function call

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11522?vs=30031=30042

BRANCH
  kcm-launchfeedback-cursor (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11522

AFFECTED FILES
  kcms/launch/package/contents/ui/main.qml

To: sharvey, #plasma, hein, davidedmundson, ngraham
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11522: [RFC] KCM Launch Feedback not saving cursor settings

[Scott Harvey]

sharvey added a comment.


  In D11522#230217 , @hein wrote:
  
  > The actuall bug here is pretty stupid - the function is called callLater(), 
not later(). Could you modify the patch to use callLater?
  
  
  Sure, easy enough. As long as you (or anyone, really) explains why we hold 
off on triggering this until the next event loop. Is it an optimization 
technique?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11522

To: sharvey, #plasma, hein, davidedmundson, ngraham
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11522: [RFC] KCM Launch Feedback not saving cursor settings

[Eike Hein]

hein added a comment.


  The actuall bug here is pretty stupid - the function is called callLater(), 
not later(). Could you modify the patch to use callLater?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11522

To: sharvey, #plasma, hein, davidedmundson, ngraham
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11522: [RFC] KCM Launch Feedback not saving cursor settings

[Scott Harvey]

sharvey marked an inline comment as done.
sharvey added a comment.


  Braces and semicolons removed. Taking these away was one of the ways I'd get 
a segfault. This time, however, all's well in the world. I need to keep 
debugging my environment.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11522

To: sharvey, #plasma, hein, davidedmundson, ngraham
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11522: [RFC] KCM Launch Feedback not saving cursor settings

[Scott Harvey]

sharvey updated this revision to Diff 30031.
sharvey edited the summary of this revision.
sharvey added a comment.


  - Removed erroneous braces and semicolons

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11522?vs=30028=30031

BRANCH
  kcm-launchfeedback-cursor (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11522

AFFECTED FILES
  kcms/launch/package/contents/ui/main.qml

To: sharvey, #plasma, hein, davidedmundson, ngraham
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11522: [RFC] KCM Launch Feedback not saving cursor settings

[Nathaniel Graham]

ngraham added a comment.


  Thanks for the patch! Formatting-wise, please capitalize Bug (should be BUG).

INLINE COMMENTS

> main.qml:63
>  
> -onCheckedChanged: Qt.later(function() { 
> kcm.busyCursorCurrentIndex = 0; })
> +onCheckedChanged: { kcm.busyCursorCurrentIndex = 0; }
> +

You can now remove the braces and semicolon here (and in the other instances).

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11522

To: sharvey, #plasma, hein, davidedmundson, ngraham
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11522: [RFC] KCM Launch Feedback not saving cursor settings

[Scott Harvey]

sharvey created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
sharvey requested review of this revision.

REVISION SUMMARY
  [Bug 392050] Launch Feedback KCM was not saving the cursor notifications
  properly - `Apply` button would not activate, `klaunchrc` was not being 
updated. 
  QML was using calls to `Qt.later() before setting the radio button index, 
generating 
  "unknown property" errors from QML and therefore not emitting the change 
signal. 
  Simply removed those calls and the QML functions properly, and `klaunchrc` is 
updated correctly.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kcm-launchfeedback-cursor (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11522

AFFECTED FILES
  kcms/launch/package/contents/ui/main.qml

To: sharvey
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart