dev-qt/qtwebengine-dicts: initial packaging#164
dev-qt/qtwebengine-dicts: initial packaging#164jsbronder wants to merge 2 commits intogentoo:masterfrom
Conversation
|
LGTM |
Pesa
left a comment
Pesa
left a comment
There was a problem hiding this comment.
Should PV be a timestamp maybe? Like app-portage/eclass-manpages
| KEYWORDS="~amd64 ~arm ~arm64 ~x86" | ||
| IUSE="" | ||
|
|
||
| S="${WORKDIR}" |
There was a problem hiding this comment.
we usually declare this after the dependencies, although it doesn't really matter
|
|
||
| DEPEND=" | ||
| app-dicts/myspell-en | ||
| dev-qt/qtwebengine" |
| case ${lang} in | ||
| de-1901) dict="de_1901";; | ||
| pt-BR) dict="pt-br";; | ||
| *) dict="${lang}";; |
There was a problem hiding this comment.
Switched to spaces, the tabs looked deceivingly fine to me.
| unset dict lang | ||
|
|
||
| RDEPEND=" | ||
| ${DEPEND} |
There was a problem hiding this comment.
you can put this all on one line
| bdic_fn=${bdic_fn%.dic}.bdic | ||
| bdic_fn=${bdic_fn/_/-} | ||
|
|
||
| "${EPREFIX}"/usr/$(get_libdir)/qt5/bin/qwebengine_convert_dict \ |
There was a problem hiding this comment.
use qt5_get_bindir from qmake-utils.eclass
| local dicts | ||
| local dic | ||
| local bdic_fn | ||
| local prefix |
There was a problem hiding this comment.
you can declare multiple vars in one line with local
There was a problem hiding this comment.
Sure, but I find that makes things less readable. A number of style guides recommend against it in various languages. Still, I'll change them if you insist.
|
Regarding changing PV to a date, I don't think that makes sense here. The eclass-manpages have a date as they're generated from a snapshot of the portage tree. This is using whatever hunspell dictionaries are installed which cannot be directly tied to a specific date (aside from whenever the user ran emerge). Hence I think that the 1.0 is appropriate as this is the 1.0 version of building dictionaries that qtwebengine can use. If the packaging changes in the future, then I'd change the PV at that point. |
I'm aware of that. And technically it's a problem. If the hunspell dicts are updated, the webengine dicts are not, unless the user knows that they have to manually re-emerge |
|
Also, should there be a dependency on this package from |
|
Yeah, I don't have a good way to fix that. We can make the PV match the current Qt release as you suggest. Should there also be something in the logs/einfo about it? I don't think that qtwebengine needs a strict dependency but it could be a USE-flag controlled one that defaults to enabled? |
|
Alternatively, we could see if getting subslots on the dictionaries is a possibility. |
Yeah let's do that please.
Up to you, I'm fine either way.
I thought about that too but it doesn't seem worth the effort. |
Packaging of dictionaries suitable for use by QtWebEngine per https://doc.qt.io/qt-5/qtwebengine-webenginewidgets-spellchecker-example.html. This is simply done by by running qwebengine_convert_dict on each myspell dictionary. Note that: - qt5-build is not used as the dictionaries are not directly dependent upon the version of Qt being used. - Dictionaries are built for every installed myspell dictionary. This could result in a few extra dictionaries if the user is stripping L10N_* flags but that saves complexity in the ebuild as the map from language to dictionary files(s) isn't something that can be simply done with string parsing. See comments in ebuild. Package-Manager: Portage-2.3.13, Repoman-2.3.3
Package-Manager: Portage-2.3.13, Repoman-2.3.3
3 participants
Packaging of dictionaries suitable for use by QtWebEngine per
https://doc.qt.io/qt-5/qtwebengine-webenginewidgets-spellchecker-example.html.
This is simply done by by running qwebengine_convert_dict on each
myspell dictionary.
Note that:
- qt5-build is not used as the dictionaries are not directly
dependent upon the version of Qt being used.
- Dictionaries are built for every installed myspell dictionary.
This could result in a few extra dictionaries if the user is
stripping L10N_* flags but that saves complexity in the ebuild as
the map from language to dictionary files(s) isn't something that
can be simply done with string parsing. See comments in ebuild.
Package-Manager: Portage-2.3.13, Repoman-2.3.3