Unlocalized preferences new#8304
Conversation
|
@koppor @calixtus @tobiasdiez I did not have much time to implement this yet, but changed one preference as an example. Would be great if you could cross-check if this is what we agreed on. |
tobiasdiez
left a comment
There was a problem hiding this comment.
Looks good to me! Two suggestions to further improve it:
| } | ||
|
|
||
| public void storeSettings() { | ||
| String eMailSubject = eMailReferenceSubjectProperty.getValue(); |
There was a problem hiding this comment.
I would move this to the storeExternalApplicationsPreferences method so that you don't need to expose the default value outside and have consistent behavior if some other code also needs to change the email subject.
There was a problem hiding this comment.
I guess you mean the whole comparison to the default value? That needs localization, if we leave that in storeExternalApplicationsPreferences we do not get rid of the localization dependence in the preferences.
There was a problem hiding this comment.
Yes, that's what I had in mind.
I don't think it's a bad thing per se if the JabRef preference class depends on the localization but that depends a bit on what you want to achieve: complety remove localization aspects form JabRef preferences or only remove localization of the stored preference values.
As a middle ground one could put the translation logic in ExternalApplicationsPreferences
There was a problem hiding this comment.
Target was to reduce JabRefPreferences to the bare minimum, so we can modularize the gui and the cli. So the fewer dependencies, the better.
| defaults.put(KEY_GEN_FIRST_LETTER_A, Boolean.TRUE); | ||
| defaults.put(KEY_GEN_ALWAYS_ADD_LETTER, Boolean.FALSE); | ||
| defaults.put(EMAIL_SUBJECT, Localization.lang("References")); | ||
| defaults.put(EMAIL_SUBJECT, "References"); |
There was a problem hiding this comment.
Idea: since we now loose our nice localization tests that make sure that "References" should be translated, what do you think about adding a Localization.plan method that simply returns the string. Than one could change the localization tests to also look for plain in addition to lang . That would minimize the disadvantages even more I think.
There was a problem hiding this comment.
Wouldn't be Localization.plain("References") then always the same as "References"?
There was a problem hiding this comment.
Yes. The point was to still have the localization tests which could look for Localization.plain
|
|
||
| public void setValues() { | ||
| eMailReferenceSubjectProperty.setValue(initialPreferences.getEmailSubject()); | ||
| defaultEMailReferenceSubject = Localization.lang((String) preferences.getDefaults().get(JabRefPreferences.EMAIL_SUBJECT)); |
There was a problem hiding this comment.
As we already discussed in the DevCall, I'd say: Thou shall not access low level code directly from high level context. ;-)
There was a problem hiding this comment.
So what do you propose we do instead? If we cannot access the translated default here, this approach won't work.
There was a problem hiding this comment.
Im thinking of completly different approach to defining defaults. Instead of working in the low level api of JabRefPreferences with the internal map strings, the defautls could be placed in another class DefaultPreferences which implements PreferencesServices.
There was a problem hiding this comment.
Maybe this could be a solution for our localized preferences debate?
|
Closing this issue due to inactivity 💤 |
Currently, JabRef uses some localized preferences. Example: The entry editor offers a tab "Comments". This heading can be customized.
We want to remove the Localization-dependency from
JabRefPreferencesand move the Localization to where the string is used.See ADR0023 for implementation details.
There is a first attempt and lots of discussion in #8050.
CHANGELOG.mddescribed in a way that is understandable for the average user (if applicable)