Re: Review Request 122554: knotifyconfig: Add optional dependency on QtSpeech to reenable speaking notifications.

2015-02-17 Thread Lukáš Tinkl

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122554/#review76186
---

Ship it!



src/knotifyconfigelement.h
https://git.reviewboard.kde.org/r/122554/#comment52538

whether


- Lukáš Tinkl


On Úno. 13, 2015, 8:42 odp., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122554/
 ---
 
 (Updated Úno. 13, 2015, 8:42 odp.)
 
 
 Review request for KDE Frameworks and Frederik Gladhorn.
 
 
 Repository: knotifyconfig
 
 
 Description
 ---
 
 Change config from KTTS to Speech.
 If QtSpeech is available when building return true from have_speech static 
 method.
 Otherwise return false.
 Could also add a check to see if there are any backend errors later on.
 
 
 Diffs
 -
 
   CMakeLists.txt f5aa0e78d89a4c14c3acfd9384b100e507e30067 
   src/knotifyconfigactionswidget.h 88b341a909d466b8eaf065c5220ac05f8c963697 
   src/knotifyconfigactionswidget.cpp 27012e3b803ef0a99d5bafcebea270e2d7419d62 
   src/knotifyconfigactionswidgetbase.ui 
 cbe647b70eb41ba540b7bf6d87f519b3246973a1 
   src/knotifyconfigelement.h 703952d23d1622e4b6214acc52461ea75b480254 
   src/knotifyconfigelement.cpp 5a1ac57fa5a4139682327d14c55a489450ea956b 
   src/knotifyeventlist.cpp 148bca7d33c722a4aeecabac45286a5e501c81b3 
 
 Diff: https://git.reviewboard.kde.org/r/122554/diff/
 
 
 Testing
 ---
 
 It builds, the config widget shows, the icon for speech is properly showing 
 when that configuration is enabled. Speech from konversation's new message 
 notifications are spoken.
 
 Something strange here:
 After enabling this notification, no messages are spoken until restarting the 
 application with the notifications (konversation in my test case).
 Similarly, after disabling this notification, messages are still spoken until 
 restarting the application.
 I checked and the konversation.notifyrc file is getting updated, but for some 
 reason the changes aren't taking effect imediately yet.
 
 
 Thanks,
 
 Jeremy Whiting
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 122554: knotifyconfig: Add optional dependency on QtSpeech to reenable speaking notifications.

2015-02-17 Thread Marco Martin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122554/#review76184
---

Ship it!


i'm not 100% sure on the issue below, apart from that seems fine


src/knotifyconfigelement.h
https://git.reviewboard.kde.org/r/122554/#comment52536

uuh, this seems not exported, so should be fine to rename i think


- Marco Martin


On Feb. 13, 2015, 7:42 p.m., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122554/
 ---
 
 (Updated Feb. 13, 2015, 7:42 p.m.)
 
 
 Review request for KDE Frameworks and Frederik Gladhorn.
 
 
 Repository: knotifyconfig
 
 
 Description
 ---
 
 Change config from KTTS to Speech.
 If QtSpeech is available when building return true from have_speech static 
 method.
 Otherwise return false.
 Could also add a check to see if there are any backend errors later on.
 
 
 Diffs
 -
 
   CMakeLists.txt f5aa0e78d89a4c14c3acfd9384b100e507e30067 
   src/knotifyconfigactionswidget.h 88b341a909d466b8eaf065c5220ac05f8c963697 
   src/knotifyconfigactionswidget.cpp 27012e3b803ef0a99d5bafcebea270e2d7419d62 
   src/knotifyconfigactionswidgetbase.ui 
 cbe647b70eb41ba540b7bf6d87f519b3246973a1 
   src/knotifyconfigelement.h 703952d23d1622e4b6214acc52461ea75b480254 
   src/knotifyconfigelement.cpp 5a1ac57fa5a4139682327d14c55a489450ea956b 
   src/knotifyeventlist.cpp 148bca7d33c722a4aeecabac45286a5e501c81b3 
 
 Diff: https://git.reviewboard.kde.org/r/122554/diff/
 
 
 Testing
 ---
 
 It builds, the config widget shows, the icon for speech is properly showing 
 when that configuration is enabled. Speech from konversation's new message 
 notifications are spoken.
 
 Something strange here:
 After enabling this notification, no messages are spoken until restarting the 
 application with the notifications (konversation in my test case).
 Similarly, after disabling this notification, messages are still spoken until 
 restarting the application.
 I checked and the konversation.notifyrc file is getting updated, but for some 
 reason the changes aren't taking effect imediately yet.
 
 
 Thanks,
 
 Jeremy Whiting
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 122554: knotifyconfig: Add optional dependency on QtSpeech to reenable speaking notifications.

