Tabular UI for naming linked files #12624

priyanshu16095 · 2025-03-05T15:11:31Z

Closes #11368

This PR adds a tabular construct similar to "Key patterns" for "Linked files name" in the Linked files tab in preferences.

Mandatory checks

I own the copyright of the code submitted and I licence 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 (for UI changes)
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.

priyanshu16095 · 2025-03-11T16:15:09Z

I have try to add but I am very unclear about how preference migration works.

github-actions

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.
You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

koppor · 2025-03-11T19:36:11Z

I have try to add but I am very unclear about how preference migration works.

Not sure how to help.

Example method: org.jabref.migrations.PreferencesMigrations#upgradeImportFileAndDirePatterns

It first checks if a prefernce is there. If yes, it checks the content. Then, it check whether the current content is an old content. Then it updates to the new content. Then it wirtes the new string.

Similar at org.jabref.migrations.PreferencesMigrations#upgradeKeyBindingsToJavaFX - and all other migrations.

…tabularUI

src/main/java/org/jabref/gui/linkedfile/LinkedFileNamePatternsPanel.java

src/main/java/org/jabref/logic/citationkeypattern/CitationKeyGenerator.java

src/main/java/org/jabref/logic/citationkeypattern/Pattern.java

src/main/java/org/jabref/logic/linkedfile/AbstractLinkedFileNamePatterns.java

src/main/java/org/jabref/logic/linkedfile/GlobalLinkedFileNamePatterns.java

…emModel to PatternsItemModel

…ggestionCell to PatternsSuggestionCell

trag-bot · 2025-03-21T02:25:47Z

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

trag-bot · 2025-03-21T02:35:03Z

src/main/java/org/jabref/gui/linkedfile/LinkedFileNamePatternsPanelViewModel.java

+        if ((initialNamePattern.getDefaultValue() == null) || initialNamePattern.getDefaultValue().equals(Pattern.NULL_PATTERN)) {
+            defaultPattern = "";
+        } else {
+            defaultPattern = initialNamePattern.getDefaultValue().stringRepresentation();
+        }


The code violates the fail-fast principle by nesting logic inside else branch. Should check for valid state first and return/assign early.

Maybe it is OK in this context.

trag-bot · 2025-03-21T02:36:18Z

src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTab.java

+    @Override
+    public void setValues() {
+        viewModel.setValues();
+        BibEntryTypesManager entryTypesManager = Injector.instantiateModelOrService(BibEntryTypesManager.class);


Direct instantiation through Injector in method body violates separation of concerns. The dependency should be injected through constructor or setter method.

trag-bot · 2025-03-21T02:38:22Z

src/main/java/org/jabref/logic/bibtex/comparator/MetaDataDiff.java

@@ -48,7 +48,7 @@ public static Optional<MetaDataDiff> compare(MetaData originalMetaData, MetaData
            return Optional.empty();
        } else {
            MetaDataDiff diff = new MetaDataDiff(originalMetaData, newMetaData);
-            List<Difference> differences = diff.getDifferences(new GlobalCitationKeyPatterns(CitationKeyPattern.NULL_CITATION_KEY_PATTERN));
+            List<Difference> differences = diff.getDifferences(new GlobalCitationKeyPatterns(Pattern.NULL_PATTERN));


Same GlobalCitationKeyPatterns object is created twice in the class - in compare() and toString(). This violates DRY principle and creates redundant object allocations.

trag-bot · 2025-03-21T02:38:58Z

src/main/java/org/jabref/logic/citationkeypattern/AbstractCitationKeyPatterns.java

@@ -14,12 +14,12 @@
 */
 public abstract class AbstractCitationKeyPatterns {

-    protected CitationKeyPattern defaultPattern = CitationKeyPattern.NULL_CITATION_KEY_PATTERN;
+    protected Pattern defaultPattern = Pattern.NULL_PATTERN;


Public methods should not return null values. Consider using Optional instead of allowing null patterns, even for internal representation.

trag-bot · 2025-03-21T02:42:40Z

src/main/java/org/jabref/logic/citationkeypattern/Pattern.java

+    public static List<Pattern> getAllPatterns() {
+        return Arrays.stream(Pattern.class.getDeclaredFields())


The method lacks JavaDoc despite being a complex public method that uses reflection and handles exceptions. Documentation would help explain its purpose and behavior.

trag-bot · 2025-03-21T02:44:34Z

src/main/java/org/jabref/logic/linkedfile/AbstractLinkedFileNamePatterns.java

+    }
+
+    /**
+     * Gets an object for a desired key from this LinkedFileNamePattern or one of it's parents (in the case of


JavaDoc contains grammatical error ('it's' should be 'its') and references Hashtable in description while code uses HashMap, making documentation misleading.

trag-bot · 2025-03-21T02:45:02Z

src/main/java/org/jabref/logic/linkedfile/GlobalLinkedFileNamePatterns.java

+    }
+
+    public static GlobalLinkedFileNamePatterns fromPattern(String pattern) {
+        return new GlobalLinkedFileNamePatterns(new Pattern(pattern));


Missing validation for the pattern parameter before creating new Pattern object. Input parameters should be validated to fail fast.

trag-bot · 2025-03-21T02:47:06Z

src/main/java/org/jabref/migrations/PreferencesMigrations.java

+        if (preferenceFileNamePattern != null) {
+            mainPrefsNode.put(JabRefCliPreferences.LINKED_FILE_NAME_PATTERNS_NODE, "[auth_year]");
+            if (prefs.hasKey(JabRefCliPreferences.IMPORT_FILENAMEPATTERN)) {


The code violates the fail-fast principle by nesting logic inside conditional blocks instead of returning early. The nested if condition should be restructured.

trag-bot · 2025-03-21T02:47:08Z

src/main/java/org/jabref/migrations/PreferencesMigrations.java

@@ -68,6 +68,7 @@ public static void runMigrations(JabRefGuiPreferences preferences) {
        upgradeCleanups(preferences);
        moveApiKeysToKeyring(preferences);
        removeCommentsFromCustomEditorTabs(preferences);
+        upgradeFileImportPatternToLinkedFileNamePattern(preferences, mainPrefsNode);


The method lacks proper JavaDoc documentation explaining the migration process, parameters, and potential side effects, which is important for complex migration operations.

priyanshu16095 · 2025-03-21T04:07:25Z

To remove code duplication ,
Renamed LinkedFileNamePattern and CitationKeyPattern to Pattern
Renamed LinkedFileNamePatternsPanelItemModel and CitationKeyPatternsItemModel to PatternsItemModel
Renamed LinkedFileNamePatternsSuggestionCell and CitationKeyPatternsSuggestionCell to PatternsSuggestionCell

The thing I am not sure about is whether the preference migration is correct or not.

priyanshu16095 added 26 commits February 13, 2025 03:42

Add fix options for integrity issues

9b11edf

Fix the check

02d8e75

Add a missing entry for Localization.lang

754b6ad

Fix the check

696f904

Refactor code

4ab36a8

Fix the check

28de796

Add a CHANGELOG.md entry

70ae8eb

Refactor the code

77df6ea

Make use of IntegrityIssue enum

30c81ce

Add keys in Localization.lang

5b389da

Refactor the code

3a0d9a1

Remove use of enum

a04e06a

Remove use of enum from PageChecker class

8e9341a

Add fixAll method

235c74f

Add a check for fixAll and fixByType method

9839137

Fix a bug

5754680

Use dialogService.notify() to notify

8f7bdc2

Remove unused imports

f57719c

Merge branch 'main' into integrityCheckActions

2105a13

Merge branch 'main' into integrityCheckActions

e37ba79

Add tests for methods

139777e

Fix the check and updates based on review

39dbfd1

Use enum for passing issue message

16e9817

Fix the checkstyle check

102e789

Add the gui

92d91b5

Add files in logic directory

b726f30

priyanshu16095 changed the title ~~Tabular UI~~ Tabular UI for naming linked files Mar 5, 2025

priyanshu16095 added 3 commits March 5, 2025 20:59

Revert changes in three files

86518fc

Revert some more files

6f985ec

Revert some more files

5ea4754

github-actions bot reviewed Mar 11, 2025

View reviewed changes

Merge branch 'main' of https://github.com/priyanshu16095/jabref into …

88c9e4f

…tabularUI

priyanshu16095 marked this pull request as draft March 20, 2025 01:52

priyanshu16095 added 2 commits March 21, 2025 06:47

Rename LinkedFileNamePattern and CitationKeyPattern to Pattern

de46be4

Merge branch 'main' into tabularUI

279ce8f

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

src/main/java/org/jabref/gui/linkedfile/LinkedFileNamePatternsPanel.java Show resolved Hide resolved

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

src/main/java/org/jabref/logic/citationkeypattern/CitationKeyGenerator.java Show resolved Hide resolved

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

src/main/java/org/jabref/logic/citationkeypattern/Pattern.java Show resolved Hide resolved

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

src/main/java/org/jabref/logic/linkedfile/AbstractLinkedFileNamePatterns.java Show resolved Hide resolved

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

src/main/java/org/jabref/logic/linkedfile/AbstractLinkedFileNamePatterns.java Show resolved Hide resolved

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

src/main/java/org/jabref/logic/linkedfile/GlobalLinkedFileNamePatterns.java Show resolved Hide resolved

priyanshu16095 added 5 commits March 21, 2025 07:03

Renmae LinkedFileNamePatternsPanelItemModel and CitationKeyPatternsIt…

a3383a2

…emModel to PatternsItemModel

Rename LinkedFileNamePatternsSuggestionCell and CitationKeyPatternsSu…

967a4a5

…ggestionCell to PatternsSuggestionCell

Fix the checkstyle check

1261731

Remove the redundant check

965d4df

Fix the check

a0a00ef

priyanshu16095 marked this pull request as ready for review March 21, 2025 02:29

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tabular UI for naming linked files #12624

Tabular UI for naming linked files #12624

priyanshu16095 commented Mar 5, 2025 •

edited

Loading

priyanshu16095 commented Mar 11, 2025

github-actions bot left a comment

koppor commented Mar 11, 2025

trag-bot bot commented Mar 21, 2025

trag-bot bot Mar 21, 2025

koppor Mar 21, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

priyanshu16095 commented Mar 21, 2025

		public static List<Pattern> getAllPatterns() {
		return Arrays.stream(Pattern.class.getDeclaredFields())

Tabular UI for naming linked files #12624

Are you sure you want to change the base?

Tabular UI for naming linked files #12624

Conversation

priyanshu16095 commented Mar 5, 2025 • edited Loading

Closes #11368

Mandatory checks

priyanshu16095 commented Mar 11, 2025

github-actions bot left a comment

Choose a reason for hiding this comment

koppor commented Mar 11, 2025

trag-bot bot commented Mar 21, 2025

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

koppor Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

priyanshu16095 commented Mar 21, 2025

priyanshu16095 commented Mar 5, 2025 •

edited

Loading