Concise speaking of indentation #373

Closed
nvaccessAuto opened this Issue Jan 1, 2010 · 25 comments

2 participants

@nvaccessAuto

Reported by jteh on 2009-07-24 05:15
Currently, NVDA only reads up to five tabs at the start of a line, even if there are more. Also, spaces are never read in text, which, although normally not desirable, is essential at the start of lines when working with code indented with spaces. NVDA should be able to optionally announce the number of tabs and spaces by which text is indented.
Blocked by #845

@nvaccessAuto

Comment 2 by jteh on 2009-07-24 06:23
Bzr branch: http://bzr.nvaccess.org/nvda/repeatedAndIndentation/

@nvaccessAuto

Comment 3 by jteh on 2009-12-08 06:04
The code in this branch works and is up to date. However, speaking of indentation only works if speak punctuation is enabled and it is not currently possible to disable speaking of indentation.

@nvaccessAuto

Comment 4 by jteh on 2009-12-08 06:05
Btw, the code that does this stuff is in speech._processSymbol().

@nvaccessAuto

Comment by jteh on 2011-04-14 03:19
(In #43) This is already fixed in the new code for #332. On further reflection, I think indentation needs to be handled separately.

@nvaccessAuto

Comment 6 by orcauser on 2011-05-02 18:43
Please find attached patch.
I wasn't quite sure if speakTextInfo is where you wanted the processing to happen, Hope it is reasonable.

Thank you.

@nvaccessAuto

Comment 7 by jteh on 2011-05-03 06:24
Thanks for the code. Very nice work.

There are a few changes I think need to be made (and I'm happy to do this when I get some time):

  • I don't think the calculation algorithm should hard-code spaces and tabs. This makes it hard to add other types of whitespace later (although admittedly, there will probably never be more characters than this). It also means that the user's labels for these symbols won't be used. I propose using the symbol data to retrieve the character name. The disadvantage to doing what I suggest is that it won't be able to speak plurals for space and tab, but nor can our symbol repetition code, and I don't see this as a major problem.
  • Calculating indentation could be done much faster with a regular expression, similar to the way we handle repeated symbols.
  • I haven't tested the code yet, but unless I'm missing something, it doesn't strip the actual indentation characters from the text. This means if you have symbol level set to all, you will hear, for example, "3 tabs tab tab tab blah".
  • It should probably be restricted to unit=UNIT_LINE. Currently, it could be triggered when moving by word.
@nvaccessAuto

Comment 8 by jteh on 2011-05-03 06:27
Changes:
Milestone changed from None to near-term

@nvaccessAuto

Attachment indentation.patch added by orcauser on 2011-05-04 19:24
Description:

@nvaccessAuto

Comment 9 by orcauser on 2011-05-04 19:27
Thanks for your corrections.

Please see updated patch, is this the sort of thing you were thinking of?

@nvaccessAuto

Comment 10 by jteh on 2011-05-05 08:58
I've made further changes at http://bzr.nvaccess.org/nvda/indentation/ . Please do provide feedback if you have any.

Personally, I'd prefer if NVDA didn't say "no indent" unless the indentation actually changes. Right now, whenever you focus a document, you'll hear it, which is rather annoying. I think it's fair to assume that there is no indentation unless otherwise specified or there was a change. Mesar, Mick, your thoughts?

This is a bit tricky to implement, as checking for the translated "no indent" string is a bit ugly. calculateTextIndentation() would have to return "" or similar to indicate no indent and then speakTextInfo() would convert this to the "no indent" message.

@nvaccessAuto

Comment 11 by jteh on 2011-05-05 09:02
Another thing: if you have only whitespace on a line, this gets handled by the indentation code. I'm tempted to leave this as is, as this can be important for auto indentation, etc. However, one problem this causes is that in consoles, you always get "80 space" for blank lines. I'm not sure of the best solution for this.

@nvaccessAuto

Comment 12 by orcauser on 2011-05-05 18:53
I think reporting the indentation when first gaining focus is not a bad thing.
I often do small patches here and there :) and often have various windows with code open, to enable me to cross reference and to quickly gain an understanding.

In Orca one can assign a shortcut to toggle reporting of indentation,
this helps when speed reading code before you get to the place which you are intrested in.

Having the shortcut allows for flexibility with the terminal problem, since the user can toggle without bringing up the dialogue, maybe this is the compromize we can settle on.

@nvaccessAuto

Comment 13 by jteh (in reply to comment 12) on 2011-05-05 21:32
Replying to orcauser:

I think reporting the indentation when first gaining focus is not a bad thing.

We'd still report it if there was anything other than no indentation. Right now, I hear "no indent" every time I land in an editable text field, which seems rather superfluous.

In Orca one can assign a shortcut to toggle reporting of indentation,

Yeah, that might be a good idea.

Having the shortcut allows for flexibility with the terminal problem, since the user can toggle without bringing up the dialogue, maybe this is the compromize we can settle on.

True. I'd like to solve this one another way if we can, so I'll see what I can come up with.

@nvaccessAuto

Comment 14 by jteh on 2011-06-09 03:28
Changes:
Milestone changed from near-term to 2011.3

@nvaccessAuto

Comment 15 by jteh on 2011-06-28 04:46
To do: report indentation at the start of report formatting (NVDA+f).

Mick, I'm sure we discussed something else to do here too. Can you remember what it was? :(

@nvaccessAuto

Comment 16 by jteh on 2011-09-05 03:35
Needs to wait until after autoLanguageSwitching is merged, as that branch makes significant changes to speakTextInfo.

@nvaccessAuto

Comment 17 by mdcurran on 2011-10-11 01:28
Changes:
Milestone changed from 2011.3 to 2012.1

@nvaccessAuto

Comment 18 by jteh on 2011-10-25 03:11
I've had to reimplement much of this due to changes in speakTextInfo.

I'm not really sure of the best way to handle interspersed control and format fields. The original implementation didn't really handle this at all and also had a few other bugs I didn't spot earlier. My new implementation just gathers all indentation, regardless of interspersed fields, and then speaks it before any content. This means that if you have a link beginning with two spaces at the start of the line, you won't know whether the spaces were inside the link; you'll hear "2 space link blah". Similarly, if you have one space which is underlined followed by one space which is not, you won't know where the underline occurs; you will hear "2 space underline no underline blah". I don't know whether this is ideal or not.

I also need to add code to make sure indentation is spoken in the default language. Currently, it will be spoken in the language specified by the initial format field, which is bad since it uses user interface strings.

@nvaccessAuto

Comment 19 by orcauser (in reply to comment 18) on 2011-10-25 06:50
Replying to jteh:

I'm not really sure of the best way to handle interspersed control and

format fields. The original implementation didn't really handle this

at all and also had a few other bugs I didn't spot earlier.

Yeah, sorry, the request was due to indentation in a simple text editor,
but if something is worth doing it is worth doing right, so thanks for
your percipience with this one.

My new

implementation just gathers all indentation, regardless of

interspersed fields, and then speaks it before any content. This means

that if you have a link beginning with two spaces at the start of the

line, you won't know whether the spaces were inside the link; you'll

hear "2 space link blah".

Idealy we should hear:
space link space blah

Similarly, if you have one space which is

underlined followed by one space which is not, you won't know where

the underline occurs; you will hear "2 space underline no underline

blah".

underlined space, no underline space bla.

This feature will be mostly used by technical writers/programmers, and this level of info would be very useful.

thank you.

@nvaccessAuto

Comment 20 by jteh (in reply to comment 19) on 2011-10-25 07:01
Replying to orcauser:

Yeah, sorry, the request was due to indentation in a simple text editor,

but if something is worth doing it is worth doing right, so thanks for

your percipience with this one.

You gave it a good shot and did very well. Thanks for trying and getting this started.

Idealy we should hear:

space link space blah

underlined space, no underline space bla.

This is what Mick and I came up with yesterday. However, it occurred to me: what if the next line has the same level of real indentation but has different fields? In this example, the next line could have two spaces but no underline. Should that be treated as a new level of indentation or not?

@nvaccessAuto

Comment 21 by jteh on 2011-10-25 07:02
Reporting indentation interspersed with control fields is also a nightmare when you want to query indentation; e.g. with NVDA+f. That normally doesn't report links, etc., so it's hard to know what to present there if we go down this path.

@nvaccessAuto

Comment 22 by orcauser (in reply to comment 21) on 2011-10-25 07:25
Replying to jteh:

Idealy we should hear:

space link space blah

underlined space, no underline space bla.

This is what Mick and I came up with yesterday. However, it occurred

to me: what if the next line has the same level of real indentation

but has different fields? In this example, the next line could have

two spaces but no underline. Should that be treated as a new level of

indentation or not?

Unfortunately, I think it should be treated as different, the meta info
is changing so they are not the same.

Replying to jteh:

Reporting indentation interspersed with control fields is also a

nightmare when you want to query indentation; e.g. with NVDA+f. That

normally doesn't report links, etc., so it's hard to know what to

present there if we go down this path.

Is there an alternative path?

Coming back to the link example:
space link space blah

It isnt pleasant, but what if they simply heard space, space.
They know if it was two contiguous spaces they would hear 2 spaces, they
now know something is different about it and therefore can review the
line and get space link space blah.

Thanks.

@nvaccessAuto

Comment 23 by jteh on 2011-10-31 08:41
Interspersing indentation with fields is very difficult and I'm not sure there are really many test cases where it's particularly useful. Unless I'm missing a particular use case, I think it will primarily be used in simple text editors. Therefore, I'm going to merge this as is so at least the basic functionality is there. It can always be enhanced later.

@nvaccessAuto

Comment 24 by orcauser on 2011-10-31 08:44
Agree, sounds good,
thanks.

@nvaccessAuto

Comment 25 by jteh on 2011-10-31 21:15
Merged in a0a1f78.
Changes:
State: closed

@jcsteh jcsteh was assigned by nvaccessAuto Nov 10, 2015
@nvaccessAuto nvaccessAuto added this to the 2012.1 milestone Nov 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment