Fix Trag-bot review issues

alexsukhin · alexsukhin · commit 033201c6e427 · 2025-09-11T01:35:29.000+01:00
- Removed trivial comments
- Renamed PDF-related variables
- Updated methods to return Optional
- Used Optional property for FieldFormatterCleanups
diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java
@@ -20,7 +20,6 @@ public class CleanupDialog extends BaseDialog<CleanupPreferences> {
     public CleanupDialog(BibDatabaseContext databaseContext, CleanupPreferences initialPreset, FilePreferences filePreferences) {
         setTitle(Localization.lang("Clean up entries"));
 
-        // Load FXML
         ViewLoader.view(this)
                   .load()
                   .setAsDialogPane(this);
@@ -39,7 +38,7 @@ public CleanupDialog(BibDatabaseContext databaseContext, CleanupPreferences init
             if (button.getButtonData() == ButtonBar.ButtonData.OK_DONE) {
                 Tab selectedTab = tabPane.getSelectionModel().getSelectedItem();
                 CleanupPanel panel = (CleanupPanel) selectedTab.getContent();
-                return panel.getCleanupPreferences();
+                return panel.getCleanupPreferences().orElse(null);
             } else {
                 return null;
             }
diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java
@@ -2,6 +2,7 @@
 
 import java.nio.file.Path;
 import java.util.EnumSet;
+import java.util.Objects;
 import java.util.Optional;
 
 import javafx.fxml.FXML;
@@ -19,17 +20,18 @@
 
 public class CleanupFileRelatedPanel extends VBox implements CleanupPanel {
 
-    @FXML private Label cleanupRenamePDFLabel;
+    @FXML private Label cleanupRenamePdfLabel;
 
-    @FXML private CheckBox cleanUpMovePDF;
+    @FXML private CheckBox cleanUpMovePdf;
     @FXML private CheckBox cleanUpMakePathsRelative;
-    @FXML private CheckBox cleanUpRenamePDF;
-    @FXML private CheckBox cleanUpRenamePDFonlyRelativePaths;
+    @FXML private CheckBox cleanUpRenamePdf;
+    @FXML private CheckBox cleanUpRenamePdfonlyRelativePaths;
     @FXML private CheckBox cleanUpDeletedFiles;
     @FXML private CheckBox cleanUpUpgradeExternalLinks;
 
     public CleanupFileRelatedPanel(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences, FilePreferences filePreferences) {
-        // Load FXML
+        Objects.requireNonNull(cleanupPreferences, "cleanupPreferences must not be null");
+
         ViewLoader.view(this)
                   .root(this)
                   .load();
@@ -40,45 +42,45 @@ public CleanupFileRelatedPanel(BibDatabaseContext databaseContext, CleanupPrefer
     private void init(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences, FilePreferences filePreferences) {
         Optional<Path> firstExistingDir = databaseContext.getFirstExistingFileDir(filePreferences);
         if (firstExistingDir.isPresent()) {
-            cleanUpMovePDF.setText(Localization.lang("Move linked files to default file directory %0", firstExistingDir.get().toString()));
+            cleanUpMovePdf.setText(Localization.lang("Move linked files to default file directory %0", firstExistingDir.get().toString()));
         } else {
-            cleanUpMovePDF.setText(Localization.lang("Move linked files to default file directory %0", "..."));
+            cleanUpMovePdf.setText(Localization.lang("Move linked files to default file directory %0", "..."));
 
             // Since the directory does not exist, we cannot move it to there. So, this option is not checked - regardless of the presets stored in the preferences.
-            cleanUpMovePDF.setDisable(true);
-            cleanUpMovePDF.setSelected(false);
+            cleanUpMovePdf.setDisable(true);
+            cleanUpMovePdf.setSelected(false);
         }
 
-        cleanUpRenamePDFonlyRelativePaths.disableProperty().bind(cleanUpRenamePDF.selectedProperty().not());
+        cleanUpRenamePdfonlyRelativePaths.disableProperty().bind(cleanUpRenamePdf.selectedProperty().not());
 
         cleanUpUpgradeExternalLinks.setText(Localization.lang("Upgrade external PDF/PS links to use the '%0' field.", StandardField.FILE.getDisplayName()));
 
         String currentPattern = Localization.lang("Filename format pattern (from preferences)")
                                             .concat(filePreferences.getFileNamePattern());
-        cleanupRenamePDFLabel.setText(currentPattern);
+        cleanupRenamePdfLabel.setText(currentPattern);
 
         updateDisplay(cleanupPreferences);
     }
 
     private void updateDisplay(CleanupPreferences preset) {
-        if (!cleanUpMovePDF.isDisabled()) {
-            cleanUpMovePDF.setSelected(preset.isActive(CleanupPreferences.CleanupStep.MOVE_PDF));
+        if (!cleanUpMovePdf.isDisabled()) {
+            cleanUpMovePdf.setSelected(preset.isActive(CleanupPreferences.CleanupStep.MOVE_PDF));
         }
         cleanUpMakePathsRelative.setSelected(preset.isActive(CleanupPreferences.CleanupStep.MAKE_PATHS_RELATIVE));
-        cleanUpRenamePDF.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF));
-        cleanUpRenamePDFonlyRelativePaths.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF_ONLY_RELATIVE_PATHS));
+        cleanUpRenamePdf.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF));
+        cleanUpRenamePdfonlyRelativePaths.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF_ONLY_RELATIVE_PATHS));
         cleanUpUpgradeExternalLinks.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_UPGRADE_EXTERNAL_LINKS));
         cleanUpDeletedFiles.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES));
     }
 
-    public CleanupPreferences getCleanupPreferences() {
+    public Optional<CleanupPreferences> getCleanupPreferences() {
         EnumSet<CleanupPreferences.CleanupStep> activeJobs = EnumSet.noneOf(CleanupPreferences.CleanupStep.class);
 
         if (cleanUpMakePathsRelative.isSelected()) {
             activeJobs.add(CleanupPreferences.CleanupStep.MAKE_PATHS_RELATIVE);
         }
-        if (cleanUpRenamePDF.isSelected()) {
-            if (cleanUpRenamePDFonlyRelativePaths.isSelected()) {
+        if (cleanUpRenamePdf.isSelected()) {
+            if (cleanUpRenamePdfonlyRelativePaths.isSelected()) {
                 activeJobs.add(CleanupPreferences.CleanupStep.RENAME_PDF_ONLY_RELATIVE_PATHS);
             } else {
                 activeJobs.add(CleanupPreferences.CleanupStep.RENAME_PDF);
@@ -91,6 +93,6 @@ public CleanupPreferences getCleanupPreferences() {
             activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES);
         }
 
-        return new CleanupPreferences(activeJobs);
+        return Optional.of(new CleanupPreferences(activeJobs));
     }
 }
diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java
@@ -1,6 +1,8 @@
 package org.jabref.gui.cleanup;
 
 import java.util.EnumSet;
+import java.util.Objects;
+import java.util.Optional;
 
 import javafx.fxml.FXML;
 import javafx.scene.control.CheckBox;
@@ -20,7 +22,8 @@ public class CleanupMultiFieldPanel extends VBox implements CleanupPanel {
     @FXML private CheckBox cleanUpTimestampToModificationDate;
 
     public CleanupMultiFieldPanel(CleanupPreferences cleanupPreferences) {
-        // Load FXML
+        Objects.requireNonNull(cleanupPreferences, "cleanupPreferences must not be null");
+
         ViewLoader.view(this)
                   .root(this)
                   .load();
@@ -69,7 +72,7 @@ private void updateDisplay(CleanupPreferences preset) {
     }
 
     @Override
-    public CleanupPreferences getCleanupPreferences() {
+    public Optional<CleanupPreferences> getCleanupPreferences() {
         EnumSet<CleanupPreferences.CleanupStep> activeJobs = EnumSet.noneOf(CleanupPreferences.CleanupStep.class);
 
         if (cleanUpDOI.isSelected()) {
@@ -94,6 +97,6 @@ public CleanupPreferences getCleanupPreferences() {
             activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_MODIFICATIONDATE);
         }
 
-        return new CleanupPreferences(activeJobs);
+        return Optional.of(new CleanupPreferences(activeJobs));
     }
 }
diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java
@@ -1,7 +1,9 @@
 package org.jabref.gui.cleanup;
 
+import java.util.Optional;
+
 import org.jabref.logic.cleanup.CleanupPreferences;
 
 public interface CleanupPanel {
-    CleanupPreferences getCleanupPreferences();
+    Optional<CleanupPreferences> getCleanupPreferences();
 }
diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java
@@ -1,5 +1,8 @@
 package org.jabref.gui.cleanup;
 
+import java.util.Objects;
+import java.util.Optional;
+
 import javafx.collections.FXCollections;
 import javafx.fxml.FXML;
 import javafx.scene.layout.VBox;
@@ -15,6 +18,8 @@ public class CleanupSingleFieldPanel extends VBox implements CleanupPanel {
     @FXML private FieldFormatterCleanupsPanel formatterCleanupsPanel;
 
     public CleanupSingleFieldPanel(CleanupPreferences cleanupPreferences) {
+        Objects.requireNonNull(cleanupPreferences, "cleanupPreferences must not be null");
+
         ViewLoader.view(this)
                   .root(this)
                   .load();
@@ -30,11 +35,11 @@ private void init(CleanupPreferences cleanupPreferences) {
     }
 
     @Override
-    public CleanupPreferences getCleanupPreferences() {
+    public Optional<CleanupPreferences> getCleanupPreferences() {
         FieldFormatterCleanups fieldFormatterCleanups = new FieldFormatterCleanups(
                 !formatterCleanupsPanel.cleanupsDisableProperty().getValue(),
                 formatterCleanupsPanel.cleanupsProperty()
         );
-        return new CleanupPreferences(fieldFormatterCleanups);
+        return Optional.of(new CleanupPreferences(fieldFormatterCleanups));
     }
 }
diff --git a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml
@@ -17,12 +17,12 @@
             <Insets left="20.0" />
         </VBox.margin>
         <CheckBox fx:id="cleanUpUpgradeExternalLinks" />
-        <CheckBox fx:id="cleanUpMovePDF" />
+        <CheckBox fx:id="cleanUpMovePdf" />
         <CheckBox fx:id="cleanUpMakePathsRelative" text="%Make paths of linked files relative (if possible)" />
-        <CheckBox fx:id="cleanUpRenamePDF" text="%Rename files to configured filename format pattern" />
+        <CheckBox fx:id="cleanUpRenamePdf" text="%Rename files to configured filename format pattern" />
         <VBox prefHeight="40.0" prefWidth="451.0" spacing="10.0">
-            <Label fx:id="cleanupRenamePDFLabel" />
-            <CheckBox fx:id="cleanUpRenamePDFonlyRelativePaths" text="%Only rename files that have a relative path" />
+            <Label fx:id="cleanupRenamePdfLabel" />
+            <CheckBox fx:id="cleanUpRenamePdfonlyRelativePaths" text="%Only rename files that have a relative path" />
             <VBox.margin>
                 <Insets left="20.0" />
             </VBox.margin>
diff --git a/jablib/src/main/java/org/jabref/logic/cleanup/CleanupPreferences.java b/jablib/src/main/java/org/jabref/logic/cleanup/CleanupPreferences.java
@@ -92,11 +92,13 @@ public CleanupPreferences updateWith(CleanupPreferences tabPreferences) {
             }
         );
 
+        FieldFormatterCleanups mergedFormatters =
+                Optional.ofNullable(tabPreferences.getFieldFormatterCleanups())
+                        .filter(ff -> !ff.getConfiguredActions().isEmpty())
+                        .orElse(getFieldFormatterCleanups());
+
         return new CleanupPreferences(
-                mergedJobs,
-                tabPreferences.getFieldFormatterCleanups() != null
-                        ? tabPreferences.getFieldFormatterCleanups()
-                        : getFieldFormatterCleanups()
+                mergedJobs, mergedFormatters
         );
     }
 
diff --git a/jablib/src/main/resources/l10n/JabRef_en.properties b/jablib/src/main/resources/l10n/JabRef_en.properties
@@ -2845,6 +2845,8 @@ Writing\ metadata\ to\ %0=Writing metadata to %0
 Get\ more\ themes...=Get more themes...
 
 Miscellaneous=Miscellaneous
+Single\ field =Single field
+Multi-field=Multi-field
 File-related=File-related
 
 Add\ selected\ entry(s)\ to\ library=Add selected entry(s) to library

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,9 @@`
`1`	`1`	`package org.jabref.gui.cleanup;`
`2`	`2`
	`3`	`+import java.util.Optional;`
	`4`	`+`
`3`	`5`	`import org.jabref.logic.cleanup.CleanupPreferences;`
`4`	`6`
`5`	`7`	`public interface CleanupPanel {`
`6`		`- CleanupPreferences getCleanupPreferences();`
	`8`	`+ Optional<CleanupPreferences> getCleanupPreferences();`
`7`	`9`	`}`