Miscellaneous refactoring - II #13197

subhramit · 2025-05-30T21:26:43Z

Follow-up to #13178

Addresses Miscellaneous refactoring #13178 (comment) and Miscellaneous refactoring #13178 (comment)
Remove generic exceptions from codebase wherever possible
Remove unused observable change parameters in JabRefCliPreferences
Use toList() in the classes touched above where applicable

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.

Signed-off-by: subhramit <subhramit.bb@live.in>

jabgui/src/main/java/org/jabref/gui/openoffice/OOBibBaseConnect.java

jabgui/src/main/java/org/jabref/gui/util/FilteredListProxy.java

subhramit · 2025-05-30T21:32:11Z

jablib/src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java

+                case "*" -> { // for all direct subdirs
+                    String restOfFileString = StringUtil.join(fileParts, "/", index + 1, fileParts.length);
+
+                    final Path rootDirectory = currentDirectory;
+                    try (Stream<Path> pathStream = Files.walk(currentDirectory, 1)) {
+                        List<Path> subDirs = pathStream
+                                .filter(path -> isSubDirectory(rootDirectory, path))  // We only want to transverse directories (and not the current one; this is already done below)
+                                .toList();
+
+                        for (Path subDir : subDirs) {
+                            resultFiles.addAll(findFile(entry, subDir, restOfFileString, extensionRegExp));
                        }
+                    } catch (UncheckedIOException ioe) {
+                        throw ioe.getCause();
                    }
                }
-            }
-            // Do for all direct and indirect subdirs
-            if ("**".equals(dirToProcess)) {
-                String restOfFileString = StringUtil.join(fileParts, "/", index + 1, fileParts.length);
-
-                final Path rootDirectory = actualDirectory;
-                try (Stream<Path> pathStream = Files.walk(actualDirectory)) {
-                    // We only want to transverse directory (and not the current one; this is already done below)
-                    for (Path path : pathStream.filter(element -> isSubDirectory(rootDirectory, element)).toList()) {
-                        resultFiles.addAll(findFile(entry, path, restOfFileString, extensionRegExp));
+                case "**" -> { // for all direct and indirect subdirs
+                    String restOfFileString = StringUtil.join(fileParts, "/", index + 1, fileParts.length);
+
+                    final Path rootDirectory = currentDirectory;
+                    try (Stream<Path> pathStream = Files.walk(currentDirectory)) {
+                        List<Path> subDirs = pathStream
+                                .filter(path -> isSubDirectory(rootDirectory, path))  // We only want to transverse directories (and not the current one; this is already done below)
+                                .toList();
+
+                        for (Path subDir : subDirs) {
+                            resultFiles.addAll(findFile(entry, subDir, restOfFileString, extensionRegExp));
+                        }
+                    } catch (UncheckedIOException ioe) {
+                        throw ioe.getCause();
                    }


These two are very similar - only difference being the method overload used for Files.walk. The first one uses Files.walk(currentDirectory, 1) for max depth 1, while the second uses Files.walk(currentDirectory).

I thought a lot about unifying them as case "*", "**" but would need something like

int maxDepth = dirToProcess.equals("*") ? 1 : Integer.MAX_VALUE;

and use it as the second parameter, but I don't really like Integer.MAX_VALUE here to imply infinite depth.
To avoid this - if I use the respective overloads, it would require if statements for the entire try-with-resources blocks and thus would beat the purpose of unifying the cases using switch.

Thoughts?

You are right not using Integer.MAX_VALUE. I think sometimes, code duplication is ok 😅

If both branches are covered by tests, they are prepared for future refactorings 😅

jabkit/src/main/java/org/jabref/cli/PdfUpdate.java

jablib/src/main/java/org/jabref/generators/LtwaListMvGenerator.java

…nder` Signed-off-by: subhramit <subhramit.bb@live.in>

jablib/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java

jablib/src/main/java/org/jabref/logic/exporter/OOCalcDatabase.java

jablib/src/main/java/org/jabref/logic/exporter/OpenDocumentRepresentation.java

jablib/src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java

jablib/src/main/java/org/jabref/logic/importer/fileformat/MrDLibImporter.java

jablib/src/main/java/org/jabref/logic/importer/util/MathMLParser.java

jablib/src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java

subhramit · 2025-05-31T09:51:04Z

Damn, windows jablib tests took 34 min this time
it seems to take time after

WARNING: Use --enable-native-access=ALL-UNNAMED to avoid a warning for callers in this module
WARNING: Restricted methods will be blocked in a future release unless native access is enabled

Signed-off-by: subhramit <subhramit.bb@live.in>

trag-bot · 2025-06-01T09:20:54Z

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

…or-test * upstream/main: (102 commits) Try to fix output Improve AI preferences UI and templates (JabRef#13202) Bump jablib/src/main/abbrv.jabref.org from `6926b83` to `333c2f1` (JabRef#13216) Bump jablib/src/main/resources/csl-styles from `8a2317a` to `c3df987` (JabRef#13215) Fixed search result focus handling (JabRef#13174) New Crowdin updates (JabRef#13214) Add Pseudonymization to CLI (JabRef#13158) Try parallel gource build Update gource.yml Fix position of checkout Preapre: Enable gradle configuration cache (JabRef#13212) Add yml as YAML extension (JabRef#13213) Fix wrong detection of issue numbers (JabRef#13211) Miscellaneous refactoring - II (JabRef#13197) Run Windows tests only on main (and on demand) (JabRef#13210) Fix porcelain for consistency check (JabRef#13209) Use setup-jbang action (instead of custom call of .sh script) (JabRef#13208) Add link to JabRef guru (JabRef#13207) Switch to gradlex for modularity (JabRef#13112) feat(ci-cd): change issue URL pattern (JabRef#13206) ...

subhramit added 3 commits May 31, 2025 01:43

Use Files.walk, unify logic, rename actual -> current

2014fae

Signed-off-by: subhramit <subhramit.bb@live.in>

Use try-with-resources

ee6e107

Signed-off-by: subhramit <subhramit.bb@live.in>

Remove generic exceptions, remove unused observables

f31bd0c

Signed-off-by: subhramit <subhramit.bb@live.in>