skinning-internationalization

[tanya]

skinning-internationalization

• Provide a way skinning the titles and tooltips in the application
• Provide a document that describes what the skins will look like
• Provide a way to change skins in Sophie
• Provide an example skin that contains all of the changeable titles in Sophie

See
SkinningInternationalization?

[todor]

good analysis

[tanya]

log:branches/private/tanya/2411

[meddle]

To im-f.

[pap]

• Commited to trunk in [9118], [9138] and [9140]
• Although some requirements are not met and the ticket quality is very poor the task is(was) accepted due to its importance.
• The code is not well enough structured and the flow of logic could be better.
• The way the NONE skin is treated should be improved.
• Remove the unnecessary LanguageSkin file.
• The classes seem to be too much for a single package.
• Remove commented out code Skin
• What about making the getValue and getValues methods package private or protected and expose value getting only through SkinUtil in order to ensure usage of default skin.
• Also fix logging (actually this is my fault). Now there are too much log messages in SkinManager and they are not with the proper LogLevel.
• SkinEntry is a bit misleading name for what it does. Also its javadoc isn't correct - it is not an ExtensionPoint.
• It can and probably should be made a generic by the returned object.
• Also it's kind is sometimes referred to as "type". Be more consistent in naming - choose one of these.
• The JavaDoc of TextSkinEntry is bad - this is not an extension of the extension point. There is another semantic.
• Same about SkinPartsProvider.
• Probably the SkinNames is no longer needed. Also the values of the constants should be changed - probably "language:default" and such.
• Don't forget to fix the getValues methods. These return all available skin values for a given recordid.
• The way a value class is classified to a certain pack may also be extensible.
• To me it seems that the searchSkin method doesn't need to be public. Also this method searches the fallback two times for the same record and this can be avoided (see how Skin.getValue works).
• I think that it is somewhat better to use "language pack" instead of "language skin". This is about documentation, variables, ConfigKey names, etc.
• About the default skins
  • The way with lazy getting is nice because it is lazy.
  • But you can also put the default skin in the cache for the none key and this way ommit all the special code for handling default skin.
  • Your decision. But i think that createSkin should not return the default skin if you continue using the lazy getting. Just null.
• loadBaseSkinParts is not a good name. They are just skin parts.
• getContents can be loadPart.
• Extract more constants.
• Please rewrite the createSkin method. I think it can be simpler and with less return statements.
• In the initRecord method please consider cases where the split would not return exatcly three pieces. They can be less or more.
• The assertations in {{{createValue}} can be put in a single one. Also you may put a message there.
• In the getContents method trim the lines before checking them for comments or emptyness. Also double-check whether the try/catch/finally blocks always work correctly. It seems so, but I usually see a different way to do that (see the attached patch).
• The thing about default skins should also be consistent in the constructor of SkinManager.
• And last but not least, I really thank you for all the effort.

[meddle]

Closing all the tickets before M Y1