Fix hang when exporting at the CLI (#10383)

* Some code refactorings - pass RemotePreferences to handleMultipleAppInstances - Introduce separate call to processArguments() - Add test case convertBibtexToTablerefsabsbib * Fix issue * Fix typo * Replace sourceforge.net link to www.jabref.org link (and use ISO date format) * Fix localization strings * Fix typo - and remove no-additional-value-comment * Somre more localiaztion improvements * Fix scope * Fix hanging * Fix strings * More fixes * More fixes * The ArgumentProcessor seems to be independent from Globals.prefs * Remove dot * Update abbreviations * better safe than sorry * Insert System.exit(0);
JabRef · Sep 16, 2023 · 94bc5c2 · 94bc5c2
1 parent 3160499
commit 94bc5c2
Show file tree

Hide file tree

Showing 19 changed files with 122 additions and 80 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,12 +13,14 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 
 - We added the possibility to find (and add) papers that cite or are cited by a given paper. [#6187](https://github.com/JabRef/jabref/issues/6187)
 - We added an error-specific message for when a download from a URL fails. [#9826](https://github.com/JabRef/jabref/issues/9826)
-- We added support for customizating the citation command (e.g., `[@key1,@key2]`) when [pushing to external applications](https://docs.jabref.org/cite/pushtoapplications). [#10133](https://github.com/JabRef/jabref/issues/10133)
+- We added support for customizing the citation command (e.g., `[@key1,@key2]`) when [pushing to external applications](https://docs.jabref.org/cite/pushtoapplications). [#10133](https://github.com/JabRef/jabref/issues/10133)
 - We added an integrity check for more special characters. [#8712](https://github.com/JabRef/jabref/issues/8712)
 - We added protected terms described as "Computer science". [#10222](https://github.com/JabRef/jabref/pull/10222)
 
 ### Changed
 
+- In the exports listrefs, tablerefs, tablerefsabsbib, use ISO date format in the footer.
+
 ### Fixed
 
 - It is possible again to use "current table sort order" for the order of entries when saving. [#9869](https://github.com/JabRef/jabref/issues/9869)
@@ -28,6 +30,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 - We fixed an issue where the ISBN fetcher returned the entrytype `misc` for certain ISBN numbers [#10348](https://github.com/JabRef/jabref/issues/10348)
 - We fixed a bug where an exception was raised when saving less than three export save orders in the preference. [#10157](https://github.com/JabRef/jabref/issues/10157)
 - We fixed an issue where it was possible to create a group with no name or with a group separator inside the name [#9776](https://github.com/JabRef/jabref/issues/9776)
+- JabRef does not hang anymore when exporting via CLI. [#10380](https://github.com/JabRef/jabref/issues/10380)
 
 ### Removed
 

diff --git a/buildres/abbrv.jabref.org b/buildres/abbrv.jabref.org
diff --git a/src/main/java/org/jabref/cli/ArgumentProcessor.java b/src/main/java/org/jabref/cli/ArgumentProcessor.java
@@ -67,13 +67,20 @@ public class ArgumentProcessor {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(ArgumentProcessor.class);
     private final JabRefCLI cli;
-    private final List<ParserResult> parserResults;
+
+    // Written once by processArguments()
+    private List<ParserResult> parserResults = List.of();
+
     private final Mode startupMode;
     private final PreferencesService preferencesService;
     private final FileUpdateMonitor fileUpdateMonitor;
     private final BibEntryTypesManager entryTypesManager;
     private boolean noGUINeeded;
 
+    /**
+     * First call the constructor, then call {@link #processArguments()}.
+     * Afterward, you can access the {@link #getParserResults()} and other getters.
+     */
     public ArgumentProcessor(String[] args,
                              Mode startupMode,
                              PreferencesService preferencesService,
@@ -84,8 +91,6 @@ public ArgumentProcessor(String[] args,
         this.preferencesService = preferencesService;
         this.fileUpdateMonitor = fileUpdateMonitor;
         this.entryTypesManager = entryTypesManager;
-
-        this.parserResults = processArguments();
     }
 
     /**
@@ -145,8 +150,8 @@ private Optional<ParserResult> importFile(String argument) {
 
         Optional<ParserResult> importResult = importFile(file, importFormat);
         importResult.ifPresent(result -> {
-            OutputPrinter printer = new SystemOutputPrinter();
             if (result.hasWarnings()) {
+                OutputPrinter printer = new SystemOutputPrinter();
                 printer.showMessage(result.getErrorMessage());
             }
         });
@@ -161,22 +166,21 @@ private Optional<ParserResult> importFile(Path file, String importFormat) {
                     fileUpdateMonitor);
 
             if (!"*".equals(importFormat)) {
-                System.out.println(Localization.lang("Importing") + ": " + file);
+                System.out.println(Localization.lang("Importing %0", file));
                 ParserResult result = importFormatReader.importFromFile(importFormat, file);
                 return Optional.of(result);
             } else {
                 // * means "guess the format":
-                System.out.println(Localization.lang("Importing in unknown format") + ": " + file);
+                System.out.println(Localization.lang("Importing file %0 as unknown format", file));
 
                 ImportFormatReader.UnknownFormatImport importResult =
                         importFormatReader.importUnknownFormat(file, new DummyFileUpdateMonitor());
 
-                System.out.println(Localization.lang("Format used") + ": " + importResult.format());
+                System.out.println(Localization.lang("Format used: %0", importResult.format()));
                 return Optional.of(importResult.parserResult());
             }
         } catch (ImportException ex) {
-            System.err
-                    .println(Localization.lang("Error opening file") + " '" + file + "': " + ex.getLocalizedMessage());
+            System.err.println(Localization.lang("Error opening file '%0'", file) + "\n" + ex.getLocalizedMessage());
             return Optional.empty();
         }
     }
@@ -185,19 +189,16 @@ public List<ParserResult> getParserResults() {
         return parserResults;
     }
 
-    public boolean hasParserResults() {
-        return !parserResults.isEmpty();
-    }
-
-    private List<ParserResult> processArguments() {
+    public void processArguments() {
         if ((startupMode == Mode.INITIAL_START) && cli.isShowVersion()) {
             cli.displayVersion();
         }
 
         if ((startupMode == Mode.INITIAL_START) && cli.isHelp()) {
             JabRefCLI.printUsage(preferencesService);
             noGUINeeded = true;
-            return Collections.emptyList();
+            this.parserResults = Collections.emptyList();
+            return;
         }
 
         // Check if we should reset all preferences to default values:
@@ -220,7 +221,8 @@ private List<ParserResult> processArguments() {
         if (cli.isExportMatches()) {
             if (!loaded.isEmpty()) {
                 if (!exportMatches(loaded)) {
-                    return Collections.emptyList();
+                    this.parserResults = Collections.emptyList();
+                    return;
                 }
             } else {
                 System.err.println(Localization.lang("The output option depends on a valid input option."));
@@ -275,7 +277,7 @@ private List<ParserResult> processArguments() {
             doAuxImport(loaded);
         }
 
-        return loaded;
+        this.parserResults = loaded;
     }
 
     private void writeMetadataToPdf(List<ParserResult> loaded,
@@ -475,15 +477,14 @@ private boolean exportMatches(List<ParserResult> loaded) {
                         Globals.entryTypesManager);
                 Optional<Exporter> exporter = exporterFactory.getExporterByName(formatName);
                 if (exporter.isEmpty()) {
-                    System.err.println(Localization.lang("Unknown export format") + ": " + formatName);
+                    System.err.println(Localization.lang("Unknown export format %0", formatName));
                 } else {
                     // We have an TemplateExporter instance:
                     try {
-                        System.out.println(Localization.lang("Exporting") + ": " + data[1]);
+                        System.out.println(Localization.lang("Exporting %0", data[1]));
                         exporter.get().export(databaseContext, Path.of(data[1]), matches, Collections.emptyList(), Globals.journalAbbreviationRepository);
                     } catch (Exception ex) {
-                        System.err.println(Localization.lang("Could not export file") + " '" + data[1] + "': "
-                                + Throwables.getStackTraceAsString(ex));
+                        System.err.println(Localization.lang("Could not export file '%0' (reason: %1)", data[1], Throwables.getStackTraceAsString(ex)));
                     }
                 }
             }
@@ -503,7 +504,7 @@ private void doAuxImport(List<ParserResult> loaded) {
         }
 
         if (usageMsg) {
-            System.out.println(Localization.lang("no base-BibTeX-file specified") + "!");
+            System.out.println(Localization.lang("no base-BibTeX-file specified!"));
             System.out.println(Localization.lang("usage") + " :");
             System.out.println("jabref --aux infile[.aux],outfile[.bib] base-BibTeX-file");
         }
@@ -634,32 +635,31 @@ private void exportFile(List<ParserResult> loaded, String[] data) {
         } else if (data.length == 2) {
             // This signals that the latest import should be stored in the given
             // format to the given file.
-            ParserResult pr = loaded.get(loaded.size() - 1);
+            ParserResult parserResult = loaded.get(loaded.size() - 1);
 
-            Path path = pr.getPath().get().toAbsolutePath();
-            BibDatabaseContext databaseContext = pr.getDatabaseContext();
+            Path path = parserResult.getPath().get().toAbsolutePath();
+            BibDatabaseContext databaseContext = parserResult.getDatabaseContext();
             databaseContext.setDatabasePath(path);
             List<Path> fileDirForDatabase = databaseContext
                     .getFileDirectories(preferencesService.getFilePreferences());
-            System.out.println(Localization.lang("Exporting") + ": " + data[0]);
+            System.out.println(Localization.lang("Exporting %0", data[0]));
             ExporterFactory exporterFactory = ExporterFactory.create(
                     preferencesService,
                     Globals.entryTypesManager);
             Optional<Exporter> exporter = exporterFactory.getExporterByName(data[1]);
             if (exporter.isEmpty()) {
-                System.err.println(Localization.lang("Unknown export format") + ": " + data[1]);
+                System.err.println(Localization.lang("Unknown export format %0", data[1]));
             } else {
                 // We have an exporter:
                 try {
                     exporter.get().export(
-                            pr.getDatabaseContext(),
+                            parserResult.getDatabaseContext(),
                             Path.of(data[0]),
-                            pr.getDatabaseContext().getDatabase().getEntries(),
+                            parserResult.getDatabaseContext().getDatabase().getEntries(),
                             fileDirForDatabase,
                             Globals.journalAbbreviationRepository);
                 } catch (Exception ex) {
-                    System.err.println(Localization.lang("Could not export file") + " '" + data[0] + "': "
-                            + Throwables.getStackTraceAsString(ex));
+                    System.err.println(Localization.lang("Could not export file '%0' (reason: %1)", data[0], Throwables.getStackTraceAsString(ex)));
                 }
             }
         }

diff --git a/src/main/java/org/jabref/cli/Launcher.java b/src/main/java/org/jabref/cli/Launcher.java
@@ -65,17 +65,18 @@ public static void main(String[] args) {
             BibEntryTypesManager entryTypesManager = new BibEntryTypesManager();
             Globals.entryTypesManager = entryTypesManager;
 
-            // Init preferences
+            // Initialize preferences
             final JabRefPreferences preferences = JabRefPreferences.getInstance();
-            Globals.prefs = preferences;
-            PreferencesMigrations.runMigrations(preferences, entryTypesManager);
 
             // Early exit in case another instance is already running
-            if (!handleMultipleAppInstances(ARGUMENTS, preferences)) {
+            if (!handleMultipleAppInstances(ARGUMENTS, preferences.getRemotePreferences())) {
                 return;
             }
 
-            // Init rest of preferences
+            Globals.prefs = preferences;
+            PreferencesMigrations.runMigrations(preferences, entryTypesManager);
+
+            // Initialize rest of preferences
             configureProxy(preferences.getProxyPreferences());
             configureSSL(preferences.getSSLPreferences());
             initGlobals(preferences);
@@ -86,13 +87,17 @@ public static void main(String[] args) {
 
                 // Process arguments
                 ArgumentProcessor argumentProcessor = new ArgumentProcessor(
-                        ARGUMENTS, ArgumentProcessor.Mode.INITIAL_START,
+                        ARGUMENTS,
+                        ArgumentProcessor.Mode.INITIAL_START,
                         preferences,
                         fileUpdateMonitor,
                         entryTypesManager);
+                argumentProcessor.processArguments();
                 if (argumentProcessor.shouldShutDown()) {
                     LOGGER.debug("JabRef shut down after processing command line arguments");
-                    return;
+                    // A clean shutdown takes 60s time
+                    // We don't need the clean shutdown here
+                    System.exit(0);
                 }
 
                 MainApplication.main(argumentProcessor.getParserResults(), argumentProcessor.isBlank(), preferences, fileUpdateMonitor, ARGUMENTS);
@@ -141,8 +146,7 @@ private static void initializeLogger() {
         LOGGER = LoggerFactory.getLogger(MainApplication.class);
     }
 
-    private static boolean handleMultipleAppInstances(String[] args, PreferencesService preferences) {
-        RemotePreferences remotePreferences = preferences.getRemotePreferences();
+    private static boolean handleMultipleAppInstances(String[] args, RemotePreferences remotePreferences) {
         if (remotePreferences.useRemoteServer()) {
             // Try to contact already running JabRef
             RemoteClient remoteClient = new RemoteClient(remotePreferences.getPort());

diff --git a/src/main/java/org/jabref/gui/Globals.java b/src/main/java/org/jabref/gui/Globals.java
@@ -15,7 +15,7 @@
 import org.jabref.logic.util.BuildInfo;
 import org.jabref.model.entry.BibEntryTypesManager;
 import org.jabref.model.util.FileUpdateMonitor;
-import org.jabref.preferences.JabRefPreferences;
+import org.jabref.preferences.PreferencesService;
 
 import kong.unirest.Unirest;
 
@@ -43,7 +43,7 @@ public class Globals {
     /**
      * Each test case initializes this field if required
      */
-    public static JabRefPreferences prefs;
+    public static PreferencesService prefs;
 
     /**
      * This field is initialized upon startup.

diff --git a/src/main/java/org/jabref/gui/JabRefGUI.java b/src/main/java/org/jabref/gui/JabRefGUI.java
@@ -251,7 +251,7 @@ private void openDatabases() {
         }
 
         for (ParserResult pr : failed) {
-            String message = Localization.lang("Error opening file '%0'.",
+            String message = Localization.lang("Error opening file '%0'",
                     pr.getPath().map(Path::toString).orElse("(File name unknown)")) + "\n" +
                     pr.getErrorMessage();
 

diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
@@ -217,7 +217,7 @@ public void open() {
                 dialogService.showErrorDialogAndWait(Localization.lang("File not found"), Localization.lang("Could not find file '%0'.", linkedFile.getLink()));
             }
         } catch (IOException e) {
-            dialogService.showErrorDialogAndWait(Localization.lang("Error opening file '%0'.", linkedFile.getLink()), e);
+            dialogService.showErrorDialogAndWait(Localization.lang("Error opening file '%0'", linkedFile.getLink()), e);
         }
     }
 

diff --git a/src/main/java/org/jabref/gui/importer/ImportCommand.java b/src/main/java/org/jabref/gui/importer/ImportCommand.java
@@ -146,7 +146,7 @@ private ParserResult doImport(List<Path> files, Importer importFormat) throws IO
                         if (FileUtil.isPDFFile(filename) && GrobidOptInDialogHelper.showAndWaitIfUserIsUndecided(dialogService, preferencesService.getGrobidPreferences())) {
                             importFormatReader.reset();
                         }
-                        dialogService.notify(Localization.lang("Importing in unknown format") + "...");
+                        dialogService.notify(Localization.lang("Importing file %0 as unknown format", filename.getFileName().toString()));
                     });
                     // This import method never throws an IOException
                     imports.add(importFormatReader.importUnknownFormat(filename, fileUpdateMonitor));

diff --git a/src/main/java/org/jabref/gui/remote/CLIMessageHandler.java b/src/main/java/org/jabref/gui/remote/CLIMessageHandler.java
@@ -38,7 +38,7 @@ public void handleCommandLineArguments(String[] message) {
                     preferencesService,
                     fileUpdateMonitor,
                     entryTypesManager);
-
+            argumentProcessor.processArguments();
             List<ParserResult> loaded = argumentProcessor.getParserResults();
             for (int i = 0; i < loaded.size(); i++) {
                 ParserResult pr = loaded.get(i);

diff --git a/src/main/java/org/jabref/logic/bst/BstPreviewLayout.java b/src/main/java/org/jabref/logic/bst/BstPreviewLayout.java
@@ -30,14 +30,14 @@ public BstPreviewLayout(Path path) {
         name = path.getFileName().toString();
         if (!Files.exists(path)) {
             LOGGER.error("File {} not found", path.toAbsolutePath());
-            error = Localization.lang("Error opening file '%0'.", path.toString());
+            error = Localization.lang("Error opening file '%0'", path.toString());
             return;
         }
         try {
             bstVM = new BstVM(path);
         } catch (Exception e) {
             LOGGER.error("Could not read {}.", path.toAbsolutePath(), e);
-            error = Localization.lang("Error opening file '%0'.", path.toString());
+            error = Localization.lang("Error opening file '%0'", path.toString());
         }
     }
 

diff --git a/src/main/java/org/jabref/logic/exporter/TemplateExporter.java b/src/main/java/org/jabref/logic/exporter/TemplateExporter.java
@@ -311,7 +311,6 @@ public void export(final BibDatabaseContext databaseContext,
                 missingFormatters.addAll(endLayout.getMissingFormatters());
             }
 
-            // Clear custom name formatters:
             layoutPreferences.clearCustomExportNameFormatters();
 
             if (!missingFormatters.isEmpty() && LOGGER.isWarnEnabled()) {

diff --git a/src/main/java/org/jabref/logic/layout/Layout.java b/src/main/java/org/jabref/logic/layout/Layout.java
@@ -117,7 +117,7 @@ public String doLayout(BibEntry bibtex, BibDatabase database) {
 
     /**
      * Returns the processed text. If the database argument is
-     * null, no string references will be resolved. Otherwise all valid
+     * null, no string references will be resolved. Otherwise, all valid
      * string references will be replaced by the strings' contents. Even
      * recursive string references are resolved.
      */

diff --git a/src/main/java/org/jabref/preferences/ExportPreferences.java b/src/main/java/org/jabref/preferences/ExportPreferences.java
@@ -24,7 +24,6 @@ public ExportPreferences(String lastExportExtension,
                              Path exportWorkingDirectory,
                              SaveOrder exportSaveOrder,
                              List<TemplateExporter> customExporters) {
-
         this.lastExportExtension = new SimpleStringProperty(lastExportExtension);
         this.exportWorkingDirectory = new SimpleObjectProperty<>(exportWorkingDirectory);
         this.exportSaveOrder = new SimpleObjectProperty<>(exportSaveOrder);

diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java
@@ -2287,7 +2287,7 @@ private void storeExportSaveOrder(SaveOrder saveOrder) {
      * For the export configuration, generates the SelfContainedSaveOrder having the reference to TABLE resolved.
      */
     public SelfContainedSaveOrder getSelfContainedTableSaveOrder() {
-        List<MainTableColumnModel> sortOrder = mainTableColumnPreferences.getColumnSortOrder();
+        List<MainTableColumnModel> sortOrder = getMainTableColumnPreferences().getColumnSortOrder();
         return new SelfContainedSaveOrder(
                 SaveOrder.OrderType.SPECIFIED,
                 sortOrder.stream().flatMap(model -> model.getSortCriteria().stream()).toList());