Skip to content
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

New table for Greek braille #243

Closed
wants to merge 12 commits into from

Conversation

DaveMielke
Copy link
Contributor

No description provided.

gr-forward.ctb is for forward translation.
gr-backward.ctb is for back translation.
gr-common.cti is included by both.
Back translation of English letters isn't supported yet.
@bertfrees
Copy link
Member

bertfrees commented Jan 11, 2017

Great. Looks good.

There are a couple of things we ask for every new table or table change:

  • A test. Preferably in YAML format. This is mainly for documentation purposes, to make it clear to others why a change was made. Also for your own good, to safeguard the table. If other people come along that want to make changes to the table, we of course always contact the original author first, but if you are missing we will just accept new changes as long as they don't break any tests.
  • A reference to the specification of the implemented braille code. Preferably some official document. If the document isn't available online we can put it in our braille-specs repo.
  • Table metadata. We've started tagging our tables for discovery purposes. See https://github.com/liblouis/liblouis/blob/4bf05eb/doc/liblouis.texi#L1621 (not in HTML format yet).
  • An entry in the NEWS file. (We'll do this.)
  • Possibly update Makefiles. (We'll do this.)
  • If it's a new table, see how it relates to existing tables and merge if appropriate. In this case there already exists a table for Greek (two actually but the other one is for biblical Greek which is a clear use case): gr-gr-g1.utb, by ViewPlus.
  • Proper license header

Also:

  • Update AUTHORS file. Dave is currently only listed under "BRLTTY". (We'll do this.)

@DaveMielke
Copy link
Contributor Author

DaveMielke commented Jan 11, 2017 via email

@bertfrees
Copy link
Member

bertfrees commented Jan 11, 2017

@egli What should we do with gr-gr-g1.utb?

Regarding table metadata: this is the metadata we currently have for gr-gr-g1.utb: https://github.com/liblouis/liblouis/blob/4bf05ebe25c3d28d99e988ef84dd133595b99aa1/tables/el.tbl.

@bertfrees
Copy link
Member

Regarding the test: especially the mixed Greek/English deserves some attention.

@egli
Copy link
Member

egli commented Jan 11, 2017

@bertfrees I don't know what to do with the old greek table. I see the following options

  1. Just replace it with the new table and trust @DaveMielke that it really is better
  2. Ask on the mailing list for some advice.
  3. Keep the old one around under some other name.

Option 3 is what we have traditionally been doing, basically just avoiding the issue. Not a great solution. Option 2 will likely not yield any more trustworthy results than what we have now. So I tend to go with option 1.

I think we should try option 2 and wait a week then just do option 1.

@DaveMielke
Copy link
Contributor Author

DaveMielke commented Jan 11, 2017 via email

@bertfrees
Copy link
Member

@DaveMielke I see. I didn't check it, just assumed. But you should be listed under "patches" too when your pull request #244 is merged. But don't worry about the AUTHORS file we take care of that.

@egli Yes option 3 is definitely to be avoided. We'll ask on the mailing list with the ViewPlus people in CC.

@egli egli modified the milestone: 3.1 Jan 11, 2017
@egli egli changed the title pr-gr New table for Greek braille Jan 11, 2017
@DaveMielke
Copy link
Contributor Author

DaveMielke commented Jan 11, 2017 via email

@bertfrees
Copy link
Member

bertfrees commented Jan 11, 2017

Hmm, I'm not sure. The only categories of contraction I've used so far is "contraction:no" (the default) and "contraction:full". The idea is to add more values such as "partial". We could add a value for your case too, but I don't know if it's common enough for a new category. I would say this belongs to "contraction:no", but a comment saying what you just said would be useful I think.

Copy link
Member

@bertfrees bertfrees left a comment

Choose a reason for hiding this comment

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

Thanks! Good that you included the "direction" tag. I have thought of that before, but can't seem to find it in my documentation. Note that I'm going to remove the tags from gr-common.cti because it's a table meant to be included in other tables, and only top-level tables should have metadata.

@DaveMielke
Copy link
Contributor Author

DaveMielke commented Jan 17, 2017 via email

@bertfrees
Copy link
Member

Thanks. I wanted to do the exactly same (remove the plus signs but leave the metadata in). Until we have figured out what the tool for printing the metadata is gonna look like, it doesn't matter, so I just wanted to keep things like they are. But I'm taking your arguments for having metadata in all files into consideration!

@DaveMielke
Copy link
Contributor Author

DaveMielke commented Jan 18, 2017 via email

@egli
Copy link
Member

egli commented Jan 18, 2017

Hi @DaveMielke would it be possible to add license headers to these tables the same way the others specify the license? We've been burned in the past and now we want to be extra-safe. Something like https://github.com/liblouis/liblouis/blob/master/tables/Cz-Cz-g1.utb#L3-L21 for example? That way it would be the same as all the other tables and we can use an automated tool to scan for licenses.

Thanks

@bertfrees
Copy link
Member

@DaveMielke Regarding the (temporary) convention: sure.

But making it permanent would exactly come down to what I had in mind initially, and what you were against! Right?

@DaveMielke
Copy link
Contributor Author

DaveMielke commented Jan 18, 2017 via email

@DaveMielke
Copy link
Contributor Author

DaveMielke commented Jan 18, 2017 via email

@bertfrees
Copy link
Member

OK fine. I was just a bit surprised because from the previous discussions I had the impression that you didn't like the idea of having two different formats for metadata...

By the way I know it's only suggestions but it really helps me and I appreciate your input.

@DaveMielke
Copy link
Contributor Author

DaveMielke commented Jan 18, 2017 via email

@bertfrees
Copy link
Member

OK, yes, but anyway, this is what I've had in mind from the beginning. I should have been more clear about it.

@DaveMielke
Copy link
Contributor Author

DaveMielke commented Jan 18, 2017 via email

@egli
Copy link
Member

egli commented Jan 18, 2017

OK, license is perfect now. Thanks

@bertfrees
Copy link
Member

All that is left to do now is to ask ViewPlus whether we can replace their table.

@DaveMielke Have you thought about that reference to the specification of the implemented braille code? Is there something we can point to?

@DaveMielke
Copy link
Contributor Author

DaveMielke commented Jan 26, 2017 via email

@bertfrees
Copy link
Member

OK, I see. Is there some document we can put online? It can be in Greek. If there's nothing available it's fine.

I can find some documents, but I'm not sure which corresponds to what you implemented:

I can also find the website of the organisation (http://keat.gr), but no documents there.

@DaveMielke
Copy link
Contributor Author

DaveMielke commented Jan 26, 2017 via email

@bertfrees
Copy link
Member

@egli
Copy link
Member

egli commented Mar 1, 2017

Merged manually from the https://github.com/liblouis/liblouis/compare/dm/integration branch

@egli egli closed this Mar 1, 2017
@egli
Copy link
Member

egli commented Mar 1, 2017

OK, I've asked on the ml what to do with the old table

@bertfrees
Copy link
Member

Thanks.

Maybe forward the email directly to someone of ViewPlus?

We've had complaints before because we changed file names. Of course sometimes that is unavoidable, but I think if it is possible to keep a file name that is preferable. So if the old Greek table can be removed it would be better to do it now as opposed to some time later because then we can use the old name. WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants