-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Keyword Tooltip/Cleanup for MSC codes added to Keyword Editor #12914
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
@@ -50,6 +55,8 @@ public class KeywordsEditor extends HBox implements FieldEditorFX { | |||
private static final Logger LOGGER = LoggerFactory.getLogger(KeywordsEditor.class); | |||
private static final PseudoClass FOCUSED = PseudoClass.getPseudoClass("focused"); | |||
|
|||
private Map<String, String> mscmap = MscCodeUtils.loadMscCodesFromJson("../../resources/main/org/jabref/gui/fieldeditors/msccodes/msc_codes.json"); |
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.
This won't work inside a jar, you need to use class.getResource
…to ktooltip updates from jabref main
src/main/java/org/jabref/gui/fieldeditors/msccodes/MscCodeUtils.java
Outdated
Show resolved
Hide resolved
…to ktooltip pulling changes from main to local
Please fix the failing tests before. This reduces the burden of us maintainers to review incomplete PRs |
src/main/java/org/jabref/gui/fieldeditors/msccodes/MscCodeUtils.java
Outdated
Show resolved
Hide resolved
…iptions, bug: error when trying to cleanup when editor is open
…er and special characters were added
src/main/java/org/jabref/logic/cleanup/ConvertMSCCodesCleanup.java
Outdated
Show resolved
Hide resolved
…to ktooltip adding changes from main
src/main/java/org/jabref/logic/cleanup/ConvertMSCCodesCleanup.java
Outdated
Show resolved
Hide resolved
Closing this issue due to inactivity 💤 Please ping us if you intend to resume work on this one. |
Hey @koppor, I'm waiting on review. All checks pass and the feature is implemented. Let me know if anything else is needed. |
Your pull request conflicts with the target branch. Please merge |
MSCMAP = tempMap; | ||
} | ||
|
||
public ConvertMSCCodesCleanup(BibEntryPreferences preferences, boolean convertToDescriptions) { |
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.
Boolean method parameters for public methods should be avoided. Better create two distinct methods which maybe call some private methods.
conversionPossible = true; | ||
} | ||
} catch (MscCodeLoadingException e) { | ||
logger.error(Localization.lang("Error loading MSC codes:", e)); |
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.
The logger should include the exception as the last argument to properly log the stack trace, instead of converting it to a string.
For help regarding conflict resolution, the pinned message in the gitter chat may help: https://matrix.to/#/!FrLnwBcqmXcZxTMyJj:gitter.im/$9pWlaPYKu81WXXwaU2LLkXnwg_vVWwrJjHmkwfYG8Lo?via=gitter.im&via=matrix.org&via=tchncs.de And, please, fix the submodules: https://devdocs.jabref.org/code-howtos/faq.html#submodules |
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.
Initial review
Where does src/main/resources/msc_codes.json come from? What is the license of the file?
Maybe, it could be downloaded during the build?
- We added a feature to convert keywords that resemble MSC codes to their descriptions. [#12944](https://github.com/JabRef/jabref/issues/12944) | ||
- We added a tooltip to keywords that resemble Math Subject Classification(MSC) codes. [#12944](https://github.com/JabRef/jabref/issues/12944) |
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.
swapped the entries, as the abbreviation explanation should go first. Moreover, a space comes after the term and before the bracket.
- We added a feature to convert keywords that resemble MSC codes to their descriptions. [#12944](https://github.com/JabRef/jabref/issues/12944) | |
- We added a tooltip to keywords that resemble Math Subject Classification(MSC) codes. [#12944](https://github.com/JabRef/jabref/issues/12944) | |
- We added a tooltip to keywords that resemble Math Subject Classification (MSC) codes. [#12944](https://github.com/JabRef/jabref/issues/12944) | |
- We added a feature to convert keywords that resemble MSC codes to their descriptions. [#12944](https://github.com/JabRef/jabref/issues/12944) |
Optional<HashBiMap<String, String>> optionalMscCodes = MscCodeUtils.loadMscCodesFromJson(resourceUrl); | ||
|
||
if (optionalMscCodes.isPresent()) { | ||
mscmap = optionalMscCodes.get(); // Unwrap the map if present |
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.
Remove this AI-generated comment
if (optionalMscCodes.isPresent()) { | ||
mscmap = optionalMscCodes.get(); // Unwrap the map if present | ||
} else { | ||
LOGGER.warn(Localization.lang("Resource not found: msc_codes.json")); |
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.
No need for localization here - just normal log
mscmap = HashBiMap.create(); | ||
} | ||
} catch (MscCodeLoadingException e) { | ||
LOGGER.error(Localization.lang("Error loading MSC codes:", e)); |
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.
No need for localization here - just normal log
and, no colon
--
Please review the AI generated code.
} | ||
} | ||
|
||
|
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.
No two empty lines
private final List<JabRefException> failures; | ||
|
||
public CleanupWorker(BibDatabaseContext databaseContext, FilePreferences filePreferences, TimestampPreferences timestampPreferences) { | ||
|
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.
Why this? Remove these spaces
@@ -31,8 +38,9 @@ public List<FieldChange> cleanup(CleanupPreferences preset, BibEntry entry) { | |||
|
|||
List<CleanupJob> jobs = determineCleanupActions(preset); | |||
List<FieldChange> changes = new ArrayList<>(); | |||
for (CleanupJob job : jobs) { | |||
for (CleanupJob job : jobs) { |
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.
Remove spaces at end
changes.addAll(job.cleanup(entry)); | ||
|
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.
Remove empty line
@@ -44,8 +52,18 @@ public List<FieldChange> cleanup(CleanupPreferences preset, BibEntry entry) { | |||
private List<CleanupJob> determineCleanupActions(CleanupPreferences preset) { | |||
List<CleanupJob> jobs = new ArrayList<>(); | |||
|
|||
// Special handling for MSC code conversion |
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.
Use one term for the thing done: "description conversion" maybe? - the Boolean
variable of the method contains description
(which is good); this should be done trhoughout the code and ocmments.
/* | ||
* Converts MSC codes found in keywords editor to their descriptions | ||
*/ |
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.
This is a class comment -> move above public class
The link to zbmath shared in the issue description above takes you to a PDF, which I then converted, using a Python script, into the JSON file you see. I did not see any license. This json file is not an exact copy since changes had to be made to the format because of the "," delimiter. @koppor |
Nice! Please share the Python script (https://github.com/JabRef/jabref/tree/main/scripts)
OK. I think, in the context of JabRef usage, this is OK. |
Closes #12944
Created a JSON file to hold key-value pairs of MSC codes to their respective descriptions/classifications by parsing msc_2020.pdf (which was provided in link to zbmath).
Created MscCodeUtils.java class that implements a method to load a JSON file and converts said file into a Map object.
Created ConvertMSCCodesCleanupTest.java, added to panel, and when user selects preference to convert and saves, the codes are converted to descriptions. Alternatively, when unselected, descriptions are reverted back to code.
In KeywordsEditor.java I added a private variable, mscmap, that calls the above method. Using this map object, I added a condition in the create tag method to check if the label text matches any of the keys. If it does, then we initialize a Tooltip and call install onMouseEnter and uninstall onMouseExited.
ToolTip
Cleanup
Before:
After:
Mandatory checks
CHANGELOG.md
described in a way that is understandable for the average user (if change is visible to the user)