GNOME Bugzilla – Bug 621827
Change addin categories to constants
Last modified: 2010-10-18 19:14:09 UTC
Created attachment 163872 [details] [review] Proposed feature. Bug 620402 makes addin categories visible to user. This patch makes them localizable.
The problem with the patch is that it confuse identifiers with human readable labels. The reason the strings where not translated is because they are identifiers. The translation should be dealt with by mapping identifiers with labels in the UI. To ensure there is not typo with identifiers I also suggest to make them constants and have the addins only use that.
> The problem with the patch is that it confuse identifiers with human readable > labels. The reason the strings where not translated is because they are > identifiers. I noticed this problem too. It's not big as each of them seems to appear only once in POT file. But it surely may become a bigger issue, if they get used more widely, so you are right. > The translation should be dealt with by mapping identifiers with labels in the > UI. To ensure there is not typo with identifiers I also suggest to make them > constants and have the addins only use that. I support the idea of identifiers being constants. I'll try to rewrite the patch this way.
Created attachment 171449 [details] [review] Turn addin categories into integer constants I have defined constants for addin categories and changed all addins to return them.
The change in void ExportToHtmlNoteAddin::export_button_clicked() seems to be unrelated. Was it intentional?
(In reply to comment #4) > The change in void ExportToHtmlNoteAddin::export_button_clicked() seems to be > unrelated. Was it intentional? This modification is unrelated. It is actually already committed in order to fix the other bug. I see one more accidental change at the very bottom of the patch (change of formatting). Excluding these two, are there any more notes?
Also it would be preferrable to have the category be an enum {} rather than "const int". There is no reason to populate the symbol table with them. Also they don't belong to src/sharp/dynamicmodule.hpp as this file isn't meant to be gnote specific.
Created attachment 172499 [details] [review] Change addin categories to constants Defined addin categories alongside the AbstractAddin.
Comment on attachment 172499 [details] [review] Change addin categories to constants Looks good to me.
Review of attachment 163872 [details] [review]: This solution was rejected in favor to other.