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

add source and target language dictionary metadata #891

Merged

Conversation

StefanVukovic99
Copy link
Member

image

This is just displaying them like the rest of the metadata, but I think it may also be useful in the future for

@StefanVukovic99 StefanVukovic99 requested a review from a team as a code owner May 7, 2024 13:45
Copy link

github-actions bot commented May 7, 2024

✔️ No visual differences introduced by this PR.

View Playwright Report (note: open the "playwright-report" artifact)

@StefanVukovic99 StefanVukovic99 added kind/enhancement The issue or PR is a new feature or request area/dictionary-format The issue or PR is related to dictionary formatting labels May 7, 2024
@@ -47,6 +47,14 @@
"type": "string",
"description": "Attribution information for the dictionary data."
},
"sourceLanguage": {
"type": "string",
"description": "Language of the terms in the dictionary."
Copy link
Collaborator

Choose a reason for hiding this comment

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

iso format or does that not matter?

Copy link
Member Author

@StefanVukovic99 StefanVukovic99 May 8, 2024

Choose a reason for hiding this comment

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

I considered a pattern like ^[a-z]{2,3}(-[A-Z]{2})?$, but I haven't been able to find a perfect one.
Or we could do a simple pattern with all our currently supported languages like en|de|fa|fi..., but that's another thing to tweak every time a new language is added.
Or use this https://gist.github.com/konsorten-michael/094f898c3a2847c642a920f7c6f4f575

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's use the gist? People can submit issues if something else comes up. If we want to support all languages handlebars i feel it's important to enforce all the same languages use the same string for source language

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll throw the gist in, just wanna note that it contains ISO 639-1 codes, we might need to add to that over time for languages that are only in 639-2 or 3, e.g. we have grc as an option in the dropdown.

ext/data/schemas/dictionary-index-schema.json Show resolved Hide resolved
Copy link
Collaborator

@jamesmaa jamesmaa left a comment

Choose a reason for hiding this comment

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

One more change request

"isoLanguageCode": {
"type": "string",
"description": "ISO language code (ISO 639-1 where possible, ISO 639-3 otherwise).",
"pattern": "^(aa|ab|ae|af|ak|am|an|ar|as|av|ay|az|az|ba|be|bg|bh|bi|bm|bn|bo|br|bs|ca|ce|ch|co|cr|cs|cu|cv|cy|da|de|dv|dz|ee|el|en|eo|es|et|eu|fa|ff|fi|fj|fo|fr|fy|ga|gd|gl|gn|grc|gu|gv|ha|he|hi|ho|hr|ht|hu|hy|hz|ia|id|ie|ig|ii|ik|io|is|it|iu|ja|jv|ka|kg|ki|kj|kk|kl|km|kn|ko|kr|ks|ku|kv|kw|ky|la|lb|lg|li|ln|lo|lt|lu|lv|mg|mh|mi|mk|ml|mn|mr|ms|mt|my|na|nb|nd|ne|ng|nl|nn|no|nr|nv|ny|oc|oj|om|or|os|pa|pi|pl|ps|pt|qu|rm|rn|ro|ru|rw|sa|sc|sd|se|sg|si|sk|sl|sm|sn|so|sq|sr|ss|st|su|sv|sw|ta|te|tg|th|ti|tk|tl|tn|to|tr|ts|tt|tw|ty|ug|uk|ur|uz|ve|vi|vo|wa|wo|xh|yi|yo|za|zh|zu)$"
Copy link
Member Author

Choose a reason for hiding this comment

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

This could also be an enum, but i suppose it's fine either way

@jamesmaa jamesmaa added this pull request to the merge queue May 8, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 8, 2024
@jamesmaa jamesmaa added this pull request to the merge queue May 8, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 8, 2024
@jamesmaa jamesmaa added this pull request to the merge queue May 9, 2024
Merged via the queue into themoeway:master with commit 57d928b May 9, 2024
10 checks passed
MarvNC added a commit to MarvNC/yomichan-dict-builder that referenced this pull request Jul 7, 2024
MarvNC added a commit to MarvNC/yomichan-dict-builder that referenced this pull request Jul 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dictionary-format The issue or PR is related to dictionary formatting kind/enhancement The issue or PR is a new feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants