Skip to content

Conversation

Yubo-Cao
Copy link
Collaborator

@Yubo-Cao Yubo-Cao commented Apr 3, 2025

Closes #12273

  1. I have modified the src/main/java/org/jabref/gui/journals to handle the UI logics.
  2. I have added the LTWARepository class and integrated it as a part of the JournalAbbreviationRepository to handle the LTWA abbreviation logics. I am not exactly sure if it would be appropriate to change the abbreviation class since that class appears to be largely a data class, and I am hesitant to add business logics to it.

image

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (if change is visible to the user)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.
try (InputStream resourceAsStream = JournalAbbreviationRepository.class.getResourceAsStream("/journals/ltwa-list.mv")) {
if (resourceAsStream == null) {
LOGGER.warn("There is no ltwa-list.mv. We cannot load the LTWA repository.");
return null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to make it throw instead of null and let the public method take care of that.

@calixtus calixtus changed the title Fix for issue 12273 Add support for LTWA (issue #12276) Apr 3, 2025
@Yubo-Cao
Copy link
Collaborator Author

Yubo-Cao commented Apr 5, 2025

I have updated the implementation to use Antlr. I am not sure if this issue is reproducible: whenever I tried to use the Antlr-generated source to develop on my local machine after the initial gradlew clean build, the build failed with undefined symbol errors.

Yubo-Cao added 3 commits April 5, 2025 21:48
…r-issue-12273 # Conflicts: #	src/main/java/org/jabref/cli/LtwaListMvGenerator.java #	src/main/java/org/jabref/gui/journals/UndoableAbbreviator.java #	src/main/java/org/jabref/logic/journals/ltwa/LtwaRepository.java #	src/main/java/org/jabref/logic/journals/ltwa/LtwaTsvParser.java #	src/main/java/org/jabref/logic/journals/ltwa/NormalizeUtils.java #	src/test/java/org/jabref/logic/journals/LtwaRepositoryTest.java
}

SearchState state = new SearchState(node, index);
if (visited.contains(state)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure where I nest the logic inside the "else" branches; this method is recursive.

try (InputStream resourceAsStream = JournalAbbreviationRepository.class.getResourceAsStream("/journals/ltwa-list.mv")) {
if (resourceAsStream == null) {
LOGGER.warn("There is no ltwa-list.mv. We cannot load the LTWA repository.");
return null;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to make it throw instead of null and let the public method take care of that.

var languageStr = matcher.group(3);

word = NormalizeUtils.normalize(ANNOTATION.matcher(word).replaceAll("").strip());
var abbreviation = abbreviationStr.equals("n.a.") ? null : abbreviationStr;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to acknowledge that it's not possible to serialize Optional, and I figured it would be unwise to store an extra field called isAbbreviationPresent and store something arbitary in the actual abbreviation. As such, I left it null.

Siedlerchr
Siedlerchr previously approved these changes Apr 7, 2025
Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Impressive! Works

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 7, 2025
Copy link

trag-bot bot commented Apr 7, 2025

@trag-bot didn't find any issues in the code! ✅✨

@Siedlerchr Siedlerchr enabled auto-merge April 8, 2025 18:43
@Siedlerchr Siedlerchr added this pull request to the merge queue Apr 8, 2025
Merged via the queue into JabRef:main with commit 1a4e1f3 Apr 8, 2025
1 check passed
zikunz added a commit to zikunz/jabref that referenced this pull request Apr 18, 2025
Integrate journal abbreviation toggle functionality (JabRef#12880) with the LTWA repository support from main branch. Resolve conflicts in JournalAbbreviationRepository, JournalAbbreviationLoader, and MainMenu to ensure both features work correctly together. The combined functionality allows users to enable/disable specific journal abbreviation sources while maintaining LTWA abbreviation support.
@subhramit subhramit mentioned this pull request Apr 21, 2025
3 tasks
krishnagjsForGit pushed a commit to krishnagjsForGit/jabref that referenced this pull request May 2, 2025
* Implement LTWA abbreviation * Create LTWA resource download in the gradle tasks * Connected the LTWA logic with the GUI * Updated the CHANGELOG.md * Fix according to Trag * Reimplement with Antlr * Fix errors from previous PR * Use Optional as Trag suggested * Fix the locale translation issue --------- Co-authored-by: Christoph <siedlerkiller@gmail.com>
krishnagjsForGit pushed a commit to krishnagjsForGit/jabref that referenced this pull request May 2, 2025
* Implement LTWA abbreviation * Create LTWA resource download in the gradle tasks * Connected the LTWA logic with the GUI * Updated the CHANGELOG.md * Fix according to Trag * Reimplement with Antlr * Fix errors from previous PR * Use Optional as Trag suggested * Fix the locale translation issue --------- Co-authored-by: Christoph <siedlerkiller@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

4 participants