Skip to content

Don't fail to speak single letters if speechViewer or speech logging is enabled#10879

Merged
michaelDCurran merged 2 commits into
masterfrom
i10741
Mar 17, 2020
Merged

Don't fail to speak single letters if speechViewer or speech logging is enabled#10879
michaelDCurran merged 2 commits into
masterfrom
i10741

Conversation

@michaelDCurran

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #10741

Summary of the issue:

NVDa fails to announce single letters when moving with the arrow keys and or reviewing by character, if either speechViewer is enabled, or the speech logging category is enabled in the Advanced settings panel.
The reason for this is that when speaking single letters, speech.speak is passed in a generator as the sequence, yet this generator tries to be iterated over up to 3 times (logging bad sequences, appending to speechViewer, and sending to the synth).

Description of how this pull request fixes the issue:

If the given sequence is a generator, flatten it into a list first.

Testing performed:

With neither speechViewer or speech logging enabled, speechViewer enabled but not speech logging, speech logging but not speechViewer, and both speechViewer and speech logging, move through text in notepad with the left and right arrows, ensuring that each character is read aloud.

Known issues with pull request:

None.

Change log entry:

None needed -- Regression introduced after 2019.3.

… into a list first, as we may be iterating over it multiple times (E.g. logging bad sequence types, appending to speech viewer, sending to the synth).
@LeonarddeR

Copy link
Copy Markdown
Collaborator

Could you elaborate on when a speech sequence is a generator? Note that speech.SpeechSequence defines a sequence to be a list, so a generator would be offending the type annotation.

@michaelDCurran

Copy link
Copy Markdown
Member Author

@LeonarddeR it seems that getSpellingSpeech is returning a generator and this is what is supplied to speech.speak. I'll let @feerrenrut provide more info on why he had chosen to return generators rather than lists.

@michaelDCurran michaelDCurran merged commit 28c6a72 into master Mar 17, 2020
@michaelDCurran michaelDCurran deleted the i10741 branch March 17, 2020 04:27
@nvaccessAuto nvaccessAuto added this to the 2020.1 milestone Mar 17, 2020
@feerrenrut

Copy link
Copy Markdown
Contributor

@michaelDCurran @LeonarddeR Actually getSpellingSpeech (previously called getSpeechForSpelling) was a generator as of the speech refactor. Before this, it was a generator too, but worked in a different way. The loose and not entirely correct definition of a speech sequence is a bit of a problem, it could be redefined as an iterable rather than a list? I think I would prefer to replace speech sequence with a more formal type. Currently getSpellingSpeech and speakWithoutPauses return generators. Theoretically this is helpful because speech can start before a very large chunk of text is processed. As I understand it, a generator for speakWithoutPauses is necessary for the current design of sayAllHandler, so that speech can start (and trigger call-backs) before speakWithoutPauses has finished. At least when updating speech functions to return sequences it seemed necessary to use a generator to match code paths previously taken. Of course this goes out the window if subsequent calls iterate the sequence prior to speaking. Perhaps none of these functions should be returning generators, and this kind of optimisation should be applied at another level, maybe it already is.

@tspivey

tspivey commented Mar 27, 2020

Copy link
Copy Markdown
Collaborator

One problem with this is that any addon which patches speech.speak (for example Speech History) now has to know about flattening these lists. If they don't, then characters don't read because of the generators.

@ruifontes

ruifontes commented Mar 27, 2020 via email

Copy link
Copy Markdown
Contributor

@lukaszgo1

Copy link
Copy Markdown
Contributor

@michaelDCurran @feerrenrut Are you planning to do anything with this commend by @tspivey ? This is quite problematic because in the current state breaking changes should not be made as there was no update to the addonAPI

feerrenrut added a commit that referenced this pull request Apr 20, 2020
PRs #10878 and #10879 addressed some issues with Speech.speak
 being passed a Generator.
Rather than create a list from it, ensure that the generator
 is not passed in the first place.
The typing of several functions is no much more clear,
 in particular _flattenNestedSequences now no longer
 implicitly converts generators to lists.
However, in all places it is used, generators are explicitly
 converted to lists.
Logging of generator types has been added in several places,
 enabled via the advanced settings panel 'speech' debug
 logging category.
feerrenrut added a commit that referenced this pull request Apr 20, 2020
PRs #10878 and #10879 addressed some issues with Speech.speak
 being passed a Generator.
Rather than create a list from it, ensure that the generator
 is not passed in the first place.
The typing of several functions is no much more clear,
 in particular _flattenNestedSequences now no longer
 implicitly converts generators to lists.
However, in all places it is used, generators are explicitly
 converted to lists.
Logging of generator types has been added in several places,
 enabled via the advanced settings panel 'speech' debug
 logging category.
feerrenrut added a commit that referenced this pull request Apr 21, 2020
PRs #10878 and #10879 addressed some issues with Speech.speak
 being passed a Generator.

Fixed a bug in speakTextInfo. speakWithoutPauses does not accept a 
 generator as it needs to be able to get the number of items in the
 sequence and slice it.

Rather than create a list from the generator inside Speech.speak,
ensure that the generator is not used as an argument in the first place.

The typing of several functions has now been clarified,
 in particular _flattenNestedSequences now no longer
 implicitly converts generators to lists.

However, now all usages of _flattenNestedSequences explicitly
 convert to lists.

Logging of generator types has been added in several places
 including speakWithoutPauses which needs to be able to get
 the length of a sequence and slice it. The logging can be enabled
 via the advanced settings panel 'speech' debug logging category.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NVDA alpha does not speak single letters when editing text with speech viewer enabled

7 participants