-
Notifications
You must be signed in to change notification settings - Fork 112
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 recommended dictionaries to welcome page #991
Conversation
I don't intend to add in other language dicts in this pr unless someone else wants to compile a list to add on but I'll be making it extensible so they can just be dropped in
It's the same progress bar that is used for normal dictionary imports
The import events are queued and will execute in the order the user clicked on them Only problem with this at the moment is there is no indicator for how many dicts are being imported since it's calling single dict imports one after another
It's the same footer thing we use for normal dict imports. |
Mostly done here just needs some finishing touches.
Also open to any feedback ofc |
I have to update my website every week to change the download links to point to the latest dictionary versions. I wouldn't mind maintaining some sort of manifest file on my website with links to the latest version as well. For example I could have something like Going one step further, it would be neat if dictionaries could include links to these sorts of manifest files within the |
@stephenmk would it be possible for you to make a static url that always pulls the latest jitexdex? Due to your release asset names changing every release (they include a date) it isnt possible to pull latest straight off github like that. I'd like to push this pr along even if it isn't entirely perfect. And I don't think theres too much need to pin dict versions to yomitan releases. New users aren't likely to be pulling down old versions of the extension so I don't think users downloading incompatible dicts is much of a worry. |
Maybe this is outside the scope of what you wanted to do right now, but the main issue I wanted to address with my "manifest" idea was the problem of upgrading. If I make a static url that always points to the latest yomitan ZIP file of jitendex, then there's no way for users to know that the ZIP file has been updated without making yomitan redownload the whole file, extracting it, checking the If, instead, there was a small JSON file containing jitendex metadata that yomitan can fetch, then it could alert users when new versions are available. Yomitan could then give users the option to update. For example, it might look something like this.
{
"name": "Jitendex",
"version": "24.06.19.0",
"minimum-compatible-yomitan-version": "24.06.18.0",
"icon": "https://jitendex.org/favicon.ico",
"update-url": "https://github.com/stephenmk/stephenmk.github.io/releases/download/v4.7-9/jitendex-yomitan-2024-06-19.zip",
"sha256sum": "0ea49da53e23e3cd1c2b525754f2df25a9995f1ede4077e5c3e03561b52cf950",
"notes": "Information about updated content could go here."
} |
I think that's the right approach. For simplicity I'd have the metadata json file be the index.json, since it's for storing metadata. To start with, I'd add to the index.json schema a boolean like |
@stephenmk I agree with your idea for updating but I don't plan on adding anything for updating dicts in this PR. Like stefan mentioned I think it would be best to have this info in the index.json so anybody that downloads the dict from any source gets that update information. With a static download url users who installed that way from the recommended dicts section will be able to update the dict exactly the same. I'd rather not hold up this pr on adding update functionality or even deciding on the correct format for providing update information. At the moment the alternative I see is to provide JMdict which does have a static link. I would like to use jitendex here but if we need to hold off from doing that until updating support is added, that's fine. I think it's better to use JMdict over potentially outdated versions of jitendex from having to use a link that does not get updated (or only gets updated when yomitan updates). I see the accuracy of the dictionary data provided is more important than the fancy formatting of jitendex. Especially considering we're putting this right in the welcome page of yomitan as a recommendation to install. |
It is also possible to provide update functionality for approved dictionaries after they have been downloaded without update information. It cant be 100% seamless like having the update information directly in the dict (due to security concerns we will need users to verify they want to download from the sources we provide for dicts) but this will be needed for jmdict, jmnedict, kanjidic, and maybe others anyways. |
I removed the date from the filename of latest version of the jitendex. It should always be 'jitendex-yomitan.zip' from now on. |
Thank you! |
…ictionaries to one that doesnt
This looks good to me, I think it's a fine start and can be iterated on further, especially as we get a better idea of how we want the automated updating to work. BTW, one somewhat unrelated observation is that, it seems quite likely that most dictionary files (at least any appropriately licenced ones) will be hosted in GitHub releases. At least all the ones in this PR seem to be. On top of that, while not all are built in GitHub Actions, some are. Maybe in the future we could require dictionaries to also have an artifact attestation so we have a guarantee they were built in GitHub Actions as opposed to uploaded directly to the release. That'd give people a much better guarantee that the dictionary file isn't somehow maliciously modified by someone with write access to the repo, but rather has gone through proper code review, merged, and built within GitHub. Since malicious dictionary files have a limit on what sort of damage they can do due to natural sandboxing of the extension it's probably not too important, but there are still risks like someone adding lots of shock/gore content, or doing some sort of DoS of the extension with a maliciously crafted dictionary, so IMO it wouldn't hurt to consider putting a high bar on build provenance for official dictionaries included into Yomitan like this. |
This is totally broken after merging with master. Will try to get it cleaned up soon. |
This should be good to go now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"grammar": [], | ||
"pronunciation": [] | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that updating is in, would it take a lot of work to list index.json
files instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would have to ask @StefanVukovic99 on that one. But I think it's probably better to let another pr take that one. And static links will still need to be supported even if we add index.json support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'd like that so we can show more info, like the description, but not necessary for now
tiny pr to show download progress - https://github.com/Kuuuube/yomitan/pull/2/files |
CodSpeed Performance ReportMerging #991 will degrade performances by 24.67%Comparing Summary
Benchmarks breakdown
|
Fixes #1168
Currently I've only added japanese dicts but other languages can easily be added into
ext/data/recommended-dictionaries.json
.