Skip to content

Adds new German braille tables#11268

Merged
feerrenrut merged 8 commits into
nvaccess:masterfrom
AAClause:missingBrailleTables
Jul 6, 2020
Merged

Adds new German braille tables#11268
feerrenrut merged 8 commits into
nvaccess:masterfrom
AAClause:missingBrailleTables

Conversation

@AAClause

@AAClause AAClause commented Jun 17, 2020

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #11263

Summary of the issue:

New German braille tables are missing in the GUI.

Description of how this pull request fixes the issue:

Adds entries for "de-g0-bidi.utb" and "de-g1-bidi.ctb".

Testing performed:

Ran from sources. Loaded all these tables.

Known issues with pull request:

None

Change log entry:

@AAClause

Copy link
Copy Markdown
Contributor Author

@Futyn-Maker I changed the label for "uk.utb" from "Ukrainian" to "Ukrainian Grade 1". Is it OK for you?

@Futyn-Maker

Copy link
Copy Markdown

@Andre9642 Yes, I think it's OK, although in Russia and Ukraine we prefer "literary braille", but it's not important.

@Adriani90

Copy link
Copy Markdown
Collaborator

I am adding the author of that tables here, @BueVest should these tables favorized? Should DE-G0.utb and DE-G1.ctb be excluded from screen reader in favor of DE-G0-Bidi.utb and DE-G1-Bidi.ctb? I still see a comment in the newer tables that this is still experimental and should not be included in the core files yet.
Also there is still a comment in de-g0-bidi-core.uti which says that a certain line should be removed as soon as liblouis/liblouis#717
is solved. That issue is now solved, so are these german tables now complete?

@feerrenrut feerrenrut requested a review from LeonarddeR June 18, 2020 12:16

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from #11263 (comment)

I'm a bit confused about those new German tables. How experimental are they really?

note that we once choose to replace en-us-comp8.ctb by en-us-comp8-ext.utb because the latter was more complete and extended on the former, so it wouldn't do any harm when it was replaced.

What are the intentions of these German tables? Are they meant to replace the old tables? Are their file names persistent, i.e. meant to stay even when the tables are no longer experimental?

@LeonarddeR

Copy link
Copy Markdown
Collaborator

Additional details in #11263

As we are very close to 2020.2, I think I prefer the german tables to be left out of this.

@feerrenrut

Copy link
Copy Markdown
Contributor

@LeonarddeR What about the added Ukrainian table, would it be safe to add for 2020.2?

@Futyn-Maker

Copy link
Copy Markdown

@feerrenrut There isn't any oficial standards of Ukrainian computer braille, so, we've based in on Russian computer braille set with some changings of punctuations. I believe that it's better than nothing...

@LeonarddeR

Copy link
Copy Markdown
Collaborator

I"m ok with the Ukrainian ones.

There's still some debate going on about the German tables, see liblouis/liblouis#911. @BueVest owns them.

@AAClause AAClause force-pushed the missingBrailleTables branch from 6085aff to 772f61c Compare June 18, 2020 17:48
@BueVest

BueVest commented Jun 18, 2020 via email

Copy link
Copy Markdown
Contributor

@Adriani90

Copy link
Copy Markdown
Collaborator

@BueVest so do you see the bidirectional G0 and G1 tables as stable enough to be included in NVDA? Or would you advice to wait until the work on G0 and G1 tables is finished? To me they looked prety stable but you might have tested them already also in comparison with the forward translation G0 and G1 tables.

@feerrenrut

Copy link
Copy Markdown
Contributor

Given the amount of discussion here about the German braille tables, I think it is best to open a new PR just for the Ukrainian tables, and allow the discussion to continue here. The new PR (for the Ukrainian tables) should target the beta branch please.

@Futyn-Maker

Copy link
Copy Markdown

@feerrenrut What problems have You found about Ukrainian tables? The new Ukrainian computer braille table based on Russian computer braille code and now works absolutely correct. Personally I don't see any sense to continue discussions about Ukrainian tables...

Andrey

@feerrenrut

Copy link
Copy Markdown
Contributor

Not discussion, but if they are to be included in 2020.2 then a new PR must be created. This PR is mostly talking about the German tables, it might as well stay that way.

