No longer use `str` in speech sequences

feerrenrut commented 3 years ago

Is your feature request related to a problem? Please describe.

Improving semantics for internal speech processing is held back by maintaining add-on compatibility.

Describe the solution you'd like

Make str and invalid speech sequence item.
All text should be wrapped to give semantics. See UserInterface and Symbol commands in #12710
A common interface for all text content (but with more semantics than str) allows addons to consume this, while giving NVDA core developers flexibility to extend and refine the semantics used.

EG: Once addons all use:

for command in sequence if isinstance(command, TextContent):
    command.text = f"Add more info: {command.text}"

We can gradually create subclasses of TextContent with more specific semantics:

Symbol
UserInterface
Content
Context (https://github.com/nvaccess/nvda/pull/12710#issuecomment-1031277294)

Then we can offer features like UserInterface uses a different voice from content.

Describe alternatives you've considered

None

Additional context

Very relevant to this is issue #12778 "Split speech processing commands and commands for synth"

CyrilleB79 commented 2 years ago

Hi @feerrenrut

Since this one is an API-breaking change, do NVAccess devs plan themselves to implement it for 2023.1 release? Or do you need some help for this?

This was initially planned for 2022.1 but was associated with #11779's fix and was reverted due to issues linked with #11779's fix.

Probably this issue should be fixed for 2023.1 with API-breaking changes. Then #11779 could be resolved in 2023.1 or a later release.

Could you give an update of NVAccess' opinion on the resolution of this issue? Thanks.

seanbudd commented 2 years ago

@CyrilleB79, we do have plans to pick this up again for 2023.1. However, it is a large and risky piece of work, that might not make it in again. Having assistance reviving #12710 would be helpful. At the very least it would get momentum on this sooner and increase chances of getting this change in 2023.1. It would be great to address any remaining issues in #12710, and get testing as soon as possible. This will require a large breadth of testing. It would be good to cover smoke tests, system and unit tests for changed code. I suggest continuing work by opening PRs against speakSymsMoveByWord, which will become a feature branch via #14287

CyrilleB79 commented 2 years ago

Wouldn't it be a better strategy to decorrelate the resolution of:

No longer use 'str' in speech sequence (#12779), which is an API-breaking change
Issues related to symbol reporting when moving by word (#10855, #11779), which are blocked by #12779 but does not bring further API-breaking changes.

This would allow to introduce API-breaking changes for 2023.1 and have more time to resolve issues with symbols during word navigation if needed.

seanbudd commented 2 years ago

@CyrilleB79 - It is not apparent that these issues can be tackled independently, but if you find a way, that would be appropriate.

Adriani90 commented 4 days ago

cc: @beqabeqa473, @nvdaes, @ctoth this might be of interest for you.

Adriani90 commented 4 days ago

@seanbudd would fixing of this issue have a negative impact on current word by word navigation including symbols? I don't see any negative impact for now, but maybe I am wrong.

nvaccess / nvda