OpenOffice Refactor subtasks

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

@@ -218,19 +218,10 @@ public boolean isDocumentConnectionMissing() {
         return false;
     }
 
-    /// Either return a valid XTextDocument or throw NoDocumentException.
-    public XTextDocument getXTextDocumentOrThrow()
-            throws
-            NoDocumentException {
-        if (isDocumentConnectionMissing()) {
-            throw new NoDocumentException("Not connected to document");
-        }
-        return this.xTextDocument;
-    }
-
+    /// Returns either a valid XTextDocument in OOResult or a NoDocumentException error
     public OOResult<XTextDocument, OOError> getXTextDocument() {
         if (isDocumentConnectionMissing()) {
-            return OOResult.error(OOError.from(new NoDocumentException()));
+            return OOResult.error(OOError.from(new NoDocumentException("Not connected to document")));
         }
         return OOResult.ok(this.xTextDocument);
     }

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

@@ -225,7 +225,8 @@ private void initPanel() {
                 ooBase.guiActionSelectDocument(false);
             } catch (WrappedTargetException
                      | NoSuchElementException ex) {
-                throw new RuntimeException(ex);
+                LOGGER.warn("Unable to select document to work on", ex);
+                OOError.fromMisc(ex).setTitle("Unable to select document to work on").showErrorDialog(dialogService);
             }
         });
 
@@ -348,23 +349,23 @@ private void exportEntries() {
     }
 
     private List<BibDatabase> getBaseList() {
-        List<BibDatabase> databases = new ArrayList<>();
         if (openOfficePreferences.getUseAllDatabases()) {
-            for (BibDatabaseContext database : stateManager.getOpenDatabases()) {
-                databases.add(database.getDatabase());
-            }
-        } else {
-            databases.add(stateManager.getActiveDatabase()
-                                      .map(BibDatabaseContext::getDatabase)
-                                      .orElse(new BibDatabase()));
+            return new ArrayList<>(stateManager.getOpenDatabases().stream()
+                                               .map(BibDatabaseContext::getDatabase)
+                                               .toList());
         }
-
-        return databases;
+        return new ArrayList<>(List.of(
+                stateManager.getActiveDatabase()
+                            .map(BibDatabaseContext::getDatabase)
+                            .orElseGet(BibDatabase::new)));
     }
 
     private void connectAutomatically() {
         DetectOpenOfficeInstallation officeInstallation = new DetectOpenOfficeInstallation(openOfficePreferences, dialogService);
 
+        final String errorTitle = Localization.lang("Autodetection failed");
+        final String progressMessage = Localization.lang("Autodetecting paths...");
+
         if (officeInstallation.isExecutablePathDefined()) {
             connect();
         } else {
@@ -386,9 +387,9 @@ protected List<Path> call() {
                 }
             });
 
-            taskConnectIfInstalled.setOnFailed(_ -> dialogService.showErrorDialogAndWait(Localization.lang("Autodetection failed"), Localization.lang("Autodetection failed"), taskConnectIfInstalled.getException()));
+            taskConnectIfInstalled.setOnFailed(_ -> dialogService.showErrorDialogAndWait(errorTitle, errorTitle, taskConnectIfInstalled.getException()));
 
-            dialogService.showProgressDialog(Localization.lang("Autodetecting paths..."), Localization.lang("Autodetecting paths..."), taskConnectIfInstalled);
+            dialogService.showProgressDialog(progressMessage, progressMessage, taskConnectIfInstalled);
             taskExecutor.execute(taskConnectIfInstalled);
         }
     }
@@ -397,6 +398,9 @@ private void connectManually() {
         DirectoryDialogConfiguration fileDialogConfiguration = new DirectoryDialogConfiguration.Builder().withInitialDirectory(System.getProperty("user.home")).build();
         Optional<Path> selectedPath = dialogService.showDirectorySelectionDialog(fileDialogConfiguration);
 
+        final String errorTitle = Localization.lang("Could not connect to running OpenOffice/LibreOffice.");
+        final String extendedErrorTitle = Localization.lang("If connecting manually, please verify program and library paths.");
+
         DetectOpenOfficeInstallation officeInstallation = new DetectOpenOfficeInstallation(openOfficePreferences, dialogService);
 
         if (selectedPath.isPresent()) {
@@ -406,12 +410,12 @@ private void connectManually() {
                               if (value) {
                                   connect();
                               } else {
-                                  dialogService.showErrorDialogAndWait(Localization.lang("Could not connect to running OpenOffice/LibreOffice."), Localization.lang("If connecting manually, please verify program and library paths."));
+                                  dialogService.showErrorDialogAndWait(errorTitle, extendedErrorTitle);
                               }
                           })
                           .executeWith(taskExecutor);
         } else {
-            dialogService.showErrorDialogAndWait(Localization.lang("Could not connect to running OpenOffice/LibreOffice."), Localization.lang("If connecting manually, please verify program and library paths."));
+            dialogService.showErrorDialogAndWait(errorTitle, extendedErrorTitle);
         }
     }
 