Currently the changes in this PR are for the Ukrainian tables, but the description and discussion is about Germain tables.

@BueVest

BueVest commented Jun 19, 2020 via email

Copy link
Copy Markdown
Contributor

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR doesn't include the German tables, @Andre9642 could you please re-introduce those?

@AAClause AAClause force-pushed the missingBrailleTables branch from 772f61c to 18839ed Compare June 19, 2020 16:30
@AAClause AAClause changed the title Adds missing braille tables Adds new German braille tables Jun 19, 2020
@BueVest

BueVest commented Jun 19, 2020

Copy link
Copy Markdown
Contributor

Perhaps, you should also disable the old german tables as input tables. If you check the metadata of those tables, it says "direction: forward", meaning that the tables should only be used for forward translation and not back-translation, i.e. Braille input.
'Just a suggestion.

@BueVest

BueVest commented Jun 19, 2020

Copy link
Copy Markdown
Contributor

Sorry, of course, this only applies to the grade 0, 1 and 2 tables (de-g0.utb, de-g1.ctb and de-g2.ctb), not the 6 and 8 dots computer Braille tables.

@feerrenrut feerrenrut requested a review from LeonarddeR June 23, 2020 12:15

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this looks ok (thanks @Andre9642!) but I would like to get the opinion of @LeonarddeR since he is much more familiar with braille and libluis.

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @BueVest that it makes sense to disable input for the old German tables if it's really that broken as he describes. See below.

I see why we want these tables integrated and that there's real demand for it, but I'm still a bit worried about the naming, (improved back-translation, experimental). Looking at it from a user perspective:

  1. Does a user understand what's meant with back-translation? May be input fits better here, as they are already familiar with that.
  2. If only back translation was improved, I'd say let's kick out the old German tables and replace those by these. However, it seems that forward translation is also changed with regard to accented letters, according to liblouis/liblouis#911 (comment). I really dig the idea to call these tables detailed as introduced by @bertfrees in liblouis/liblouis#911 (comment)

Altogether, the discussion in liblouis/liblouis#911 really makes me believe that many things about these tables are not set in stone. If we had custom braille tables, I'd probably be against having them in core like this.

I am aware of the fact that my review isn't at all helpful in deciding what to do with this pr. May be @michaelDCurran can also spread his light on this?

Comment thread source/brailleTables.py Outdated
Comment thread source/brailleTables.py Outdated
Comment thread source/brailleTables.py Outdated
@lukaszgo1

Copy link
Copy Markdown
Contributor

I believe in the past we were adding new braille tables into the GUI only if someone has requested them to be added - as far as I see there was no such request in this case.

@Adriani90

Copy link
Copy Markdown
Collaborator

@LeonarddeR the changes in forward translation are related to tilde, dash and some other symbols which in the old german table cause some interpolations with other characters. It seems this tables are improving it and even though it is experimental, I would say there is nothing in there which would break the current user experience. Imo the only way to make a braille table really robust is extensive user testing, different braille displays etc. Thus, I vote for these tables to be included into NVDA, at least in the alpha version for a while. If it's not working well for users, then they could be replaced back by the old tables. But since @BueVest is the creator of these tables, I think he tested them of course and his statement here should have enough weight.

Regarding NVDA's Gui, I would vote for introducing a new combo box in the braille settings called "bidirectional tables" and include there all stable braille tables which allow both input and output. If the meaning of bidirectional tables is documented properly in the user guide, I don't see any significant confusion caused by this.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

I would vote for introducing a new combo box in the braille settings called "bidirectional tables" and include there all stable braille tables which allow both input and output.

How would this fit in the UX? I'm afraid this would add even more confusion.

I believe in the past we were adding new braille tables into the GUI only if someone has requested them to be added - as far as I see there was no such request in this case.

I think this applies to most cases, yes.

@Adriani90

Adriani90 commented Jun 23, 2020 via email

Copy link
Copy Markdown
Collaborator

AAClause and others added 3 commits June 23, 2020 19:52
Co-authored-by: Leonard de Ruijter <leonardder@users.noreply.github.com>
Co-authored-by: Leonard de Ruijter <leonardder@users.noreply.github.com>
Co-authored-by: Leonard de Ruijter <leonardder@users.noreply.github.com>
@Adriani90

