Add toggle functionality for journal abbreviation lists

[zikunz]

Toggle Functionality for Journal Abbreviation Lists

Closes #12468.

Documentation repo's issue can be found at Jabref/user-documentation#560

What

This feature is comprehensive and solves more than what is asked in #12468.

Specifially, this PR adds the ability to enable/disable any journal abbreviation lists in JabRef, including both the built-in list and external CSV files. Users can now toggle all types of journal abbreviation lists on/off to make them enabled/disabled.

We can disable and re-enable the built-in list:

If the built-in list is disabled, journal names found in the built-in list will not be abbreviated or unabbreviated:

In addition, external CSV files can be disabled or enabled. Likewise, if an external CSV file is disabled, journal names found in the external CSV file will not be abbreviated or unabbreviated too.

Why

In the original codebase, all journal abbreviation lists were always active once loaded. Users had no way to temporarily disable specific abbreviation sources without completely removing them. This was particularly problematic with the built-in list, which could not be removed at all. Detailed reasons about why we want to disable built-in list sometimes can be found in #12468.

This feature improves workflow flexibility by allowing users to:

• Temporarily enable/disable specific sources during editing sessions
• Control which lists are active without removing them

How

The implementation follows JabRef's existing architecture patterns

Key Features

Filtering Logic

• Only enabled abbreviation sources will apply changes during operations
• Disabled abbreviation sources have no effect on journal name processing
• The repository dynamically filters abbreviations based on source enabled state

Persistence

• Toggle states are stored in preferences only upon clicking "Save"
• If the user clicks "Cancel" or closes the dialog (X), previous states are restored
• States persist between application restarts

UI Feedback

• Visual indicators show enabled/disabled state for each list
• Toggle button provides easy access to state change
• Dropdown shows current state with clear visual indicators

Design Considerations

Performance Considerations

The implementation reloads the journal repository before each abbreviation operation to ensure it uses the latest toggle states. This approach has minimal performance impact because:

1. Repository loading is very efficient as it only builds active abbreviation sets
2. Abbreviation operations are typically infrequent user actions
3. The benefits of having accurate, up-to-date toggle states outweigh the small overhead of reloading

I have also tested with fairly large abbreviation lists and observed no noticeable performance degradation.

Design Alternatives Considered

1. Event-based updates: I considered using an event system to update the repository when toggle states change. While potentially more efficient, this approach would be more complex and error-prone, requiring careful synchronization between preference changes and repository state.

2. Runtime filtering: Another approach was to keep all abbreviations loaded and filter at runtime during abbreviation operations. This would be slightly faster but would require more memory and complicate the core lookup methods.

3. Separate repositories: I also considered maintaining separate repositories for each source, but this would diverge significantly from JabRef's existing architecture and complicate cross-source operations.

The chosen approach of reloading the repository provides the best balance of reliability, maintainability, and adherence to existing architecture patterns.

Proposed Code Changes

Frontend Changes (GUI)

1. JournalAbbreviationsTab.java

   • Purpose: User interface tab for managing journal abbreviations
   • Changes:
     • Added toggle button next to journal list dropdown
     • Created custom ListCell displaying checkmarks (✓) or circles (○) to indicate enabled state
     • Implemented real-time UI refresh when toggling lists
   • Why: Provides the visual interface for users to see and change enabled states

2. AbbreviationsFileViewModel.java

   • Purpose: View model for a single abbreviation file
   • Changes:
     • Added enabledProperty to track and bind state
     • Implemented getter/setter methods for state access
     • Added path handling for consistent preference storage
   • Why: Provides the model binding for UI components to reflect enabled state changes

3. JournalAbbreviationsTabViewModel.java

   • Purpose: Manages the journal abbreviation tab's business logic
   • Changes:
     • Implemented state tracking with proper model update notification
     • Added event handling for toggle button click
     • Added methods to build file list with correct initial states
     • Connected UI state to persistence layer
   • Why: Connects the UI actions to the model and ensures proper state synchronization

4. MainMenu.java

   • Purpose: Builds the application's main menu
   • Changes:
     • Updated constructor calls to accommodate new repository loading mechanism
   • Why: Required to accommodate the new repository loading approach in AbbreviateAction

Backend Changes (Logic)

5. JournalAbbreviationRepository.java

   • Purpose: Central repository that stores and retrieves abbreviations
   • Changes:
     • Added source tracking with sourceToAbbreviations map to associate abbreviations with their sources
     • Added enabledSources map to track enabled/disabled state of each source
     • Modified core lookup methods to filter based on enabled state
     • Implemented proper updating of active abbreviations when toggling
   • Why: This is the core component where abbreviation filtering happens. When a source is disabled, its abbreviations shouldn't be used in operations

6. JournalAbbreviationLoader.java

   • Purpose: Loads abbreviations from various sources into the repository
   • Changes:
     • Modified to respect enabled states from preferences
     • Implemented consistent key naming for external list identification
     • Added robust error handling for missing files
   • Why: The loader needed to initialize the repository with the correct enabled states and ensure proper source identification

7. AbbreviateAction.java

   • Purpose: Handles abbreviation/unabbreviation commands from menu
   • Changes:
     • Implemented repository reload before abbreviation operations
     • Ensured operations use the latest toggle states
   • Why: Without reloading the repository, toggle state changes wouldn't affect already-running abbreviation operations

8. JournalAbbreviationPreferences.java

   • Purpose: Stores user preferences for journal abbreviations
   • Changes:
     • Added enabledExternalLists map to store enabled/disabled states
     • Added methods to check if a specific source has an explicit enabled setting
     • Enhanced preference retrieval with multiple lookup strategies
   • Why: Provides the persistence mechanism for toggle states between application sessions

9. JabRefCliPreferences.java

   • Purpose: Handles preference loading/saving at application level
   • Changes:
     • Added settings for journal abbreviation toggle states
     • Integrated with the overall preference system
   • Why: Ensures toggle states are properly saved/loaded at application level

Testing Strategy

Test Files and Coverage

1. JournalAbbreviationRepositoryTest.java

   • Purpose: Tests the core backend functionality for source tracking and toggle behavior
   • Test Cases:
     • multipleSourcesCanBeToggled: Verifies that multiple sources can be independently enabled/disabled
     • disabledSourcesAreFilteredFromLookup: Ensures abbreviations from disabled sources aren't returned in lookups
     • builtInListCanBeToggled: Confirms the built-in list can be toggled like external sources
     • toggleStateAffectsAbbreviationSets: Tests that abbreviation sets are properly filtered by source state

2. JournalAbbreviationsViewModelTabTest.java

   • Purpose: Tests the UI view model for toggle functionality
   • Test Cases:
     • addBuiltInListInitializesWithCorrectEnabledState: Verifies built-in list loaded with correct enabled state
     • enabledExternalListFiltersAbbreviationsWhenDisabled: Tests that UI reflects filtered abbreviations
     • storeSettingsPersistsEnabledStateToPreferences: Ensures toggle states are saved to preferences

Implementation Additions for Testing

The following methods and properties were added specifically to support testing:

1. AbbreviationsFileViewModel.java

   • Added refreshAbbreviations() method to simulate UI updates when toggle state changes
   • Added thorough isEnabled(), setEnabled(), and enabledProperty() methods for test validation

2. JournalAbbreviationsTabViewModel.java

   • Added markAsDirty() method to allow tests to mark files as needing to be saved
   • Enhanced storeSettings() to provide verification points for proper state persistence

3. JournalAbbreviationRepository.java

   • Implemented addCustomAbbreviation(abbreviation, sourcePath, enabled) with source tracking for test scenarios
   • Added test-friendly mapping between abbreviations and their sources for verification

4. JournalAbbreviationPreferences.java

   • Added hasExplicitEnabledSetting() method to improve testability of preference storage
   • Implemented clean getters/setters for toggle states to simplify test verification

