Generalise some code which is not specific to speech #228

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

2 participants

@nvaccessAuto

Reported by jteh on 2008-11-18 12:41
There is some code which is in the speech module or named for speech, even though it is not specific to speech and is in fact now used by braille.

Tasks:

  • Move processPositiveStates, processNegativeStates, REASON_, silent from speech to controlTypes, as these are also used by braille.
  • Rename speech*Labels to just *Labels in controlTypes, as these labels are full labels and are not associated with speech.
  • Provide labels for speech and braille based on controlTypes.*labels:
    • speech probably doesn't need its own labels.
    • Maybe it can just use controlTypes.*labels directly?
    • braille should use controlTypes.*labels as a base, but override to provide abbreviations.
@nvaccessAuto

Attachment 228-1.patch added by orcauser on 2011-06-17 18:00
Description:

@nvaccessAuto

Comment 1 by orcauser on 2011-06-17 18:02
Attached patch contains 3 commits:
1 and 2, deals with the above two tasks, and the third commit based on the previous two closes #1288.

The symbols to be used for #1288 are up for discussion, and some of them should probably not be translatable.
Thoughts?
Thanks.

@nvaccessAuto

Comment 2 by jteh on 2011-06-18 06:29
I haven't had a chance to look at this yet, but it'd be great if you could split the fix for #1288 into a separate patch depending on this one, which makes it somewhat easier to deal with. If you're not familiar enough with bzr to do this quickly, don't worry about it; it is easy enough for me to do myself.

@nvaccessAuto

Attachment 228-2.patch added by orcauser on 2011-06-18 07:40
Description:

@nvaccessAuto

Comment 3 by orcauser on 2011-06-18 07:47
Thanks Jamie,
Updated patch only contains the first 2 commits mensioned above.
Very familiar with the git workflow, but bzr is new to me, so still finding my feat. :)

@nvaccessAuto

Comment 4 by jteh on 2011-06-28 04:37
Thanks Mesar. This looks great.

This can't be merged before 2011.2 moves out of main, but needs to be merged ASAP after that happens to prevent major bit rot. Unfortunately, I suspect there is already some bit rot, as I think some changes were made very recently in processPositiveStates/processNegativeStates.

Just a couple of comments:

  • controlTypes.labels and braille.labels should be controlTypes.roleLabels and braille.roleLabels, respectively. Perhaps you were confused by a comment in my description, but this is what I actually meant.
  • I'm wondering whether we should rename ignoreRolesOnFocus and ignoreValuesForRoles, as "ignore" doesn't seem the right concept to me. We're talking more about whether something should be presented, rather than ignored in a functional sense. That said, I can't come up with a better name that isn't overly obscure. Mick, any ideas or are you happy with ignore? Changes: Milestone changed from None to 2011.3
@nvaccessAuto

Comment 5 by jteh on 2011-09-13 14:46
As noted in my last comment, there has already been some bit rot in this patch, so it isn't trivial to merge and doesn't provide additional functionality. Therefore, deferring this to the next release.
Changes:
Milestone changed from 2011.3 to 2012.1

@nvaccessAuto

Comment 6 by jteh on 2012-01-04 07:54
0f519bc
Changes:
State: closed

@nvaccessAuto nvaccessAuto added the task label Nov 10, 2015
@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