-
-
Notifications
You must be signed in to change notification settings - Fork 661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tab characters should be considered blank #13431
Comments
Accidently submitted before writing anything, editing it now... |
Edited it now with full context |
I realized one reason why tab is probably not in I experimented with adding tab to Because of this, I no longer believe the correct solution is to add tab to I think the correct solution could be to make it so, when an empty speech sequence is produced because none of the characters are read in the current symbol level, then the empty speech sequence should be changed to "blank" |
Experimenting with the idea that if text is empty after applying dictionary substitutions and symbol level, then it should be read as "blank", I achieved the desired behavior when I altered
With that experimental change, a line with just tabs is read as "blank" when symbol level is not All and as "tab tab" when symbol level is All. Also aline with other punctuation that is only read at a certain symbol level is also read as "blank" if symbol level is switched to something lower. There is one remaining somewhat unexpected behavior. If indentation reporting is on, the two tab line is read as "2 tab blank" or "beep blank", whereas when indentation reporting is off, it is read as "tab tab". Perhaps although slightly unintuitive, it might actually be the prefferential behavior. Either way, I think the changes I proposed with a little cleanup would improve over the current behavior. Please let me know what you think and I can then create a PR |
Hi @SamKacer |
cc @SamKacer |
@CyrilleB79 yes, glad to help out and provide the PR. When I next have the time, I will put it together |
closes nvaccess#13431 (yay! a palindrome)
closes nvaccess#13431 (yay! a palindrome)
closes nvaccess#13431 (yay! a palindrome)
This should be closed I think, as this would apparently break add-ons, as seen in #14102 |
@TheQuinbox - a way to avoid breaking backwards compatibility is described in that same comment |
@seanbudd, the main suggestion I took from that is a config option, and I fail to see how that would fix it, as add-ons would still possibly break for some people if they toggled the setting. A .1 release though, possibly? |
Hello, I don't think this should be closed just yet. Since, just because
that approach was potentionally addon breaking, doesnt mean this issue cant
be resolved with a different approach in a backwards compatible way, which
I think it can be in my PR after I make the requested changes.
Apologies, I have fallen off with work on this due to personal reasons. I
will try to get back to this soon, make the changes for my PR and take this
forward
…On Wed, Sep 7, 2022 at 1:46 AM Quin ***@***.***> wrote:
@seanbudd <https://github.com/seanbudd>, the main suggestion I took from
that is a config option, and I fail to see how that would fix it, as
add-ons would still possibly break for some people if they toggled the
setting. A .1 release though, possibly?
—
Reply to this email directly, view it on GitHub
<#13431 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AIOBHCKO7V6MNGAT4LZDYS3V47QWBANCNFSM5P7VZ7RA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
closes #13431 After processing a text for dictionary substitutions and symbol level, the result might be an empty string, which leads to nothing being spoken at all (not even "blank"). Something should always be spoken when reading text, since nothing being spoken gives the appearance of NVDA freezing. Description of how this pull request fixes the issue: When a speech sequence is submitted to speech.speak, if all strings of the sequence become empty due to symbol level and dictionary substitution processing, then the sequence is considered blank and "blank" is appended to it. The function now also takes an additional argument, suppressBlanks, which is false by default. This parameter allows callers to suppress the "blank" being appended to the sequence, even if the conditions are met. The main intention is to suppress this behavior during say all. Usages of speech.speak have been updated to specify this parameter when appropriate.
Reverted in #15032. |
Continuing the conversation from #15024: @SamKacer I am not so very opposed to a three-way configurable option for "
And I think, if NV Access wants this, you need a separate option for these same issues in regards to lines containing only whitespace (i.e. #13431), as the situations are not identical. "not so very opposed" doesn't mean I like it or recommend it, but I do recognize there are probably enough users who want it, at least for the whitespace case, that it may not be the worst thing ever provided it's configurable and not the default. The last I will probably say on this, is that personally I would much prefer you wait until we get a filter |
To clarify, the case of tab (i.e. is the subject of this particular issue) doesn't easily generalize to all types of symbols/punctuation. I think it is perfectly valid to treat tabs as blank, as a line containing only tabs is blank from a visual perspective. To summarize what I said earlier, speaking symbols like brackets, underscores, etc. as blank when they are the only symbols on a line is a no go for me, even when optional. A replacement like "no text" is longer but much better covers the actual situation and is therefore perfectly valid. Thus, it's only just the invalidity of the concept of blankness that bothers me. |
I am not in favor of adding one or more parameters in the GUI until we are confident that a significant group of users will use each option of these parameters. Else, the number of parameters in the GUI risks to grow indefinitely. For more specific use cases, I'd prefer a separate add-on. IMO by default, the lines with nothing or only blank characters (spaces, newline, tabs, nul char) should be reported as blank as they are in Narrator or in Jaws. Today "tab" is missing in the list of blank characters. Of course, care should be taken with the following situations:
I think that providing a PR with what I have just described (even with no configurable option at all) could be a first step to solve this issue. Then we will be able to figure if people (and how many) need something more advanced. This step-by-step approach would also avoid to revert the whole work whereas a part of the work is completely valid and agreed by all. |
Steps to reproduce:
Read the two lines between "start" and "end":
(Note: The first line after start has 4 spaces and the next line has 2 tabs)
Actual behavior:
They are read as:
Further, if indentation reporting is on, then they are read as:
Expected behavior:
I would expect the line with the 2 tabs should be read as "blank" when indentation reporting is turned off, since it is a blank line when indentation reporting is turned on. Nothing being read at all causes confusion when reading code, since it can seem as if NVDA has stopped responding.
System configuration
NVDA installed/portable/running from source: installed
NVDA version: 2021.3.3
Cause of problem
In
speech.py
, whether text is considered blank is determine by this code:The tab character is missing from
BLANK_CHUNK_CHARS
, which is why it isn't considered blank. The solution would be to also include it in the set.Not sure if there is something I am missing and this is actually the intended behavior, but the inconsistency between the line being blank when indentation reporting is on/off makes me think it is a bug.
If it is agreed this is a bug, I would be happy to provide the PR as my first contribution to the NVDA project.
The text was updated successfully, but these errors were encountered: