Add Pause/Resume action for pausing and resuming speech synthesis.
AbandonedPublic

Authored by whiting on Sep 19 2019, 10:56 PM.

Details

Summary

Only enable the action when there's text being spoken or when speech is paused.
Also added ability to select the QtSpeech plugin/engine to use so it doesn't randomly choose one for us.

Test Plan

Pause and resume are working here.

Diff Detail

Repository
R223 Okular
Branch
pauseresumespeech
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17116
Build 17134: arc lint + arc unit
whiting created this revision.Sep 19 2019, 10:56 PM
Restricted Application added a project: Okular. · View Herald TranscriptSep 19 2019, 10:56 PM
Restricted Application added a subscriber: okular-devel. · View Herald Transcript
whiting requested review of this revision.Sep 19 2019, 10:56 PM
whiting edited the test plan for this revision. (Show Details)Sep 19 2019, 10:57 PM
whiting added a reviewer: aacid.
sander added a subscriber: sander.Sep 20 2019, 6:51 AM
sander added inline comments.
ui/pageview.cpp
2613

Please avoid this whitespace change.

whiting updated this revision to Diff 66553.Sep 20 2019, 5:08 PM

Removed whitespace changes.

whiting marked an inline comment as done.Sep 20 2019, 5:08 PM
whiting updated this revision to Diff 66795.Wed, Sep 25, 1:25 AM
  • Add user okular user setting for which QtSpeech engine to use.
whiting edited the summary of this revision. (Show Details)Wed, Sep 25, 1:28 AM
whiting edited the test plan for this revision. (Show Details)
aacid added a comment.Wed, Sep 25, 8:13 PM

Sounds sensible, what about the TODO you have on the top?

conf/dlgaccessibility.cpp
38

const

39

use for ( : )

whiting updated this revision to Diff 66860.Wed, Sep 25, 10:30 PM

Changed engines stringlist to const and use for instead of foreach.

Also removed context menu action for pause resume

  • Add user okular user setting for which QtSpeech engine to use.
whiting updated this revision to Diff 66865.Thu, Sep 26, 5:48 AM
  • Only enable Pause or Resume when speaking or paused.
whiting updated this revision to Diff 66866.Thu, Sep 26, 6:10 AM
  • When tts engine is changed via okular settings recreate tts object.

Sounds sensible, what about the TODO you have on the top?

Done. I think this is ready to use now. When users change the engine in the config dialog the tts object stops any speech and recreates the underlying QTetxToSpeech object with the chosen engine. Stop action is also enabled when paused or speaking, the same as the pause/resume action.

whiting updated this revision to Diff 66979.Sat, Sep 28, 4:55 AM
  • Fix building okular without QTextToSpeech by adding ifdefs.

Sorry for being a pain, but your patch contains lots of whitespace changes again (in conf/dlgaccessibilitybase.ui). I agree that you probably do want them changed, but please do so in a separate patch.

Besides, can you please post your patch as a merge request on https://invent.kde.org/kde/okular/ ? That is not purely cosmetic -- invent.kde.org has a CI system.

Besides that: I like the patch!

conf/dlgaccessibility.cpp
43

This doesn't follow the coding style in the rest of the file: There should be a whitespace right after '(' and before ')'.

whiting edited the summary of this revision. (Show Details)Thu, Oct 3, 3:22 PM

Sorry for being a pain, but your patch contains lots of whitespace changes again (in conf/dlgaccessibilitybase.ui). I agree that you probably do want them changed, but please do so in a separate patch.

Besides, can you please post your patch as a merge request on https://invent.kde.org/kde/okular/ ? That is not purely cosmetic -- invent.kde.org has a CI system.

Besides that: I like the patch!

Ok, I'll fix the whitespace and move it to the other review tool, no problem.

Sorry for being a pain, but your patch contains lots of whitespace changes again (in conf/dlgaccessibilitybase.ui). I agree that you probably do want them changed, but please do so in a separate patch.

Besides, can you please post your patch as a merge request on https://invent.kde.org/kde/okular/ ? That is not purely cosmetic -- invent.kde.org has a CI system.

Besides that: I like the patch!

Ok, I'll fix the whitespace and move it to the other review tool, no problem.

Ok, removed whitespace changes that designer did for me and pushed to my clone and created new merge request here: https://invent.kde.org/kde/okular/merge_requests/51

aacid added a comment.Mon, Oct 7, 6:02 PM

Can you please close this then?

whiting abandoned this revision.Mon, Oct 7, 6:09 PM

Yep, Abandoning.