@@ -440,6 +444,11 @@ private void updateButtonAvailability() {
     }
 
     private void connect() {
+        final String connectionError = Localization.lang("Could not connect to running OpenOffice/LibreOffice.");
+        final String autodetectionFailedError = Localization.lang("Autodetection failed");
+        final String progressMessage = Localization.lang("Autodetecting paths...");
+        final String loggerMessage = "Could not connect to running OpenOffice/LibreOffice";
+
         Task<OOBibBase> connectTask = new Task<>() {
             @Override
             protected OOBibBase call() throws BootstrapException, CreationException, IOException, InterruptedException {
@@ -457,7 +466,9 @@ protected OOBibBase call() throws BootstrapException, CreationException, IOExcep
                 ooBase.guiActionSelectDocument(true);
             } catch (WrappedTargetException
                      | NoSuchElementException e) {
-                throw new RuntimeException(e);
+                LOGGER.warn("Unable to connect to document", e);
+                OOError.fromMisc(e).setTitle("Unable to connect to document").showErrorDialog(dialogService);
+                return;
             }
 
             // Enable actions that depend on a connection
@@ -469,16 +480,16 @@ protected OOBibBase call() throws BootstrapException, CreationException, IOExcep
             LOGGER.error("autodetect failed", ex);
             switch (ex) {
                 case UnsatisfiedLinkError unsatisfiedLinkError -> {
-                    LOGGER.warn("Could not connect to running OpenOffice/LibreOffice", unsatisfiedLinkError);
+                    LOGGER.warn(loggerMessage, unsatisfiedLinkError);
 
                     dialogService.showErrorDialogAndWait(Localization.lang("Unable to connect. One possible reason is that JabRef "
                             + "and OpenOffice/LibreOffice are not both running in either 32 bit mode or 64 bit mode."));
                 }
                 case IOException ioException -> {
-                    LOGGER.warn("Could not connect to running OpenOffice/LibreOffice", ioException);
+                    LOGGER.warn(loggerMessage, ioException);
 
-                    dialogService.showErrorDialogAndWait(Localization.lang("Could not connect to running OpenOffice/LibreOffice."),
-                            Localization.lang("Could not connect to running OpenOffice/LibreOffice.")
+                    dialogService.showErrorDialogAndWait(connectionError,
+                            connectionError
                                     + "\n"
                                     + Localization.lang("Make sure you have installed OpenOffice/LibreOffice with Java support.") + "\n"
                                     + Localization.lang("If connecting manually, please verify program and library paths.") + "\n" + "\n" + Localization.lang("Error message:"),
@@ -490,11 +501,11 @@ protected OOBibBase call() throws BootstrapException, CreationException, IOExcep
                 }
                 case null,
                      default ->
-                        dialogService.showErrorDialogAndWait(Localization.lang("Autodetection failed"), Localization.lang("Autodetection failed"), ex);
+                        dialogService.showErrorDialogAndWait(autodetectionFailedError, autodetectionFailedError, ex);
             }
         });
 
-        dialogService.showProgressDialog(Localization.lang("Autodetecting paths..."), Localization.lang("Autodetecting paths..."), connectTask);
+        dialogService.showProgressDialog(progressMessage, progressMessage, connectTask);
         taskExecutor.execute(connectTask);
     }
 
@@ -588,15 +599,9 @@ private void pushEntries(CitationType citationType, boolean addPageInfo) {
     /// @return true if all entries have citation keys, if it so may be after generating them
     private boolean checkThatEntriesHaveKeys(List<BibEntry> entries) {
         // Check if there are empty keys
-        boolean emptyKeys = false;
-        for (BibEntry entry : entries) {
-            if (entry.getCitationKey().isEmpty()) {
-                // Found one, no need to look further for now
-                emptyKeys = true;
-                break;
-            }
-        }
-
+        // Found one, no need to look further for now
+        boolean emptyKeys = entries.stream()
+                                   .anyMatch(entry -> entry.getCitationKey().isEmpty());
         // If no empty keys, return true
         if (!emptyKeys) {
             return true;

jablib/src/main/java/org/jabref/logic/openoffice/action/EditMerge.java

@@ -141,6 +141,9 @@ private static boolean checkAddToGroup(ScanState state, CitationGroup group, XTe
             return false;
         }
 
+        final String exceptionTitle = "MergeCitationGroups failed";
+        final String loggerMessage = "MergeCitationGroups: cursorBetween.end != currentGroupCursor.end";
+
         Objects.requireNonNull(state.currentGroupCursor);
         Objects.requireNonNull(state.cursorBetween);
         Objects.requireNonNull(state.prev);
@@ -187,14 +190,21 @@ private static boolean checkAddToGroup(ScanState state, CitationGroup group, XTe
 
         // assume: currentGroupCursor.getEnd() == cursorBetween.getEnd()
         if (UnoTextRange.compareEnds(state.cursorBetween, state.currentGroupCursor) != 0) {
-            LOGGER.warn("MergeCitationGroups: cursorBetween.end != currentGroupCursor.end");
-            throw new IllegalStateException("MergeCitationGroups failed");
+            LOGGER.warn(loggerMessage);
+            throw new IllegalStateException(exceptionTitle);
         }
 
-        /*
-         * Try to expand state.currentGroupCursor and state.cursorBetween by going right to reach
-         * rangeStart.
-         */
+        return checkCouldExpand(state, currentRange, loggerMessage, exceptionTitle);
+    }
+
+    /// Helper method for checkAddToGroup. Tries to expand state.currentGroupCursor and state.cursorBetween by going right to reach rangeStart.
+    ///
+    /// @param state          The CitationGroup to test.
+    /// @param currentRange   The XTextRange corresponding to group.
+    /// @param loggerMessage  The failure message for the LOGGER.
+    /// @param exceptionTitle The custom exception message.
+    /// @return false if cannot expand, true if can.
+    private static boolean checkCouldExpand(ScanState state, XTextRange currentRange, String loggerMessage, String exceptionTitle) {
         XTextRange rangeStart = currentRange.getStart();
         boolean couldExpand = true;
         XTextCursor thisCharCursor =
@@ -219,11 +229,10 @@ private static boolean checkAddToGroup(ScanState state, CitationGroup group, XTe
 
             // These two should move in sync:
             if (UnoTextRange.compareEnds(state.cursorBetween, state.currentGroupCursor) != 0) {
-                LOGGER.warn("MergeCitationGroups: cursorBetween.end != currentGroupCursor.end (during expand)");
-                throw new IllegalStateException("MergeCitationGroups failed");
+                LOGGER.warn(loggerMessage + " (during expand)");
+                throw new IllegalStateException(exceptionTitle);
             }
         }
-
         return couldExpand;
     }
 

jablib/src/main/java/org/jabref/logic/openoffice/backend/NamedRangeReferenceMark.java

@@ -247,6 +247,13 @@ public XTextCursor getFillCursor(XTextDocument doc)
         if (fullText.length() < 2) {
             throw new IllegalStateException("getFillCursor: fullText.length() < 2 (after loop)'%n");
         }
+
+        // NamedRangeReferenceMark.checkFillCursor(beta);
+        return getStringForFillCursor(full, fullText, debugThisFun);
+    }
+
+    /// Helper method for getFillCursor
+    public XTextCursor getStringForFillCursor(XTextCursor full, String fullText, boolean debugThisFun) {
         XTextCursor beta = full.getText().createTextCursorByRange(full);
         beta.collapseToStart();
         beta.goRight((short) 1, false);
@@ -311,8 +318,6 @@ public XTextCursor getFillCursor(XTextDocument doc)
                 LOGGER.debug("getFillCursor: omega(8) covers '{}', should be '{}'", omega.getString(), right);
             }
         }
-
-        // NamedRangeReferenceMark.checkFillCursor(beta);
         return beta;
     }
 

jablib/src/main/java/org/jabref/logic/openoffice/frontend/OOFrontend.java

@@ -325,7 +325,7 @@ public List<RangeForOverlapCheck<CitationGroupId>> viewCursorRanges(XTextDocumen
     ///
     /// Note: Here we directly communicate to the document, not through the backend. This is because mapping ranges to footnote marks does not depend on how do we mark or structure those ranges.
     public List<RangeForOverlapCheck<CitationGroupId>>
-    footnoteMarkRanges(XTextDocument doc, List<RangeForOverlapCheck<CitationGroupId>> citationRanges) {
+    footnoteMarkRanges(List<RangeForOverlapCheck<CitationGroupId>> citationRanges) {
         // We partition by XText and use a single range from
         // each partition to get at the corresponding footnotemark range.
 
@@ -402,7 +402,7 @@ static String rangeOverlapsToMessage(List<RangeOverlap<RangeForOverlapCheck<Cita
         // ranges.addAll(userRanges);
         ranges.addAll(bibliographyRanges(doc));
         ranges.addAll(citationRanges);
-        ranges.addAll(footnoteMarkRanges(doc, citationRanges));
+        ranges.addAll(footnoteMarkRanges(citationRanges));
 
         List<RangeOverlap<RangeForOverlapCheck<CitationGroupId>>> overlaps =
                 RangeOverlapBetween.findFirst(doc,
@@ -432,7 +432,7 @@ public OOVoidResult<JabRefException> checkRangeOverlaps(XTextDocument doc,
         ranges.addAll(userRanges);
         ranges.addAll(bibliographyRanges(doc));
         ranges.addAll(citationRanges);
-        ranges.addAll(footnoteMarkRanges(doc, citationRanges));
+        ranges.addAll(footnoteMarkRanges(citationRanges));
 
         List<RangeOverlap<RangeForOverlapCheck<CitationGroupId>>> overlaps =
                 RangeOverlapWithin.findOverlappingRanges(doc, ranges, requireSeparation, reportAtMost);

jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLCitationOOAdapter.java

@@ -320,34 +320,7 @@ private void updateAllCitationsWithNewStyle(CitationStyle style, CSLCitationType
                 StringBuilder finalText = new StringBuilder();
                 Iterator<BibEntry> iterator = entries.iterator();
 
-                while (iterator.hasNext()) {
-                    BibEntry currentEntry = iterator.next();
-
-                    // We re-generate the citation in the new style and update it in the document
-                    String newCitation;
-
-                    if (isAlphaNumericStyle) {
-                        newCitation = CSLFormatUtils.generateAlphanumericInTextCitation(currentEntry, unifiedBibDatabaseContext);
-                    } else {
-                        newCitation = CitationStyleGenerator.generateCitation(List.of(currentEntry), style.getSource(), HTML_OUTPUT_FORMAT, unifiedBibDatabaseContext, bibEntryTypesManager);
-                    }
-
-                    String formattedCitation = CSLFormatUtils.transformHTML(newCitation);
-
-                    if (isNumericStyle) {
-                        formattedCitation = updateSingleOrMultipleCitationNumbers(formattedCitation, List.of(currentEntry));
-                        String prefix = CSLFormatUtils.generateAuthorPrefix(currentEntry, unifiedBibDatabaseContext);
-                        formattedCitation = prefix + formattedCitation;
-                    } else if (!isAlphaNumericStyle) {
-                        formattedCitation = CSLFormatUtils.changeToInText(formattedCitation);
-                    }
-
-                    finalText.append(formattedCitation);
-
-                    if (iterator.hasNext()) {
-                        finalText.append(",");
-                    }
-                }
+                updateCitations(style, iterator, unifiedBibDatabaseContext, finalText);
 
                 markManager.updateMarkAndTextWithNewStyle(mark, finalText.toString(), CSLCitationType.IN_TEXT);
             }
@@ -377,6 +350,46 @@ private void updateAllCitationsWithNewStyle(CitationStyle style, CSLCitationType
         }
     }
 
+    /// Helper method for updateAllCitationsWithNewStyle.
+    ///
+    /// @param style                     The current style we are using.
+    /// @param iterator                  Used to iterate through entries.
+    /// @param unifiedBibDatabaseContext Used to generate citations.
+    /// @param finalText                 Resulting text for updated citations.
+    private void updateCitations(CitationStyle style, Iterator<BibEntry> iterator, BibDatabaseContext unifiedBibDatabaseContext, StringBuilder finalText) {
+        boolean isAlphaNumericStyle = style.isAlphanumericStyle();
+        boolean isNumericStyle = style.isNumericStyle();
+
+        while (iterator.hasNext()) {
+            BibEntry currentEntry = iterator.next();
+
+            // We re-generate the citation in the new style and update it in the document
+            String newCitation;
+
+            if (isAlphaNumericStyle) {
+                newCitation = CSLFormatUtils.generateAlphanumericInTextCitation(currentEntry, unifiedBibDatabaseContext);
+            } else {
+                newCitation = CitationStyleGenerator.generateCitation(List.of(currentEntry), style.getSource(), HTML_OUTPUT_FORMAT, unifiedBibDatabaseContext, bibEntryTypesManager);
+            }
+
+            String formattedCitation = CSLFormatUtils.transformHTML(newCitation);
+
+            if (isNumericStyle) {
+                formattedCitation = updateSingleOrMultipleCitationNumbers(formattedCitation, List.of(currentEntry));
+                String prefix = CSLFormatUtils.generateAuthorPrefix(currentEntry, unifiedBibDatabaseContext);
+                formattedCitation = prefix + formattedCitation;
+            } else if (!isAlphaNumericStyle) {
+                formattedCitation = CSLFormatUtils.changeToInText(formattedCitation);
+            }
+
+            finalText.append(formattedCitation);
+
+            if (iterator.hasNext()) {
+                finalText.append(",");
+            }
+        }
+    }
+
     /// Checks if an entry has already been cited before in the document.
     /// Required for consistent numbering of numeric citations - if present, the number is to be reused, else a new number is to be assigned.
     public boolean isCitedEntry(BibEntry entry) {