2015-02-13 Thread Aleix Pol Gonzalez

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122554/#review75983
---


+1

What's the status of QtSpeech?

- Aleix Pol Gonzalez


On Feb. 13, 2015, 4:06 a.m., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122554/
 ---
 
 (Updated Feb. 13, 2015, 4:06 a.m.)
 
 
 Review request for KDE Frameworks and Frederik Gladhorn.
 
 
 Repository: knotifyconfig
 
 
 Description
 ---
 
 Change config from KTTS to Speech.
 If QtSpeech is available when building return true from have_speech static 
 method.
 Otherwise return false.
 Could also add a check to see if there are any backend errors later on.
 
 
 Diffs
 -
 
   CMakeLists.txt f5aa0e78d89a4c14c3acfd9384b100e507e30067 
   src/knotifyconfigactionswidget.h 88b341a909d466b8eaf065c5220ac05f8c963697 
   src/knotifyconfigactionswidget.cpp 27012e3b803ef0a99d5bafcebea270e2d7419d62 
   src/knotifyconfigactionswidgetbase.ui 
 cbe647b70eb41ba540b7bf6d87f519b3246973a1 
   src/knotifyconfigelement.h 703952d23d1622e4b6214acc52461ea75b480254 
   src/knotifyconfigelement.cpp 5a1ac57fa5a4139682327d14c55a489450ea956b 
   src/knotifyeventlist.cpp 148bca7d33c722a4aeecabac45286a5e501c81b3 
 
 Diff: https://git.reviewboard.kde.org/r/122554/diff/
 
 
 Testing
 ---
 
 It builds, the config widget shows, the icon for speech is properly showing 
 when that configuration is enabled. Speech from konversation's new message 
 notifications are spoken.
 
 Something strange here:
 After enabling this notification, no messages are spoken until restarting the 
 application with the notifications (konversation in my test case).
 Similarly, after disabling this notification, messages are still spoken until 
 restarting the application.
 I checked and the konversation.notifyrc file is getting updated, but for some 
 reason the changes aren't taking effect imediately yet.
 
 
 Thanks,
 
 Jeremy Whiting
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 122554: knotifyconfig: Add optional dependency on QtSpeech to reenable speaking notifications.

2015-02-13 Thread Jeremy Whiting


 On Feb. 13, 2015, 4:49 a.m., Aleix Pol Gonzalez wrote:
  +1
  
  What's the status of QtSpeech?

I'll blog about it shortly.


- Jeremy


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122554/#review75983
---


On Feb. 12, 2015, 8:06 p.m., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122554/
 ---
 
 (Updated Feb. 12, 2015, 8:06 p.m.)
 
 
 Review request for KDE Frameworks and Frederik Gladhorn.
 
 
 Repository: knotifyconfig
 
 
 Description
 ---
 
 Change config from KTTS to Speech.
 If QtSpeech is available when building return true from have_speech static 
 method.
 Otherwise return false.
 Could also add a check to see if there are any backend errors later on.
 
 
 Diffs
 -
 
   CMakeLists.txt f5aa0e78d89a4c14c3acfd9384b100e507e30067 
   src/knotifyconfigactionswidget.h 88b341a909d466b8eaf065c5220ac05f8c963697 
   src/knotifyconfigactionswidget.cpp 27012e3b803ef0a99d5bafcebea270e2d7419d62 
   src/knotifyconfigactionswidgetbase.ui 
 cbe647b70eb41ba540b7bf6d87f519b3246973a1 
   src/knotifyconfigelement.h 703952d23d1622e4b6214acc52461ea75b480254 
   src/knotifyconfigelement.cpp 5a1ac57fa5a4139682327d14c55a489450ea956b 
   src/knotifyeventlist.cpp 148bca7d33c722a4aeecabac45286a5e501c81b3 
 
 Diff: https://git.reviewboard.kde.org/r/122554/diff/
 
 
 Testing
 ---
 
 It builds, the config widget shows, the icon for speech is properly showing 
 when that configuration is enabled. Speech from konversation's new message 
 notifications are spoken.
 
 Something strange here:
 After enabling this notification, no messages are spoken until restarting the 
 application with the notifications (konversation in my test case).
 Similarly, after disabling this notification, messages are still spoken until 
 restarting the application.
 I checked and the konversation.notifyrc file is getting updated, but for some 
 reason the changes aren't taking effect imediately yet.
 
 
 Thanks,
 
 Jeremy Whiting
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 122554: knotifyconfig: Add optional dependency on QtSpeech to reenable speaking notifications.

