-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Observable preferences F (GuiPreferences, Proxy and Remote) #8166
Conversation
# Conflicts: # src/main/java/org/jabref/preferences/JabRefPreferences.java
} | ||
|
||
public void storeSettings() { | ||
storeRemoteSettings(); | ||
storeProxySettings(); | ||
|
||
storeProxySettings(new ProxyPreferences( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's late so I might missing something obvious here, but: You create a new ProxyPreference object here and storeProxySettings only calls the set*
methods. Since there are no bindings of that proxypreference object to JabRefPreferences, this will essentially do nothing?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're missing something. I think.
storeProxyPreferences takes just the members one by one of the newly created object and stores them in the original preferences object, that was supplied by JabRefPreferences. See NetworkTabViewModel::storeProxySettings
:
private void storeProxySettings(ProxyPreferences newProxyPreferences) {
if (!newProxyPreferences.equals(proxyPreferences)) {
ProxyRegisterer.register(newProxyPreferences);
}
proxyPreferences.setUseProxy(newProxyPreferences.shouldUseProxy());
proxyPreferences.setHostname(newProxyPreferences.getHostname());
proxyPreferences.setPort(newProxyPreferences.getPort());
proxyPreferences.setUseAuthentication(newProxyPreferences.shouldUseAuthentication());
proxyPreferences.setUsername(newProxyPreferences.getUsername());
proxyPreferences.setPassword(newProxyPreferences.getPassword());
}
The reason for creating a new preferences object here is to keep the test proxy button.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay thanks, I missed that indeed.
l10n fails:
|
Thanks, just started IntelliJ a second ago to fix it, but you were quicker. :-) |
Follow up to #8165
CHANGELOG.md
described in a way that is understandable for the average user (if applicable)