Add option to not announce indentation change for blank lines to Document Formatting Settings

[SamKacer]

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

The problem is the one my IgnoreBlanksIndentationReporting addon aims to solve. It is described in the readme, but I will also rephrase it herefor full context.

Line indentation reporting is a very useful feature in NVDA, particularly for programmers since indentation is an important part of formatting code and in some languages like python it can even affect the meaning of the code. However, in its current form it can be too noisy by announcing indentation changes in contexts when it is not useful.

To explain the last point, consider this code fragment:

def foo():
	x = 42

	return x

def bar():

The current behaviour of NVDA is to report indentation changes for any line where the indentation has changed, even if the line is blank. So, the example would be read like:

def foo():
tab x = 42
no indent blank
tab return x
no indent blank
def bar():

The disadvantage for this behaviour is that for most programming languages, like python, a blank line has no semantic significance and is just used to visually separate lines of code with no change to the code's meaning. Therefore, by reporting the change of indentation upon entering a blank line and then reporting it again after landing on the next line is just noise that makes it harder to focus on understanding the code.

Describe the solution you'd like

To increase the signal to noise ratio in situations like the one described above, it would be good if settings would have a toggle like "Announce indentation changes on blank lines". By default it would be on, since that is the current behavior. When toggled off, it would result in the above code fragment read as:

def foo():
tab x = 42
blank
return x

no indent def bar():

In this case, indentation changes are announced 2 times instead of 4 and each time it is semantically significant to understanding the code, as opposed to the default behavior.

This behavior can be implemented quite simply and I would be happy to contribute a pull request. Inside speech.speech the getTextInfoSpeech function decides whether to announce an indentation change with this if:

if reportIndentation and speakTextInfoState and allIndentation!=speakTextInfoState.indentationCache:

The IgnoreBlanksIndentationAddon changes this behavior so it does not announce the indentation change for a blank line by altering it to this:

isNotBlank = any(isinstance(t, str) and not all(c in LINE_END_CHARS for c in t) for t in textWithFields)
if reportIndentation and speakTextInfoState and isNotBlank and allIndentation!=speakTextInfoState.indentationCache:

Note: LINE_END_Chars is defined as LINE_END_CHARS = { '\r', '\n' }

Since the proposed solution is to implement it as a native NVDA toggleable feature, it would differ from the addon implementation by also taking into account if the setting is toggled on/off, but otherwise it wouldn't require other changes to work.

Describe alternatives you've considered

The alternative is the aforementioned addon I developed. The major flaw with the addon implementation is that since the logic whether to announce indentation changes is buried in the getTextInfoSpeech function, to override this behavior, the addon must perform a monkey patch and replace getTextInfoSpeech with a copy where the only change is the aforementioned if statement.

The consequence of this is that any time there is any change in getTextInfoSpeech, no matter how minor or whether it is in any way related to indentation reporting, the addon has to be updated to also handle the new NVDA version, by defining a new version of the function with the one line change. The addon has to check the NVDA version and select the correct version of the monkey patched function. Supporting older versions of NVDA along with future releases will grow in complexity if the function is ever significantly restructured.

[cary-rowen]

Hi @SamKacer ,

I agree, I've been using the "Ignore blanks Indentation Reporting" add-on.

Thanks

[SamKacer]

I accidently submitted it too early before I wrote the whole proposal. I just edited the initial comment with the full context

@cary-rowen thanks :) I am glad you found the addon useful

[seanbudd]

Would an appropriate alternative be to make the API simpler here, allowing the add-on to be more stable, without needing to be updated as regularly with NVDA?

[cary-rowen]

Hi,

I still think this behavior should be implemented in NVDA itself, since indentation is almost meaningless for blank lines, What's more, this option is optional, and for those who really need it, they can configure it to their preferred reporting method.
@SamKacer 's add-on solved my annoyance, but more people would benefit if NVDA itself had such an optional configuration, and the cost of this change is small.
Thanks

[SamKacer]

@seanbudd part of the problem is that there isn't an API to begin with, as in a programming interface that could be consumed by an addon, they way script_handler and event handlers provide an interface for developing an addon. The addon works by overriding an internal function of NVDA that is not meant to be used programmatically by addons.

The function in question (getTextInfoSpeech) could perhaps be split up into smaller pieces and then the addon would have smaller surface which it would have to monkey patch, which wouldn't solve the maintainability issue with the addon, though it would certainly make the issue smaller than it is now.

However, what I think needs to be taken into account is that such a restructuring of getTextInfoSpeech would be a much bigger change than what is proposed here and would be much more complex and require a lot more regression testing and work to confirm the restructuring is correct.

To summarize some of my points (as well a point by @cary-rowen):

• proposed implementation is small and simple, restructuring getTextInfoSpeech is larger and more complex
• splitting up getTextInfoSpeech would make maintenence issue smaller, but same in nature
• the feature could have wider appeal for the NVDA community
• current behavior would be by default, feature easy to toggle off/on

Perhaps though I just can't see a way speech.py could be refactored to make the addon a better alternative over incorporating the feature into NVDA. So, if anyone could propose anything concrete then it would help me understand if it would be the better solution.

Finally, perhaps the feature can be made as an addon without monkey patching. If someone would know of a way to do it in a way that eliminates it, then that would be a suitable alternative solution.

[brunoprietog]

Yes, this would be a cool feature. I also use that add-on to shovel that annoying problem a bit.

But I for one would like the ad to be different. If The following line is blank, and the following line has no indentation, NVDA should report the indentation change on the blank line, not the subsequent line. Something like this:

def foo():
tab x = 42
blank
return x
no indent blank
def bar():

This is because it is confusing when navigating with up and down arrows repeatedly between these lines. The indentation change is not consistently announced when going up and down.

[SamKacer]

@brunoprietog thanks for your input. I have been thinking it over and I see several problems with your request.

Firstly, I'll just try to formalize what I understand you meant by lack of consistency. I understand it as, if indentation reporting is consistent, then if indentation change is announced on line 2 reading down, then when reading up, indentation change should be announced on line 2 as well.

This inconsistency is currently inherent to indentation reporting in NVDA and isn't introduced by the proposed feature. For example, consider this snippet:

	line 1
	line 2

line 4

With normal indentation reporting, when reading down, this is read as:

tab line 1
line 2
no indent blank
line 4

When reading up, it is:

line 4
blank
tab line 2
line 1

Reading down, the change is announced on line 3, but reading up it is announced on line 2. So, this is not consistent either and inconsistency is currently part of indentation reporting in NVDA. Thus, I consider it a seperate topic from this feature proposal, because if such a change would be introduce in the proposed alternative reporting mode, it would need to be implemented for default reporting as well, so that the two modes are consistent in this regard.

Because of this, if you are interested in such a change to indentation reporting in NVDA, I think a seperate issue should be created to discuss it. I could leave it here, but since I've been thinking this over so much, I will also expand on other problems that I thought of with that request.

Firstly, there would still be inconsistency in reporting, Taking the same code snippet, if we applied the proposed rule, that if reading a blank line, and the next line also doesn't have indentation, then no indent should be announced, reading down it would be read as:

tab line 1
line 2
no indent blank
line 4

Reading up, it would be as:

line 4
blank
tab line 2
line 1

So, down it is line 3, and up it is line 2, still being inconsistent.

You could have consistency if you changed the rule to be: when reading a blank line, if next line has different indentation, then announce indentation change. Then when both reading the snippet down or up, indentation change is announced on line 3. Essentially, instead of the rule being a special case for no indentation, it would apply generally to any changes in indentation.

However, with this rule you have a different problem. To explain, consider this snippet:

def foo():
	if cond:
		bar()

	baz()

Applying the revised rule, it would be read as:

def foo():
tab if cond:
2 tab bar()
tab blank
baz()

The issue is that it is giving misleading information, since it is announcing there is a tab on the blank line, whereas it is actually a completely blank line. Furthermore, reading it in the other direction, the blank line would read as "2 tab blank", which would be inconsistent with how it is read down, although it would be consistent in the location when the indentation change is announced. So, on second thought, I can't even think of a rule right now that would give total consistency.

Finally, even if there was a rule that would give total consistency, the other issue is the practicality of implementing this. Currently, in the context where indentation change announcements are decided (see speech.speech.getTextInfoSpeech), only the current line being read is in scope. To implement the rule or any of the revisions, the next line would also need to be in scope. And even that wouldn't be enough. To explain, consider this snippet:

	line 1


	line 4

If only the next line was considered when applying the rule, then it would be read as:

tab line 1
no indent blank
blank
tab line 4

So, it would be back to the same problem my proposed feature aims to solve. Therefore, not only the next line would need to be in scope, but the entire rest of the document.

This would require much larger changes in the code, further increase complexity in an already complex function, and would likely have major implications on performance, making reading have noticeably higher latency when indentation reporting is turned on.

For these reasons, I don't think striving for such consistency in indentation reporting is a good idea, although I can certainly understand where you are coming from and do sympathize. The inconsistency in how indentation is announced when reading down/up may be unintuitive at first, but the rule is fairly simple and is understood through practise in a short amount of time.

As I wrote earlier in this comment, I do think this is a topic seperate from the feature proposed in this issue, so further discussion would probably be best continued in a seperate github issue.

[SamKacer]

I noticed there are no tests for indentation reporting. I double checked by deliberately breaking indentation reporting and all tests still passed. So I just wanted to ask, what is the expected level of testing for this feature? Should I implement some system tests?

I looked at doing some unit tests since those run much faster than system tests. But there aren't any existing tests for getTextInfoSpeech and there is a lot of state to reproduce to be able to test it.

[seanbudd]

System tests for this in notepad would be appreciated