2015-02-13 Thread Jeremy Whiting

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122554/
---

(Updated Feb. 13, 2015, 12:42 p.m.)


Review request for KDE Frameworks and Frederik Gladhorn.


Changes
---

Updated to use TTS name instead of Speech.


Repository: knotifyconfig


Description
---

Change config from KTTS to Speech.
If QtSpeech is available when building return true from have_speech static 
method.
Otherwise return false.
Could also add a check to see if there are any backend errors later on.


Diffs (updated)
-

  CMakeLists.txt f5aa0e78d89a4c14c3acfd9384b100e507e30067 
  src/knotifyconfigactionswidget.h 88b341a909d466b8eaf065c5220ac05f8c963697 
  src/knotifyconfigactionswidget.cpp 27012e3b803ef0a99d5bafcebea270e2d7419d62 
  src/knotifyconfigactionswidgetbase.ui 
cbe647b70eb41ba540b7bf6d87f519b3246973a1 
  src/knotifyconfigelement.h 703952d23d1622e4b6214acc52461ea75b480254 
  src/knotifyconfigelement.cpp 5a1ac57fa5a4139682327d14c55a489450ea956b 
  src/knotifyeventlist.cpp 148bca7d33c722a4aeecabac45286a5e501c81b3 

Diff: https://git.reviewboard.kde.org/r/122554/diff/


Testing
---

It builds, the config widget shows, the icon for speech is properly showing 
when that configuration is enabled. Speech from konversation's new message 
notifications are spoken.

Something strange here:
After enabling this notification, no messages are spoken until restarting the 
application with the notifications (konversation in my test case).
Similarly, after disabling this notification, messages are still spoken until 
restarting the application.
I checked and the konversation.notifyrc file is getting updated, but for some 
reason the changes aren't taking effect imediately yet.


Thanks,

Jeremy Whiting

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 122554: knotifyconfig: Add optional dependency on QtSpeech to reenable speaking notifications.

2015-02-13 Thread Frederik Gladhorn


 On Feb. 13, 2015, 11:49 a.m., Aleix Pol Gonzalez wrote:
  +1
  
  What's the status of QtSpeech?
 
 Jeremy Whiting wrote:
 I'll blog about it shortly.

I think QtSpeech is Qt 5.6 material realistically. It's usable on most 
platforms but needs cleanup and API review and a couple of features are not 
done on all platforms.


- Frederik


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122554/#review75983
---


On Feb. 13, 2015, 7:42 p.m., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122554/
 ---
 
 (Updated Feb. 13, 2015, 7:42 p.m.)
 
 
 Review request for KDE Frameworks and Frederik Gladhorn.
 
 
 Repository: knotifyconfig
 
 
 Description
 ---
 
 Change config from KTTS to Speech.
 If QtSpeech is available when building return true from have_speech static 
 method.
 Otherwise return false.
 Could also add a check to see if there are any backend errors later on.
 
 
 Diffs
 -
 
   CMakeLists.txt f5aa0e78d89a4c14c3acfd9384b100e507e30067 
   src/knotifyconfigactionswidget.h 88b341a909d466b8eaf065c5220ac05f8c963697 
   src/knotifyconfigactionswidget.cpp 27012e3b803ef0a99d5bafcebea270e2d7419d62 
   src/knotifyconfigactionswidgetbase.ui 
 cbe647b70eb41ba540b7bf6d87f519b3246973a1 
   src/knotifyconfigelement.h 703952d23d1622e4b6214acc52461ea75b480254 
   src/knotifyconfigelement.cpp 5a1ac57fa5a4139682327d14c55a489450ea956b 
   src/knotifyeventlist.cpp 148bca7d33c722a4aeecabac45286a5e501c81b3 
 
 Diff: https://git.reviewboard.kde.org/r/122554/diff/
 
 
 Testing
 ---
 
 It builds, the config widget shows, the icon for speech is properly showing 
 when that configuration is enabled. Speech from konversation's new message 
 notifications are spoken.
 
 Something strange here:
 After enabling this notification, no messages are spoken until restarting the 
 application with the notifications (konversation in my test case).
 Similarly, after disabling this notification, messages are still spoken until 
 restarting the application.
 I checked and the konversation.notifyrc file is getting updated, but for some 
 reason the changes aren't taking effect imediately yet.
 
 
 Thanks,
 
 Jeremy Whiting
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 122554: knotifyconfig: Add optional dependency on QtSpeech to reenable speaking notifications.

2015-02-13 Thread Frederik Gladhorn


 On Feb. 13, 2015, 12:17 p.m., Martin Klapetek wrote:
  src/knotifyconfigactionswidgetbase.ui, line 141
  https://git.reviewboard.kde.org/r/122554/diff/1/?file=348626#file348626line141
 
  Is Jovie still used in QtSpeech? If not, it should be removed from here

I tend to agree to TTS or even better Text to Speech/Text-To-Speech.


- Frederik


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122554/#review75987
---


On Feb. 13, 2015, 7:42 p.m., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122554/
 ---
 
 (Updated Feb. 13, 2015, 7:42 p.m.)
 
 
 Review request for KDE Frameworks and Frederik Gladhorn.
 
 
 Repository: knotifyconfig
 
 
 Description
 ---
 
 Change config from KTTS to Speech.
 If QtSpeech is available when building return true from have_speech static 
 method.
 Otherwise return false.
 Could also add a check to see if there are any backend errors later on.
 
 
 Diffs
 -
 
   CMakeLists.txt f5aa0e78d89a4c14c3acfd9384b100e507e30067 
   src/knotifyconfigactionswidget.h 88b341a909d466b8eaf065c5220ac05f8c963697 
   src/knotifyconfigactionswidget.cpp 27012e3b803ef0a99d5bafcebea270e2d7419d62 
   src/knotifyconfigactionswidgetbase.ui 
 cbe647b70eb41ba540b7bf6d87f519b3246973a1 
   src/knotifyconfigelement.h 703952d23d1622e4b6214acc52461ea75b480254 
   src/knotifyconfigelement.cpp 5a1ac57fa5a4139682327d14c55a489450ea956b 
   src/knotifyeventlist.cpp 148bca7d33c722a4aeecabac45286a5e501c81b3 
 
 Diff: https://git.reviewboard.kde.org/r/122554/diff/
 
 
 Testing
 ---
 
 It builds, the config widget shows, the icon for speech is properly showing 
 when that configuration is enabled. Speech from konversation's new message 
 notifications are spoken.
 
 Something strange here:
 After enabling this notification, no messages are spoken until restarting the 
 application with the notifications (konversation in my test case).
 Similarly, after disabling this notification, messages are still spoken until 
 restarting the application.
 I checked and the konversation.notifyrc file is getting updated, but for some 
 reason the changes aren't taking effect imediately yet.
 
 
 Thanks,
 
 Jeremy Whiting
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 122554: knotifyconfig: Add optional dependency on QtSpeech to reenable speaking notifications.

2015-02-13 Thread Martin Klapetek

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122554/#review75987
---


As I said in the other review, I believe the user facing strings should use 
TTS rather than Speech as TTS or Text-To-Speech is an estabilished 
abbreviation in the field. Any other opinions on that? Frederik?

As for the config not updating, can you try this:

```
 src/knotifyconfig.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/knotifyconfig.cpp b/src/knotifyconfig.cpp
index af6be92..e9b684b 100644
--- a/src/knotifyconfig.cpp
+++ b/src/knotifyconfig.cpp
@@ -37,6 +37,7 @@ static KSharedConfig::Ptr retrieve_from_cache(const QString 
filename, QStandard
 {
 QCacheQString, KSharedConfig::Ptr cache = *static_cache;
 if (cache.contains(filename)) {
+(*cache[filename])-reparseConfiguration();
 return *cache[filename];
 }
 

```


src/knotifyconfigactionswidgetbase.ui
https://git.reviewboard.kde.org/r/122554/#comment52421

Is Jovie still used in QtSpeech? If not, it should be removed from here


- Martin Klapetek


On Feb. 13, 2015, 4:06 a.m., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122554/
 ---
 
 (Updated Feb. 13, 2015, 4:06 a.m.)
 
 
 Review request for KDE Frameworks and Frederik Gladhorn.
 
 
 Repository: knotifyconfig
 
 
 Description
 ---
 
 Change config from KTTS to Speech.
 If QtSpeech is available when building return true from have_speech static 
 method.
 Otherwise return false.
 Could also add a check to see if there are any backend errors later on.
 
 
 Diffs
 -
 
   CMakeLists.txt f5aa0e78d89a4c14c3acfd9384b100e507e30067 
   src/knotifyconfigactionswidget.h 88b341a909d466b8eaf065c5220ac05f8c963697 
   src/knotifyconfigactionswidget.cpp 27012e3b803ef0a99d5bafcebea270e2d7419d62 
   src/knotifyconfigactionswidgetbase.ui 
 cbe647b70eb41ba540b7bf6d87f519b3246973a1 
   src/knotifyconfigelement.h 703952d23d1622e4b6214acc52461ea75b480254 
   src/knotifyconfigelement.cpp 5a1ac57fa5a4139682327d14c55a489450ea956b 
   src/knotifyeventlist.cpp 148bca7d33c722a4aeecabac45286a5e501c81b3 
 
 Diff: https://git.reviewboard.kde.org/r/122554/diff/
 
 
 Testing
 ---
 
 It builds, the config widget shows, the icon for speech is properly showing 
 when that configuration is enabled. Speech from konversation's new message 
 notifications are spoken.
 
 Something strange here:
 After enabling this notification, no messages are spoken until restarting the 
 application with the notifications (konversation in my test case).
 Similarly, after disabling this notification, messages are still spoken until 
 restarting the application.
 I checked and the konversation.notifyrc file is getting updated, but for some 
 reason the changes aren't taking effect imediately yet.
 
 
 Thanks,
 
 Jeremy Whiting
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 122554: knotifyconfig: Add optional dependency on QtSpeech to reenable speaking notifications.

2015-02-12 Thread Jeremy Whiting

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122554/
---

Review request for KDE Frameworks and Frederik Gladhorn.


Repository: knotifyconfig


Description
---

Change config from KTTS to Speech.
If QtSpeech is available when building return true from have_speech static 
method.
Otherwise return false.
Could also add a check to see if there are any backend errors later on.


Diffs
-

  CMakeLists.txt f5aa0e78d89a4c14c3acfd9384b100e507e30067 
  src/knotifyconfigactionswidget.h 88b341a909d466b8eaf065c5220ac05f8c963697 
  src/knotifyconfigactionswidget.cpp 27012e3b803ef0a99d5bafcebea270e2d7419d62 
  src/knotifyconfigactionswidgetbase.ui 
cbe647b70eb41ba540b7bf6d87f519b3246973a1 
  src/knotifyconfigelement.h 703952d23d1622e4b6214acc52461ea75b480254 
  src/knotifyconfigelement.cpp 5a1ac57fa5a4139682327d14c55a489450ea956b 
  src/knotifyeventlist.cpp 148bca7d33c722a4aeecabac45286a5e501c81b3 

Diff: https://git.reviewboard.kde.org/r/122554/diff/


Testing
---

It builds, the config widget shows, the icon for speech is properly showing 
when that configuration is enabled. Speech from konversation's new message 
notifications are spoken.

Something strange here:
After enabling this notification, no messages are spoken until restarting the 
application with the notifications (konversation in my test case).
Similarly, after disabling this notification, messages are still spoken until 
restarting the application.
I checked and the konversation.notifyrc file is getting updated, but for some 
reason the changes aren't taking effect imediately yet.


Thanks,

Jeremy Whiting

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel