-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Fixed Enable resetting of NameDisplayPreferences #14765
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
base: main
Are you sure you want to change the base?
Conversation
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
jabgui/src/main/java/org/jabref/gui/maintable/NameDisplayPreferences.java
Show resolved
Hide resolved
jabgui/src/main/java/org/jabref/gui/preferences/JabRefGuiPreferences.java
Outdated
Show resolved
Hide resolved
jabgui/src/main/java/org/jabref/gui/preferences/JabRefGuiPreferences.java
Outdated
Show resolved
Hide resolved
jabgui/src/main/java/org/jabref/gui/preferences/JabRefGuiPreferences.java
Show resolved
Hide resolved
|
Hey @calixtus i have made changes as suggested |
|
Your code currently does not meet JabRef's code guidelines. We use Checkstyle to identify issues. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Source Code Tests / Checkstyle (pull_request)" and click on it. In case of issues with the import order, double check that you activated Auto Import. You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports. Please carefully follow the setup guide for the codestyle. Afterwards, please run checkstyle locally and fix the issues, commit, and push. |
|
Clear your preferences and try to run again. I expect it to fail, because getBoolean etc. looks for a value in the defaults map, which is not present in backing store with cleared preferences. |
| if (getBoolean(NAMES_AS_IS)) { | ||
| return NameDisplayPreferences.DisplayStyle.AS_IS; | ||
| } | ||
| if (getBoolean(NAMES_FIRST_LAST)) { |
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.
Will fail with fresh install / cleared preferences. Did you try it?
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.
i will get on it and let u know
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.
Stick to the plan. Modify the helper methods and follow the guide in the issue description.
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.
And please test you changes manually.
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.
NEW
private NameDisplayPreferences.AbbreviationStyle getNameAbbreviationStyle() {
nameDisplayPreferences = NameDisplayPreferences.getDefault();
return nameDisplayPreferences.getAbbreviationStyle();
}OLD
private NameDisplayPreferences.AbbreviationStyle getNameAbbreviationStyle() {
NameDisplayPreferences.AbbreviationStyle abbreviationStyle = NameDisplayPreferences.AbbreviationStyle.NONE; // default
if (getBoolean(ABBR_AUTHOR_NAMES)) {
abbreviationStyle = NameDisplayPreferences.AbbreviationStyle.FULL;
} else if (getBoolean(NAMES_LAST_ONLY)) {
abbreviationStyle = NameDisplayPreferences.AbbreviationStyle.LASTNAME_ONLY;
}
return abbreviationStyle;
}Hey @calixtus , since I can't use getBoolean method is this a right approach then?
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.
Sorry, I'm travelling. Back in about 12 days
User description
Closes #14412
Steps to test
Mandatory checks
CHANGELOG.mdin a way that is understandable for the average user (if change is visible to the user)PR Type
Enhancement
Description
Added private constructor with default display preferences
Added
getDefault()static method for creating default instancesAdded
setAll()method to copy preferences from another instanceDiagram Walkthrough
File Walkthrough
NameDisplayPreferences.java
Add default constructor and utility methodsjabgui/src/main/java/org/jabref/gui/maintable/NameDisplayPreferences.java
style and abbreviation style
getDefault()method to create instances withdefault preferences
setAll()method to copy display and abbreviation stylefrom another
NameDisplayPreferencesinstance