New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom braille tables #3304
Comments
Comment 1 by tyrylu on 2014-03-19 17:15 |
Comment 4 by jteh on 2015-03-25 05:03 I think this is add-on territory. However, as @dkager noted in ticket:4996#comment:1, we'll probably need to tweak the core slightly to make this possible from an add-on. I think two tweaks would be needed:
|
Comment 5 by dkager on 2015-03-25 11:15 I'm assuming this specific solution doesn't require its own ticket. Please correct if wrong. |
Comment 6 by dkager on 2015-03-26 15:24 Replying to jteh:
This is easy enough, but while looking at the code I came up with some additional changes to make appending custom braille tables more convenient:
Replying to jteh:
This one is a bit more interesting. My suggestion for finding custom tables is:
Currently the list of translation tables to use with louis.translate is built in braille.Region.update. At first glance this seems highly inefficient. It would only get worse if we started statting the filesystem when building the list. My initial suggestion would be to add a class attribute braille.Region._translationTables that is initialized once and only updated if the user changes the translation table in the GUI. For symmetry, brailleInput should be updated in a similar fashion. Thoughts? I'd like to implement this but I want to do it right. |
Comment 7 by leonarddr (in reply to comment 6) on 2015-03-26 21:41
I tend to disagree. When pressing the OK button and a table couldn't be found, an error message should be displayed similar to the braille display error message. Loading a default table puts you on the wrong track. |
Comment 8 by dkager (in reply to comment 7) on 2015-03-27 07:53
Thanks. That would be even better of course. And it should probably be done in more places (such as typing "hello" in the capital pitch change edit field silently converting the string to 0). |
Comment 9 by nvdakor on 2015-03-27 15:41 |
Comment 12 by dkager on 2015-03-29 12:37 This patch works, but I would really like to factor all of the table code out into a new module brailleTableHandler. Initial responsibilities of this module:
Some interesting future use cases for brailleTableHandler:
Another change I would like to bring up for discussion is to provide a standardized place for add-ons to store custom tables. The rationale behind this is that distinguishing tables based on absolute versus relative paths isn't intuitive. For example, it seems convenient to write the following lines to append a custom table:
This won't work because configPath doesn't have to be absolute. You can add os.path.abspath, but that isn't directly apparent. Nor can you write paths like Of course there are other ways to address the issue. Having implemented a basic fix I can't say I'm satisfied with the results. Maybe @jteh could shed some light on what is possible/allowed/desirable. One major consequence of moving the table definitions into their own file is that existing patches will likely break. On the other hand, such a change would improve maintainability even if support for custom tables isn't added. |
Comment 13 by jteh (in reply to comment 12) on 2015-03-30 02:28 Replying to dkager:
I agree we need more advanced table handling, but I think it can just go in the braille module, unless you have strong reasons to split it into a separate module.
Makes sense.
Makes sense, though this will probably only be called when caching.
Ideally, this wouldn't be necessary for in-built tables. We should probably make the build system verify that all referenced in-built tables exist and fail the build if they don't. However, it's definitely necessary for add-on tables.
Makes sense.
A while ago, I was thinking that language information should be stored in the tables data structure, which would require refactoring it somewhat. This way, you don't have to define information for a table in multiple places. However, because a table can actually support multiple regions, perhaps it makes more sense to have a separate language map as you suggest. Hmm.
I do agree there needs to be an easy way for add-ons to do this, but:
I'm not keen on this idea because it means add-ons will be placing data (other than configuration) outside their add-on directory. That makes them much harder to remove and means data could be left behind. Note that it's already possible for an add-on to get its directory from its own code:
Which is something that has bitten us in the past and something we probably should fix, but that's a separate issue. |
Comment 14 by jteh (in reply to comment 6) on 2015-03-30 02:35
Makes sense.
It's not really an issue now, but as you say, any increased complexity requires caching.
I'd probably prefer it was an instance variable on braille.BrailleHandler, primarily because that's also where the active display is set.
Backwards incompatible changes are never ideal, but I think this one is necessary for performance if we implement custom tables, etc. You should make sure to refresh the cache in braille.BrailleHandler.handleConfigProfileSwitch. |
Comment 15 by jteh (in reply to comment 7) on 2015-03-30 02:37
That's fine, but there still needs to be a fall back to a default table if, for example, NVDA is started with a missing table. We don't want a user to completely lose braille in that case. |
Comment 16 by dkager (in reply to comment 13) on 2015-03-30 19:08
Based on your later suggestion in comment:14 to put table handling in braille.BrailleHandler:
I'd say better safe than sorry, i.e. have a fallback table for all situations. Should make the logic slightly less complex too.
The liblouis table filenames are mostly uniform so could in theory be used to identify most table languages and regions. Still, adding this explicitly to the table definitions sounds a whole lot safer. A stand-alone language-to-table map shouldn't be necessary, you can look up the appropriate table for a given key anyway. Something for another ticket I think.
Yeah, you could either have a single function to:
|
Comment 17 by dkager on 2015-05-18 14:46
Long story short, the fix for allowing absolute paths for braille table filenames is easy enough, but it's a bit harder to find a good place for caching the currently active tables. You can't just read this from the config dictionary because this doesn't include absolute paths for stock tables, so it takes some extra computing. |
Comment 18 by dkager on 2015-06-14 11:10 |
Comment 19 by dkager on 2015-07-23 19:18
All should be pretty straightforward and, if acceptable, should be all the custom braille table support NVDA needs IMO. Branch: t3304 |
Comment 21 by leonarddr (in reply to comment 19) on 2015-07-24 07:47
I agree that this avoids situations as in #4986, however it makes it really harder to discover whether a table doesn't exist. NVDA might switch to US braille without the user noticing it, making it almost impossible to track down bugs like #4986. |
Comment 22 by dkager (in reply to comment 21) on 2015-07-24 07:56
In this case a warning will also be written to the logs (or even an error, I forget). This then sends an audible alert as well. Also, if you're expecting Danish braille but you get US 8-dot computer braille you'll definitely notice the difference. I tried. :) |
Comment 23 by leonarddr (in reply to comment 22) on 2015-07-24 08:19
The audible alerts are suppressed in release versions, so that makes a little difference. |
Comment 24 by dkager (in reply to comment 23) on 2015-07-24 08:27
We could pop up an error too. Not sure what the policy is for that. @jteh? |
Also #5489. |
@Adriani90, yes indeed.
So far, this new approach seems quite promising.
Existing tables (same file name) are overridden, even when indirectly included. |
The original plan was to allow absolute paths as table filenames, thereby allowing addons to specify their own table location. But I think that got lost a long time ago. |
This implementation allows for this, too. I also added the missing fall-back mechanism on translation tables (as found on the input tables) for cases where the configuration points to a missing table - eg. when disabling or removing addons - but extended it to apply also on profile switching. The only thing is that the current automatic translation of older tables names to newer tables names as of a liblouis update can lead to unexpected behavior if there is collision, but I do not think I am the one to take the decision when to drop this backward-compatibility piece. |
@JulienCochuyt: Is your work somehow based on the work by @dkager which he did earlier? |
Nope. The comments in PR #5489 helped me find my way to understand liblouis process, but this is original work based on a custom table resolver. |
@JulienCochuyt: When is this expected to arrive in a pr? We are working on a new Dutch braille standard with the Dutch Braille Authority and we'd be pretty interested in having an ability for users to test braille tables the easy way. |
@LeonarddeR
@andre9642 is working on something like that in Braille extender.
Why to duplicate pr’s?
|
Whenever you like, thanks for your interest.
Because: |
@zstanecic it's not a problem, I don’t have exclusivity on this feature. ☺ As @JulienCochuyt said, there is more advantages if this feature is directly into NVDA Core. Also it's much easier to integrate it like this. In any case, I look forward to testing that! |
I haven’t said that it is an exclusivity.
I’ve said, that something can be improved from your code and used for the PR.
|
@zstanecic I see, thanks. But the implementation by @JulienCochuyt is completely different and better. And mine is currently not finished / is still under consideration. |
This code is awesome, especially the liblouis resolver part. I made some pre pr comments in accessolutions@820ff89 |
Thanks a lot for all these useful comments. Much appreciated. |
@LeonarddeR, |
Feel free to take your time. I won't be able to look into this before next week. |
Reported by falinn.onda on 2013-06-25 11:49
Following the discussion in
http://community.nvda-project.org/ticket/3284#comment:5
I think it is a good idea that NVDA will enable users to customize tables,
if they know the table format (by referring to louise docs).
In response to jteh
" * Coming up with an elegant user interface for this is tricky.
general, users should never touch the NVDA program directory, only their
config directory."
jteh, i agree with both comments.
I did not mean that NVDA will add a graphical user interface or that NVDA will have full documentation about customizing tables.
I do not think that is necessary since there is already a very good table definition format defined and explained in detail by louise documentation.
I think other users can tweak tables to their needs like i was able to after reading louise documentation.
the only thing i am suggesting is to have several table files which the user will be able to edit. I agree that it is bad practice to edit the default standard tables in louise/tables and that is exactly why i think it is a good idea to have few table files in userConfig\brailletables (or similar).
The only thing i need from NVDA is to have an option in the braille setting input/output tables combo boxes which lets me choose to use those custom files i wrote.
It is the user responsibility to edit the file outside NVDA in a text editor of his choice (or get a file someone else wrote).).
For example if we can put other1_custom_user_table.ctb other2_custom_user_table.ctb and other3_custom_user_table.ctb
in userConfig\brailleTables. (i'm suggesting file names starting with o because there is no such tables that start with o right now which will make it easy to switch to those combo )entries by pressing o ).
I am aware that not everyone will understand how to edit tables but most advanced users will benefit from this option and maybe give the tables to other users in their community or publish online like addons.
In NVDA documentation we can only add a chapter titled "Customizing braille tables":
Advanced users who wish to use custom braille tables can edit the tables files x, y, z in the folder w. For a definition of the file format see luise documentation link.
Note that users who only wish to add symbols to ecisting tables can use the include command.
Here we can have a small example file that includes english braille table and adds additional letter and punctuation as example.
What do you think?
Blocking #4996
The text was updated successfully, but these errors were encountered: