Regretfully, up-to-now the plugin names were used directly and cannot be translated.
Details
Details
Diff Detail
Diff Detail
- Repository
- R326 Kalzium
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Comment Actions
IMHO it's better if you use the addItem(name, data) API, storing the name of each plugin in the combobox. Then it's easy to fetch the data of the selected item.
Also, the use of QOverload does not belong to this i18n fix, so it needs to be a separate patch.
Comment Actions
Make the connection between ui.styleCombo::currentIndexChanged and MoleculeDialog::slotUpdateScenePlugin ligical (first sends the number of index to the second).
Remove extra space at the end of the line.
Comment Actions
The approach here should be changed: instead of hardcoding the names of plugins in the styleCombo, they ought to be read directly from the sceneModel() of the GL widget. This way:
- the strings come directly from avogadro itself, no more need to translate them in kalzium
- the list of styles reflects what avogadro provides, so no more mismatch between what is hardcoded in kalzium vs the plugins avogadro has
- the documentation of kalzium ought to better not list all the plugins/styles available, since they depend on what avogadro has (mentioning a couple is more than enough, IMHO)
This also means that more i18n work is needed in avogadro itself.
| doc/index.docbook | ||
|---|---|---|
| 494 | This change is fine, just commit it directly outside of this review request. | |
| 498–502 | This change is fine, so just commit it outside of this review request. | |
| src/tools/moleculeview.cpp | ||
| 176 | It "works" mostly because there is no i18n system in avogadrolibs, even though the strings are supposed to be translatable (marked with tr(), mentioned "shown in the gui" in the API docs, etc). | |
Comment Actions
What is the way to proceed?
It seems I have a good friction with the current Avogadro maintainers. What should be changed? The translations are in the repo and on Launchpad, just not packed by the distributions.
Should I abandon this revision for some better times?
Could someone shake some salt of knowledge on my life about how to fix this "in model"?
Why it was good to use such an approach for other parts of Kalzium (can be easily proven by searching "->addItem")?
Thanks in advance for your answers.
Comment Actions
The one I explained above.
It seems I have a good friction with the current Avogadro maintainers. What should be changed? The translations are in the repo and on Launchpad, just not packed by the distributions.
Looking at avogadro 1.2.0 (Qt4 version), it seems they ship them, and at least the Debian packaging bundles them in the "avogadro" package.
The other thing is making sure that avogadro actually uses them (which seems to be done in 1.2.0).
Should I abandon this revision for some better times?
Following my suggestions would be a better way forward.
Could someone shake some salt of knowledge on my life about how to fix this "in model"?
ui.glWidget->sceneModel() returns a QAbstractItemModel, which can be iterated using its API.
Why it was good to use such an approach for other parts of Kalzium (can be easily proven by searching "->addItem")?
git log and find out who/why did that.
Comment Actions
Abandoned because the lack of needed knowledge from the author to satisfy the reviewer requirements.