Add a command to show the symbol replacement by nvdaes · Pull Request #9287 · nvaccess/nvda

nvdaes · 2019-02-17T04:34:22Z

Link to issue number:

Summary of the issue:

Sometimes, it's useful to review and copy symbol replacements, to ensure how they are writen.

Description of how this pull request fixes the issue:

A command has been added to show the replacement for the symbol where the review cursor is placed. This information is presented in browse mode, using ui.browseableMessage().

The dialog title shows the language for which symbols can be edited when the command is activated, that is, autoLanguageSwitching setting is not considered. Only the language used for SpeechSymbols dialog is addressed. This is retrieved using the same code contained in the gui to present the language in the dialog title for Speech Symbols, i.e., using characterProcessing and languageHandler.getLanguageDescription() for the corresponding locale.

The text corresponding to the current character is retrieved via text property of textInfo object of api.getReviewPosition(). This text is compared with the corresponding characterProcessing.processSpeechSymbol(). If the results are differents, then the symbol replacement is present and is shown in browse mode, after the symbol itself, for example:

Expanded symbol (English)
,
comma

Testing performed:

Assign a gesture (NVDA+alt+.) for the command.
Place the review cursor on different symbols and press NVDA+alt+., using different voices like variants of English and Spanish.

Known issues with pull request:

None

Change log entry:

Section: New features
Added a command to show the replacement for the symbol where the review cursor is positioned (#9286)

…anguage * The gesture can be assigned in text review category * In the symbol itself, corresponding to the character where the review cursor is placed, and the replacement, are shown in browse mode * The title of the browse mode dialog contains the language for the current voice, i.e., the language for which the symbols can be edited from Preferences submenu

lukaszgo1 · 2019-02-17T19:33:36Z

Would it be possible to add a second command to show a replacement for the symbol on which cared is located? For people who like me works with review cursor not following the cared it would be a huge benefit.

nvdaes · 2019-02-17T19:54:40Z

Replying to lukaszgo1:

Would it be possible to add a second command to show a replacement for the symbol on which cared is located? For people who like me works with review cursor not following the cared it would be a huge benefit.

I started this PR before it's accepted since, if it isn't, I will use the code for Emoticons add-on. Let's wait for other feedback and reviews, but your suggestion could make sense. I haven't assigned a gesture for this command so we don't waste too much keystrokes, and I think that, if only it's possible to add a unique command, it's better to use review cursor, not caret, for flexibility. But both commands could benefit.

BTW, ui.browseableMessage() is the better option for me since it's very navigable. Anyway, it can be opened multiple times, but this maybe considered in a separate issue, since this affects for example to formatting dialog (which can be opened NVDA+f twice).

LeonarddeR

You do not need a special symbol processor for this, just use speech.getCurrentLanguage. I can see why you don't take language switching into account, though I consider this a bit of a drawback to this approach. I'm not sure what's currently the best way to do this.
Please make this script similar to the report formatting script (i.e. just speak the info at first press and show a browsable message at second press. I think that's more consistent.

LeonarddeR · 2019-02-19T18:49:08Z

Actually, I think it must be possible to get the appropriate language from the textInfo in a similar way to how speech.speakTextInfo does it.

nvdaes · 2019-02-19T18:58:24Z

Yes, I will try to do it. Enviado desde mi iPhone

…

El 19 feb 2019, a las 19:49, Leonard de Ruijter ***@***.***> escribió: Actually, I think it must be possible to get the appropriate language from the textInfo in a similar way to how speech.speakTextInfo does it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

nvdaes · 2019-02-19T22:27:25Z

@LeonarddeR, I have addresed your request and have tested this in NV Access website with autoLanguageSwitching activated, and in Notepad. Also, pressing twice on normal characters (then the browseable message is not shown), and over symbols.
I have changed also the user guide.
Feel free to request more changes or edit what you want.

…nt language

nvdaes · 2019-02-20T05:32:52Z

I have addressed other requested change, not using symbolProcessor, but just current language.
If we could add a gesture in NVDA's core for this, it will be great, though this is not needed since it can be done from text review category.
I think this is ready from my part.
Comments will be appreciated.

LeonarddeR · 2019-05-09T17:27:33Z

+		expandedSymbol = characterProcessing.processSpeechSymbol(curLanguage, text)
+		repeats=scriptHandler.getLastScriptRepeatCount()
+		if repeats == 0:
+			# Explicitly tether here


Rather than tethering and using speech.SpeakTextInfo, I think I'd prefer just calling ui.message(expandedSymbol)

LeonarddeR · 2019-05-09T17:31:17Z

+			# Explicitly tether here
+			braille.handler.setTether(braille.handler.TETHER_REVIEW, auto=True)
+			speech.speakTextInfo(info,unit=textInfos.UNIT_CHARACTER,reason=controlTypes.REASON_CARET)
+		elif expandedSymbol != text:


There is some overlap between this script and the script_review_currentCharacter script. On closer inspection, I think it makes sense to let this script only report something if there is a real replacement. In other words, if the current character isn't replaced by something else, it might make sense to report just that: No symbol pronunciation rule for {text}. @feerrenrut: thoughts?

LeonarddeR · 2019-05-09T17:36:38Z

+			speech.speakTextInfo(info,unit=textInfos.UNIT_CHARACTER,reason=controlTypes.REASON_CARET)
+		elif expandedSymbol != text:
+			# Translators: title for expanded symbol dialog.
+			ui.browseableMessage("%s\n%s" % (text, expandedSymbol), _("Expanded symbol %s" % languageHandler.getLanguageDescription(curLanguage)))


For speech users, I think it is confusing that both the original and replacement are shown here. I initially thought that the replacement was just duplicated.

Suggested change

ui.browseableMessage("%s\n%s" % (text, expandedSymbol), _("Expanded symbol %s" % languageHandler.getLanguageDescription(curLanguage)))

ui.browseableMessage(_(

"Character: {character}\n"

"Replacement: {replacement}"

).format(character=text, replacement=expandedSymbol),

_("Expanded symbol ({language})").format(

language=languageHandler.getLanguageDescription(curLanguage)

))

LeonarddeR · 2019-05-09T17:39:18Z

+		elif expandedSymbol != text:
+			# Translators: title for expanded symbol dialog.
+			ui.browseableMessage("%s\n%s" % (text, expandedSymbol), _("Expanded symbol %s" % languageHandler.getLanguageDescription(curLanguage)))
+	script_review_currentSymbol.__doc__=_("Speaks the symbol where the review cursor is positioned. Pressed twice, shows the symbol and the text used to speak it in browse mode")


Suggested change

script_review_currentSymbol.__doc__=_("Speaks the symbol where the review cursor is positioned. Pressed twice, shows the symbol and the text used to speak it in browse mode")

script_review_currentSymbol.__doc__=_(

"Reports the symbol where the review cursor is positioned. "

"Pressed twice, shows the symbol and the text used to speak it in browse mode"

)

LeonarddeR · 2019-05-09T17:40:27Z

 | Select then Copy from review cursor | NVDA+f9 | NVDA+f9 | none | Starts the select then copy process from the current position of the review cursor. The actual action is not performed until you tell NVDA where the end of the text range is |
 | Select then Copy to review cursor | NVDA+f10 | NVDA+f10 | none | On the first press, text is selected from the position previously set start marker up to and including the review cursor's current position. After pressing this key a second time, the text will be copied to the Windows clipboard |
 | Report text formatting | NVDA+f | NVDA+f | none | Reports the formatting of the text where the review cursor is currently situated. Pressing twice shows the information in browse mode |
+| Report current symbol replacement | None | None | none | Speaks the symbol where the review cursor is positioned. Pressed twice, shows the symbol and the text used to speak it in browse mode. |


Are there more entries in here that have no default assignments?

nvdaes · 2019-05-10T19:47:19Z

@LeonarddeR , requested changes have been addressed in 85a0e3d

@LeonarddeR asked:

In user_docs/en/userGuide.t2t:
@@ -401,6 +401,7 @@ The following commands are available for reviewing text:

| Select then Copy from review cursor | NVDA+f9 | NVDA+f9 | none | Starts the select then copy process from the current position of the review cursor. The actual action is not performed until you tell NVDA where the end of the text range is |

| Select then Copy to review cursor | NVDA+f10 | NVDA+f10 | none | On the first press, text is selected from the position previously set start marker up to and including the review cursor's current position. After pressing this key a second time, the text will be copied to the Windows clipboard |

Are there more entries in here that have no default assignments?

I think this would be the unique entry. Could we use windows+NVDA+. as an assigned gesture for this command?
NVDA+. is used in laptop keyboard for reviewing the current character, and windows+., to use the emoji panel on Win10. so maybe a gesture easy to remember.
Please feel free to request any other changes or change what you want.

Thanks

dpy013 · 2019-05-10T22:37:22Z

The following error occurred while building NVDA17213:
checkPotAction(["tests \ checkPot"],["output \ nvda_snapshot_pr9287-17213,35d31059.pot"]) The message has no translation comments. Source: globalCommands.py:1212 Message: Character: {character} \ n 1 error, 0 unexpected success, 75 expected error scons: *** [tests \ checkPot] error 1 scons: The building terminated due to an error. Command exit code 2
thank
cc
@nvdaes

…itioned at a symbol - Use %s instead of format, and add translator comments for _("Character %s") and _("Replacement %s") - When the cursor is not positioned at a symbol, report just _("No symbol") instead of announcing the character where it's situated, to avoid inconsistent messages if the text is blank.

nvdaes · 2019-05-11T01:54:17Z

Fixed in dfdab0f Thanks El 11/05/2019 a las 0:37, eric escribió:

…

The following error occurred while building NVDA17213: checkPotAction(["tests \ checkPot"],["output \ nvda_snapshot_pr9287-17213,35d31059.pot"]) The message has no translation comments. Source: globalCommands.py:1212 Message: Character: {character} \ n 1 error, 0 unexpected success, 75 expected error scons: *** [tests \ checkPot] error 1 scons: The building terminated due to an error. Command exit code 2 thank cc @nvdaes <https://github.com/nvdaes> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9287 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADYTVZDQC74CVDDCDVFHR2LPUX2KZANCNFSM4GX5OG4A>.

LeonarddeR · 2019-05-13T17:38:58Z

+		expandedSymbol = characterProcessing.processSpeechSymbol(curLanguage, text)
+		if expandedSymbol == text:
+			# Translators: Reported when there is no symbol at the position of the review cursor.
+			ui.message(_("No symbol"))


I would prefer "no symbol replacement". NO symbol is ambiguous in the case where the cursor is at a symbol, but the symbol has no replacement defined.

* When the symbol at the review cursor has no replacement, specify that there is no replacement for it, instead of mentioning that there is no symbol at this position.

nvdaes · 2019-05-13T18:59:35Z

OK, addressed in 6b1bf1e El 13/05/2019 a las 19:39, Leonard de Ruijter escribió:

…

***@***.**** commented on this pull request. ------------------------------------------------------------------------ In source/globalCommands.py <#9287 (comment)>: > @@ -1188,6 +1188,36 @@ def script_review_endOfLine(self,gesture): script_review_endOfLine.__doc__=_("Moves the review cursor to the last character of the line where it is situated in the current navigator object and speaks it") script_review_endOfLine.category=SCRCAT_TEXTREVIEW + def script_review_currentSymbol(self,gesture): + info=api.getReviewPosition().copy() + info.expand(textInfos.UNIT_CHARACTER) + text = info.text + curLanguage = None + if config.conf['speech']['autoLanguageSwitching']: + for field in info.getTextWithFields({}): + if isinstance(field,textInfos.FieldCommand) and field.command=="formatChange": + curLanguage=field.field.get('language') + if curLanguage is None: + curLanguage = speech.getCurrentLanguage() + expandedSymbol = characterProcessing.processSpeechSymbol(curLanguage, text) + if expandedSymbol == text: + # Translators: Reported when there is no symbol at the position of the review cursor. + ui.message(_("No symbol")) I would prefer "no symbol replacement". NO symbol is ambiguous in the case where the cursor is at a symbol, but the symbol has no replacement defined. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9287?email_source=notifications&email_token=ADYTVZCBNCCLDUDIC5KJTE3PVGRTXA5CNFSM4GX5OG4KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBYOZORA#pullrequestreview-236820292>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADYTVZDWN2CFEZNWH32IPQTPVGRTXANCNFSM4GX5OG4A>.

LeonarddeR · 2019-05-13T19:13:40Z

@feerrenrut: I'm done reviewing this

feerrenrut

Thanks for your work on this!

Overall it looks good, just a couple of small things I would like to see changed.

nvdaes · 2019-05-25T16:15:54Z

@feerrenrut, I think all your requested changes have been addressed.
Anyway:

I prefer to use script decoration, as requested by you, but now I think this is the only command using this in globalCommands.
Seems the globalCommans file has other inconsistencies about readability, for example some variables are assigned without spaces between the variable and the value not being method parameters, just variables. Maybe addressed in a different PR, but we can leave this with the script decoration. I have imported script at the top of the file adding: from scriptHandler import script.

Thanks for considering this PR!

Put first: _getCurrentLanguageForTextInfo Then: script_review_currentSymbol This is consistent with _reportFormattingHelper and script_reportFormatting order, and more readable.

nvdaes · 2019-05-25T20:51:35Z

I have put the helper function before the script and I think I won't change anything else unless it's requested. I like the idea of the helper function, for readability and since this could be useful for add-ons like Emoticons, by Christianlm, making possible more flexibility to extend the functionality implemented in NVDA's core. Thanks a lot for this suggestion.

feerrenrut · 2019-05-27T09:53:04Z

but now I think this is the only command using this in globalCommands.

I think the best strategy to introduce changes in style like this is to do it incrementally, along with other changes that bring value. This helps to ensure the changes are tested properly.

feerrenrut

Thanks @nvdaes

nvdaes · 2019-05-27T12:28:58Z

Replying to @feerrenrut:

feerrenrut commented 3 hours ago

but now I think this is the only command using this in globalCommands.
I think the best strategy to introduce changes in style like this is to do it incrementally, along with other changes that bring value. This helps to ensure the changes are tested properly.

OK. Thanks for merging this.

LeonarddeR suggested changes Feb 19, 2019

View reviewed changes

Addressed requested changes

96837ba

Address more requested changes: don't use symbolProcessor, just curre…

fd28807

…nt language

LeonarddeR added the AfterPython3Transition label Apr 24, 2019

feerrenrut removed the AfterPython3Transition label May 8, 2019

feerrenrut requested review from LeonarddeR and feerrenrut May 9, 2019 16:58

LeonarddeR suggested changes May 9, 2019

View reviewed changes

Address requested changes by @LeonarddeR

85a0e3d

Merge branch 'master' into showExpandedSymbol

d1a97a8

LeonarddeR reviewed May 13, 2019

View reviewed changes

nvdaes added 2 commits May 13, 2019 20:27

Merge branch 'master' into showExpandedSymbol

e976c44

Address change requested by @LeonarddeR:

6b1bf1e

* When the symbol at the review cursor has no replacement, specify that there is no replacement for it, instead of mentioning that there is no symbol at this position.

LeonarddeR approved these changes May 13, 2019

View reviewed changes

feerrenrut suggested changes May 25, 2019

View reviewed changes

Comment thread source/globalCommands.py Outdated

Comment thread source/globalCommands.py Outdated

Comment thread source/globalCommands.py Outdated

nvdaes added 2 commits May 25, 2019 16:45

Merge branch 'master' into showExpandedSymbol

3d0789b

Addressed changes requested by @feerrenrut

495d70a

Cosmetics: put helper function before script

adcaf7d

Put first: _getCurrentLanguageForTextInfo Then: script_review_currentSymbol This is consistent with _reportFormattingHelper and script_reportFormatting order, and more readable.

feerrenrut approved these changes May 27, 2019

View reviewed changes

feerrenrut merged commit 41acae1 into nvaccess:master May 27, 2019

nvaccessAuto added this to the 2019.2 milestone May 27, 2019

feerrenrut added a commit that referenced this pull request May 27, 2019

Update changes file for PR #9287

bba3166

-			ui.browseableMessage("%s\n%s" % (text, expandedSymbol), _("Expanded symbol %s" % languageHandler.getLanguageDescription(curLanguage)))
+			ui.browseableMessage(_(
+				"Character: {character}\n"
+				"Replacement: {replacement}"
+			).format(character=text, replacement=expandedSymbol),
+			_("Expanded symbol ({language})").format(
+				language=languageHandler.getLanguageDescription(curLanguage)
+			))

Uh oh!

Conversation

nvdaes commented Feb 17, 2019 • edited by LeonarddeR Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing performed:

Known issues with pull request:

Change log entry:

Uh oh!

lukaszgo1 commented Feb 17, 2019

Uh oh!

nvdaes commented Feb 17, 2019

Uh oh!

LeonarddeR left a comment

Choose a reason for hiding this comment

Uh oh!

LeonarddeR commented Feb 19, 2019

Uh oh!

nvdaes commented Feb 19, 2019 via email

Uh oh!

nvdaes commented Feb 19, 2019

Uh oh!

nvdaes commented Feb 20, 2019

Uh oh!

LeonarddeR May 9, 2019

Choose a reason for hiding this comment

Uh oh!

LeonarddeR May 9, 2019

Choose a reason for hiding this comment

Uh oh!

LeonarddeR May 9, 2019

Choose a reason for hiding this comment

Uh oh!

LeonarddeR May 9, 2019

Choose a reason for hiding this comment

Uh oh!

LeonarddeR May 9, 2019

Choose a reason for hiding this comment

Uh oh!

nvdaes commented May 10, 2019

Uh oh!

dpy013 commented May 10, 2019

Uh oh!

nvdaes commented May 11, 2019 via email

Uh oh!

LeonarddeR May 13, 2019

Choose a reason for hiding this comment

Uh oh!

nvdaes commented May 13, 2019 via email

Uh oh!

LeonarddeR commented May 13, 2019

Uh oh!

feerrenrut left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nvdaes commented May 25, 2019

Uh oh!

nvdaes commented May 25, 2019

Uh oh!

feerrenrut commented May 27, 2019

Uh oh!

feerrenrut left a comment

Choose a reason for hiding this comment

Uh oh!

nvdaes commented May 27, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nvdaes commented Feb 17, 2019 •

edited by LeonarddeR

Loading