These additions ensure the toggle functionality can be thoroughly tested while maintaining clean separation between the application code and test code.

Mandatory checks

• [x] I own the copyright of the code submitted and I license it under the MIT license
• [x] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
• [x] Tests created for changes (if applicable)
• [x] Manually tested changed features in running JabRef (always required)
• [x] Screenshots added in PR description (if change is visible to the user)
• [x] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
• [x] 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.

[subhramit]

Please update your screenshots here as well.

[zikunz]

@subhramit Got it, I will make sure all the screenshots will be updated in the PR content above :)

[zikunz]

I tried to fix the two screenshots in the PR content, may I ask whether you are able to see them? @subhramit

[subhramit]

I tried to fix the two screenshots in the PR content, may I ask whether you are able to see them? @subhramit

yes, they are fine now

[zikunz]

Thank you! @subhramit

[zikunz]

Update: the bug is now fixed and I proceed to refactor the code now. Afterwards, this PR will be ready for review :)

[jabref-machine]

Your pull request conflicts with the target branch.

Please merge upstream/main with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line.

[jabref-machine]

Your pull request conflicts with the target branch.

Please merge upstream/main with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line.

[zikunz]

@koppor @Siedlerchr @subhramit

I have completed resolving the 6 merge conflicts between my PR and PR #12880 (which closed #12273). The resolved changes are reflected in my recent commit with the commit message of "feat: resolve merge conflicts with journal abbreviation toggle feature".

To properly test the combined functionality, I first proceeded to git checkout 1a4e1f38864e9bb1fc336a366ec24c3c0d48a94f in main branch to understand the behavior after merging #12880 to main (without my code changes).

During my local testing of the merged functionality, I observed some behavior that I would like to confirm is working as intended:

1. When testing with the LTWA abbreviation mode, I noticed that the word "graph" does not get abbreviated to "gr." despite the csv file found in #12273 containing the entry -graph-;"-gr.";"eng".

2. I can successfully abbreviate "Journal of Polymer Science Part A" to "J. Polym. Sci. A" using LTWA mode (matching a test case in #12273), but when I try to unabbreviate it, it does not revert to the original text. In addition, "Journal of Polymer Science Part A" does not seem to be found in the the csv file found in #12273.

3. When attempting to abbreviate "Dem" (this is achieved by first abbreviating "Demonstration" using the shortest unique mode) using LTWA mode, the result is an empty string.

I want to ensure that my merge conflict resolution is correct and that I am understanding the expected behavior of the LTWA abbreviation functionality properly. Could you please advise if these observations align with the intended functionality of the LTWA implementation, or if further adjustments are needed?

Thank you for your guidance.

[koppor]

@Yubo-Cao Can you look into https://github.com/JabRef/jabref/pull/12912#issuecomment-2816039611 maybe? I currently don't have the resources to properly answer.

[zikunz]

@Yubo-Cao Can you look into #12912 (comment) maybe? I currently don't have the resources to properly answer.

@koppor Thank you!

@Yubo-Cao I look forward to your guidance. Thank you in advance!

[Yubo-Cao]

@Yubo-Cao Can you look into #12912 (comment) maybe? I currently don't have the resources to properly answer.

@koppor Thank you!

@Yubo-Cao I look forward to your guidance. Thank you in advance!

Apologies for the late reply. The behavior you found is as expected:

1. When there is only one or two words in the journal title, LTWA suggests doing nothing with it, since it is typically a proper noun (e.g., The Lancet).
2. In any other cases, we would break down the title into subsequences and match them with the LTWA list to perform the abbreviation. For example, Journal of Polymer Science Part A became [Journal] [stopwods are dropped] [Polymer] [Science] [A].
3. Dem is recognized by the parser as a German article—currently, that has to be the expected behavior since we couldn't infer the language for which the journal belongs to.
4. Unabbreviate is effectively impossible. Since LTWA removes all the stopwords, we couldn't magically reproduce of from the abbreviated version either. I thought the fjournal field would take care of that?

[zikunz]

@Yubo-Cao Can you look into #12912 (comment) maybe? I currently don't have the resources to properly answer.

@koppor Thank you! @Yubo-Cao I look forward to your guidance. Thank you in advance!

Apologies for the late reply. The behavior you found is as expected:

1. When there is only one or two words in the journal title, LTWA suggests doing nothing with it, since it is typically a proper noun (e.g., The Lancet).
2. In any other cases, we would break down the title into subsequences and match them with the LTWA list to perform the abbreviation. For example, Journal of Polymer Science Part A became [Journal] [stopwods are dropped] [Polymer] [Science] [A].
3. Dem is recognized by the parser as a German article—currently, that has to be the expected behavior since we couldn't infer the language for which the journal belongs to.
4. Unabbreviate is effectively impossible. Since LTWA removes all the stopwords, we couldn't magically reproduce of from the abbreviated version either. I thought the fjournal field would take care of that?

Thank you so much for your reply! @Yubo-Cao I apologize for not noticing your response until just now.

I am very thankful for your confirmation that all the behaviors I described are expected. This is reassuring that my resolution of the merge conflicts appears to be on the right track because there is no change in LTWA functionalities you implemented after adding toggle functionality (I will also thoroughly double-check it later on again). Thank you also for explaining the detailed business logic, design choices and implementation details - this has been extremely helpful.

@koppor All my questions were answered, I will make this PR ready for review soon. I will update everyone in #12468 :)

[zikunz]

The changes so far are looking good. How much more work do you estimate it'll take to get to a "ready" state?

See if you can get rid of Collectors and Collections wherever possible. I have left some hints in comments below and some miscellaneous code suggestions.

@subhramit Thank you so much for your thorough code review and feedback. I estimate I will need approximately 1 - 2 additional days to bring this PR to a "ready" state. Here is what remains on my task list (please let me know if I still miss any):

1. Code Refactoring (most significant effort)

   • Address all remaining review comments by trag-bot
   • Implement your suggestions regarding Collectors and Collections alternatives
   • Conduct a final quality review to identify additional improvements

2. PR Documentation Updates

   • Enhance the description with clear explanations of:
     • Modified files and rationale for changes
     • Implementation approach
     • Key design decisions and alternatives considered
     • ...

3. Documentation Repo Updates

   • Open a corresponding issue in the documentation repo
   • Submit a PR with necessary documentation changes

I am really thankful for your guidance on reducing dependencies on Collectors and Collections - I will incorporate your suggestions in my upcoming changes. The hints you have provided in the comments are especially helpful.

[zikunz]

The changes so far are looking good. How much more work do you estimate it'll take to get to a "ready" state? See if you can get rid of Collectors and Collections wherever possible. I have left some hints in comments below and some miscellaneous code suggestions.

@subhramit Thank you so much for your thorough code review and feedback. I estimate I will need approximately 1 - 2 additional days to bring this PR to a "ready" state. Here is what remains on my task list (please let me know if I still miss any):

1. Code Refactoring (most significant effort)

   • Address all remaining review comments by trag-bot
   • Implement your suggestions regarding Collectors and Collections alternatives
   • Conduct a final quality review to identify additional improvements

2. PR Documentation Updates

   • Enhance the description with clear explanations of:

     • Modified files and rationale for changes
     • Implementation approach
     • Key design decisions and alternatives considered
     • ...

3. Documentation Repo Updates

   • Open a corresponding issue in the documentation repo
   • Submit a PR with necessary documentation changes

I am really thankful for your guidance on reducing dependencies on Collectors and Collections - I will incorporate your suggestions in my upcoming changes. The hints you have provided in the comments are especially helpful.

I just updated the PR content and opened an issue in the documentation repo. I addressed all trag-bot comments and now working on resolving all comments from @subhramit's code review. The refactoring might take some time.

[zikunz]

All checks have passed and no more trag-bot comments for the time being, I will proceed to address the remaining comments @subhramit raised during code review. Please feel free to add more comments for the code changes.

[subhramit]

@zikunz note that not all tragbot comments are relevant. E.g. if you are dealing with pure bollean functions, assertTrue and assertFalse is alright to use.

[subhramit]

Could you also answer https://github.com/JabRef/jabref/pull/12912#discussion_r2052392686

[zikunz]

@zikunz note that not all tragbot comments are relevant. E.g. if you are dealing with pure bollean functions, assertTrue and assertFalse is alright to use.

Thank you for your guidance! Upon further reflection, I realized that trag-bot gave contradictory feedback where it first requested using assertTrue/assertFalse then started asking to use assertEqual. Once I recognized this inconsistency, I decided to halt any further modifications.

[zikunz]

Could you also answer #12912 (comment)

Yes sure, please give me some time, I will quote the comment and answer it :)

[zikunz]

Could you also answer #12912 (comment)

Yes sure, please give me some time, I will quote the comment and answer it :)

Thank you for raising this important design question which I did not consider. The current approach is suboptimal. Creating a new repository for every unabbreviate operation is inefficient and could potentially impact performance, especially with multiple external abbreviation lists.

On repository creation

I initially implemented it this way to ensure the operation always uses the latest toggle states, but there is definitely a better approach. I now propose implementing a caching mechanism through a JournalAbbreviationRepositoryManager that would:

• Cache the repository instance
• Only rebuild when preferences actually change
• Provide thread-safe access to the repository

This would maintain the same functionality while eliminating unnecessary repository recreation.

On preferences dependency

Regarding whether unabbreviate should depend on preferences at all, I think the answer is yes, it should, but indirectly. The unabbreviate operation needs to know which abbreviation lists are active, which is determined by user preferences.

However, I agree the current direct dependency is not ideal architecturally. A better approach would be:

1. Have the repository depend on preferences (it needs to know which sources are enabled)
2. Have the unabbreviate operation depend only on the repository
3. Use dependency injection to connect these components

This maintains the same functionality (enabling / disabling abbreviation sources) while better separating concerns.

I just implemented this repository manager approach in this PR. I also addressed all the previous comments you left. Could you please advise me what to do now? Thank you so much! @subhramit

[InAnYan]

Hi, @zikunz! I think I can hop in and help Subhramit et al. with this PR.

As JabRef is primarily done in free time by volunteers, we typically write small issues and expected small/medium PRs. But sometimes we have PRs that escalate in a lot of work.

Can you tell me:

1. Do you have any blockers?
2. Do you have any questions for design choices that should be discussed with maintainers?
3. What specifically this PR solves/done/improves? So that I know what to test for.
4. What is currently in progress and (if there is) planned in future?

For 4th point -- I think it's better to postpone this

[calixtus]

Please not another Manager. Especially not a repository manager. This is silly, almost funny. The repository provides everything the application needs. If it needs to be refreshed because preferences have changed and the loading mechanics are dependent on the preferences (which they shouldn't!) the repository needs to handle this internally.

And please do not use AI to communicate with us. Speak real, use your own words. Remember our two golden rules for use of ai: never let an ai think for you, never let an ai speak for you.

[zikunz]

Hi, @zikunz! I think I can hop in and help Subhramit et al. with this PR.

As JabRef is primarily done in free time by volunteers, we typically write small issues and expected small/medium PRs. But sometimes we have PRs that escalate in a lot of work.

Can you tell me:

1. Do you have any blockers?
2. Do you have any questions for design choices that should be discussed with maintainers?
3. What specifically this PR solves/done/improves? So that I know what to test for.
4. What is currently in progress and (if there is) planned in future?

For 4th point -- I think it's better to postpone this

Hi @InAnYan, thank you so much for helping to review this PR, please find below the answers for your 3 questions:

Q: Do you have any blockers? A: I do not have any blockers at this point of time. This feature is fully implemented and all checks have passed :)

Q: Do you have any questions for design choices that should be discussed with maintainers? A: I currently do not have any questions for design choices, but there could be design choices made which are suboptimal in this PR. I can also take another look and try to see if there are better design(s) that I did not consider and does/do not cause a big, costly change in the existing codebase.

Q: What specifically this PR solves/done/improves? So that I know what to test for. A: As seen from the PR content, from the user point of view, this PR adds a toggle functionality for all journal abbreviation lists which include both built-in lists as well as external CSV files.

After pressing toggle, a list will change from enabled to disabled or change from disabled to enabled. This will be saved only if the user clicks Save, if the user clicks x or Cancel, the state change will not be saved.

After the users save enabled / disabled states for the lists, when they try to abbreviate and unabbreviate one or more journal names, only journal names found in the enabled lists will be abbreviated or unabbreviated. If a list is disabled, journal name(s) found in the disabled list cannot be abbreviated or unabbreviated.

Please let me know if anything written above is not clear. Thank you again for your help!

[zikunz]

Please not another Manager. Especially not a repository manager. This is silly, almost funny. The repository provides everything the application needs. If it needs to be refreshed because preferences have changed and the loading mechanics are dependent on the preferences (which they shouldn't!) the repository needs to handle this internally.

And please do not use AI to communicate with us. Speak real, use your own words. Remember our two golden rules for use of ai: never let an ai think for you, never let an ai speak for you.

@calixtus Thank you for your constructive feedback. I understand your concerns.

I think I complicated things unnecessarily by creating a separate manager. When you said "the repository provides everything the application needs," it then clicked that I should have simply enhanced the repository class itself.

(cc @InAnYan) The original problem I was trying to solve was the inefficient creation of new repositories on every operation:

JournalAbbreviationRepository freshRepository = JournalAbbreviationLoader.loadRepository(journalAbbreviationPreferences);

This is suboptimal because of expensive I/O (i.e., repeatedly loading data from files), memory churn (i.e., create new objects for every operation) and no caching of previously loaded repositories.

Afterwards, I created JournalAbbreviationRepositoryManager which checks if preferences are changed before rebuilding and caches the repository instance.

This is still suboptimal because:

1. now there is unnecessary and not meaningful abstraction, creating an extra layer when the repository should handle this internally
2. (A Question to Clarify) I do not understand why loading mechanics are not dependent on the preferences. I still think the repository should use preferences because it needs to know which sources are enabled. There is a high chance that I misinterpreted what you and @subhramit were hinting to me, are you saying the repository should use preferences, but the caller should not need to manage the repository's lifecycle, instead, the repository should handle that internally?

If my understanding is fully correct now, can you please advise if I should quickly do the following?

1. Take the caching logic I wrote in JournalAbbreviationRepositoryManager
2. Move it directly into the JournalAbbreviationRepository class
3. Remove JournalAbbreviationRepositoryManager

In short,

Instead of: repository = JournalAbbreviationRepositoryManager.getInstance().getRepository(preferences);

The repository should offer: repository = JournalAbbreviationRepository.getInstance(preferences);

This way, the repository itself will handle when to reload based on preference changes, without needing extra layers. Does this approach sound right? I appreciate the guidance and am eager to improve the implementation.

[calixtus]

I think I complicated things unnecessarily by creating a separate manager. When you said "the repository provides everything the application needs," it then clicked that I should have simply enhanced the repository class itself.

This is the state it should be, maybe not it is in now.

[zikunz]

I think I complicated things unnecessarily by creating a separate manager. When you said "the repository provides everything the application needs," it then clicked that I should have simply enhanced the repository class itself.

This is the state it should be, maybe not it is in now.

Thank you so much for your guidance, @calixtus. I understand the issue (i.e., the manager class adds complexity without adding value) now completely and will work on letting the repository handle it internally, following the principle of "the simplest solution that works." :)

[jabref-machine]

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 "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.

[zikunz]

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 "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.

The project has been reorganized into a multi-module structure, I will probably resolve merge conflicts later on to adapt to this new modular structure. Latest commit to attempt to resolve merge conflicts removed.

[trag-bot[bot]]

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