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
aspell: add all missing official dictionaries #53143
Conversation
Running |
It's good to add dictionaries for other languages, however, we need a way to maintain these expressions. Basically, we need a script that can generate the data set. |
I’ll see if I can come up with a robust script for keeping these up to date. |
From: https://ftp.gnu.org/gnu/aspell/dict/0index.html Use a specialized function, buildOfficialDict to specialize builds of official aspell dictionaries and add an update script.
Added a |
The name should include the version number and this is handled just fine by buildDict.
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.
@liff This looks good, thanks! Just one point: here, it looks like a MitM to someone who runs the update script can execute arbitrary code on the computer running the update script (via the $versions
substitution), so it's I think a bit too permissive.
Maybe it'd be possible to write it in a more defensive way, to prevent that from happening?
|
||
meta = { | ||
homepage = "http://ftp.gnu.org/gnu/aspell/dict/0index.html"; | ||
} // (args.meta or {}); |
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.
This line is useless with the // removeAttrs args [ ... ]
below: args.meta
would override the whole meta
anyway. I guess you forgot "meta"
in the list below?
Would it be sufficient to restrict the version string to |
It should do it indeed! Now, I've just noticed another issue: the |
- Use HTTPS and tighter version regexp to guard against MitM - Use nix-instantiate instead of experimental nix eval - Handle dictionary-specific meta overrides
Thanks for the review. Changed from FTP to HTTPS since apparently they are going to disable FTP at some unspecified time anyway, adjusted the regexp, changed from |
@GrahamcOfBorg eval |
lynx instead of curl? This is weird.
|
@liff Do you think you could replace your usage of |
In practice hacks like those in my updater |
Changed to |
@GrahamcOfBorg eval |
Looks like everything is good! Thank you, @liff :) Merged! |
… And noticed just a bit too late to stop the push: The last 3 commits' messages are not following nixpkgs' convention (while the first 3 were). Mea culpa for not having noticed this. |
Oops, sorry. Seems I forgot about that half way when working on the requested changes. |
From:
ftp://ftp.gnu.org/gnu/aspell/dict/0index.html
Also sets the default homepage of the dictionaries to
the above URL.
The Marathi and Hindi dictionaries both contain
u-deva.{cmap,cset}
files, which cause a conflict. This patch adds those files from the Marathi dictionary into the main aspell package like Debian does.Motivation for this change
I intended to add the Finnish dictionary to
aspellDicts
but noticed there’s a fairly large bunch of official ones not included so why not add them all in one go?Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)