Copy link
Copy Markdown
Collaborator

@BueVest to which extent does solve your tables the following issues?
liblouis/liblouis#363
and
liblouis/liblouis#597

@BueVest

BueVest commented Jun 23, 2020 via email

Copy link
Copy Markdown
Contributor

@BueVest

BueVest commented Jun 23, 2020 via email

Copy link
Copy Markdown
Contributor

@bertfrees

Copy link
Copy Markdown

Chiming in... Bue, I don't think Leonard was talking about file names. Files names are clearly also an issue, but a unrelated one.

For the user interface, I recommend using as much as possible what is in the "display-name" (and/or "index-name") metadata of the Liblouis tables. This field was created specifically to be used in applications like NVDA, and to create some consistency across applications. I can't stress enough how much I would appreciate it if you would use our metadata instead of your own names.

But anyway, I'm digressing...

The naming of the German tables in question, and the corresponding Danish tables that Bue mentioned, is indeed an issue that we are aware of. Their names should indeed be harmonized, and the concerns that Leonard has about the "back-translatable" are right. I think the most likely scenario at this point is that they'll be labelled "detailed" in the next version of Liblouis.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

I can't stress enough how much I would appreciate it if you would use our metadata instead of your own names.

I see why that's preferred. Doing this automatically is a bit difficult though, as we want the table descriptions to be translatable.

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, let's crack the nut. I'm ok with this if the following two changes could be applied with regard to naming, if @BueVest also agrees.

Comment thread source/brailleTables.py Outdated
Comment thread source/brailleTables.py Outdated
@bertfrees

Copy link
Copy Markdown

Doing this automatically is a bit difficult though, as we want the table descriptions to be translatable.

Yes, I suspected that was the case. It doesn't need to be automatic though.

@Adriani90

Copy link
Copy Markdown
Collaborator

@BueVest as regarding your thoughts about grade 2 tables, they are out of scope for this PR. So you could intermediarily edit the current grade 2 table to solve it, but I would say it's not a high priority. If the bidirectional grade 2 table is created, they this might be considered.

@Adriani90

Copy link
Copy Markdown
Collaborator

One further thing that I think it is important to discuss, if this table is bidirectional but it is displayed under output tables in braille settings, this could create confusions.
@LeonarddeR do you have a suggestion how this could be solved? I mean if an additional combo box for bidirectional tables is not a solution here? There should be at least the possibility to choose "no input braille table" in the input braille table combo box in NVDA's braille settings. Otherwise the backward translation from bidirectional braille table and from a chosen input braille table could interfere here which could have some issues. Or am I wrong?

feerrenrut and others added 2 commits July 6, 2020 12:10
See review actions nvaccess#11268 (review)

Co-authored-by: Leonard de Ruijter <leonardder@users.noreply.github.com>
See review actions nvaccess#11268 (review)

Co-authored-by: Leonard de Ruijter <leonardder@users.noreply.github.com>
@feerrenrut

Copy link
Copy Markdown
Contributor

I haven't seen any opposition to the suggestions made by @LeonarddeR, so I will accept them and merge this PR.

@feerrenrut feerrenrut dismissed LeonarddeR’s stale review July 6, 2020 10:48

Changes have been made.

@feerrenrut feerrenrut added component/braille component/liblouis Issues related to liblouis, such as liblouis updates and braille table additions/changes labels Jul 6, 2020
@feerrenrut feerrenrut merged commit a426c5a into nvaccess:master Jul 6, 2020
@nvaccessAuto nvaccessAuto added this to the 2020.3 milestone Jul 6, 2020
@Adriani90

Copy link
Copy Markdown
Collaborator

@BueVest does it make sense to keep the older german output tables for Basisschrift (g0I and Vollschrift (g1)? Or should these be also removed? I am asking because the coresponding input tables are also replaced now by your bidirectional tables but the old output tables are still in place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/braille component/liblouis Issues related to liblouis, such as liblouis updates and braille table additions/changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing braille tables in the Braille NVDA Settings

9 participants