Ticket #228 (closed task: fixed)

Opened 3 years ago

Last modified 5 weeks ago

Generalise some code which is not specific to speech

Reported by: jteh Owned by: jteh
Priority: minor Milestone: 2012.1
Component: Core Version: development
Keywords: Cc:
Operating system: Blocked by:
Blocking:

Description

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.

Attachments

228-1.patch Download (79.6 KB) - added by orcauser 8 months ago.
228-2.patch Download (78.7 KB) - added by orcauser 8 months ago.

Change History

Changed 8 months ago by orcauser

comment:1 Changed 8 months ago by orcauser

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.

comment:2 Changed 8 months ago by jteh

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.

Changed 8 months ago by orcauser

comment:3 Changed 8 months ago by orcauser

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. :)

comment:4 Changed 7 months ago by jteh

  • Milestone set to 2011.3

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?

comment:5 Changed 5 months ago by jteh

  • Milestone changed from 2011.3 to 2012.1

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.

comment:6 Changed 5 weeks ago by jteh

  • Status changed from new to closed
  • Resolution set to fixed
Note: See TracTickets for help on using tickets.