Add support for loading .blg warnings in Integrity Check Dialog #12866

wanling0000 · 2025-03-31T20:04:48Z

Closes #11998

This PR adds support for: Check integrity: Support for BibTeX .blg files

Users can now:

Automatically load the default .blg file from the same folder as the .bib file;
Select a custom .blg file via Browse and load BibTeX warnings;
Click Reset to revert to the default .blg path;
Replace old .blg warnings when switching files (i.e., only show current .blg file's messages);
Persist .blg file path in MetaData (saved along with .bib file).

Change Overview

Layer	Component	Responsibility
Model	`BibWarning`	Represent parsed `.blg` warning
	`MetaData.get/set/clearBlgFilePath()`	Store user-defined `.blg` file path
Logic	`BibtexLogParser`	Parse `.blg` lines into `BibWarning` objects
	`BibWarningToIntegrityMessageConverter`	Convert warnings to `IntegrityMessage`
	`BibLogPathResolver`	Resolve `.blg` file path: user-defined or fallback
	`MetaDataParser`	Add support for `BLG_FILE_PATH` in `.bib` metadata
	`MetaDataSerializer`	Serialize .blg file paths into metadata
ViewModel	`BibLogSettingsViewModel`	Wraps logic and exposes path + warning state to UI
UI	`BibLogSettingsPane.fxml`	Browse/Reset path selection UI
	`BibLogSettingsPane.java`	Controller handling Browse/Reset and notifying changes
	`IntegrityCheckDialog.java`	Load `.blg` warnings and refresh table display
	`IntegrityCheckAction.java`	Include `.blg` warnings during initial dialog show

Screenshots

❓Limitations of .blg Parsing – I would appreciate feedback

Right now, the .blg parser in this PR (BibtexLogParser) only supports a very simple warning format:

Warning--[message] in [entryKey]

But I found that in real .blg files, there are other kinds of warnings that don’t follow this format.
For example, in this GitHub issue: James-Yu/LaTeX-Workshop#2324

Warning--string name "ccr" is undefined
--line 35 of file paper.bib

This kind of warning does not point to a specific entry or field, so currently it is ignored by the parser.

In this PR, I used KEY_FIELD as a fallback when the field is missing. This is just a temporary solution so the UI can still display something. But the UI currently assumes that every IntegrityMessage has a field, otherwise it may not work properly.

I’m wondering:

Would it make sense to introduce an interface like IntegrityMessage, or is it better to wait and see more .blg formats before making this change?

At the moment I don’t have many .blg test files, so I’m not sure how common these “fieldless” warnings are.
If you have any suggestions on how to investigate this, I’d be happy to look into it!

Any feedback or thoughts on this would be really helpful, thank you!

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.

…d path resolution

trag-bot · 2025-03-31T20:06:40Z

src/main/java/org/jabref/gui/LibraryTab.java

+    public DialogService getDialogService() {
+        return dialogService;
+    }


The new public method 'getDialogService' returns 'null' if 'dialogService' is not initialized. It should return an Optional to avoid potential NullPointerExceptions.

Modified to pass DialogService from the constructor parameter

Do DI by constructor. Getters for singletons are a no go.

Siedlerchr · 2025-03-31T20:11:06Z

src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.java

+     */
+    private void loadBibLogSettingsPane() {
+        try {
+            FXMLLoader loader = new FXMLLoader(getClass().getResource("BibLogSettingsPane.fxml"));


@calixtus Don't we use the afterburner fx stuff here?

koppor · 2025-03-31T20:11:33Z

src/main/java/org/jabref/model/biblog/BibWarning.java

+/**
+ *
+ */
+public class BibWarning {


(quick initial comment)

Can this be a record? 😅

(and no empty JavaDoc please)

Thanks! I've converted it to a record and removed the empty JavaDoc

koppor · 2025-03-31T20:12:13Z

src/main/java/org/jabref/logic/biblog/BibtexLogParser.java

+     */
+    private Optional<BibWarning> parseWarningLine(String line) {
+        // TODO: Support additional warning formats
+        Pattern compile = Pattern.compile("^Warning--([a-zA-Z ]+) in ([^\\s]+)$");


Put this into private static final. - and use named groups 😅

Updated as suggested 👍

Siedlerchr · 2025-03-31T20:14:21Z

src/main/java/org/jabref/gui/integrity/BibLogSettingsPane.java

+
+    @FXML
+    private void onBrowse() {
+        FileChooser fileChooser = new FileChooser();


You can use jabref's dialogService for this: dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(path -> ...)

and with FileDialogConfiguration offers the Builder pattern.
(see e.g NewLibraryFromPdfAction)

Thanks, already updated as suggested

trag-bot · 2025-03-31T20:14:32Z

src/main/java/org/jabref/model/metadata/MetaData.java

+        return blgFilePath;
+    }
+
+    public void setBlgFilePath(Path path) {


The method uses Objects.requireNonNull to check for null, which should be replaced with @nonnull annotation as per the guidelines.

trag-bot · 2025-03-31T20:15:25Z

src/test/java/org/jabref/logic/biblog/BibWarningToIntegrityMessageConverterTest.java

+
+        List<IntegrityMessage> messages = BibWarningToIntegrityMessageConverter.convert(warnings, context);
+
+        assertEquals(1, messages.size());


Assertion statements should not include the message parameter. The method name should already convey the expected behavior.

@trag-bot This is wrong

…ntegrity

src/main/java/org/jabref/gui/integrity/BibLogSettingsViewModel.java

trag-bot · 2025-04-02T12:41:00Z

src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java

+        metaData.getBlgFilePaths().forEach((user, path) -> {
+            stringyMetaData.put(MetaData.BLG_FILE_PATH + "-" + user, Collections.singletonList(path.toString().trim()));
+        });


The addition of .blg file paths serialization in org.jabref.logic.exporter.MetaDataSerializer requires corresponding test updates to ensure the new functionality is covered.

already covered this change in the test method serializeUserSpecificBlgPath() ・ࡇ・

What does this - do? Path, naming separator?

Maybe this could be a constant?

I think, the other meta data fields do it, too.

I am OK without constant - follow-up can refactor the whole MetaDataParser ^^

src/test/java/org/jabref/logic/exporter/MetaDataSerializerTest.java

src/test/java/org/jabref/logic/importer/util/MetaDataParserTest.java

trag-bot · 2025-04-02T16:57:14Z

src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java

@@ -60,6 +61,9 @@ public static Map<String, String> getSerializedStringMap(MetaData metaData,
                .put(MetaData.FILE_DIRECTORY_LATEX + '-' + user, Collections.singletonList(path.toString().trim())));
        metaData.getVersionDBStructure().ifPresent(
                versionDBStructure -> stringyMetaData.put(MetaData.VERSION_DB_STRUCT, Collections.singletonList(versionDBStructure.trim())));
+        metaData.getBlgFilePaths().forEach((user, path) -> {


The addition of serializing .blg file paths requires corresponding tests to be updated or added to ensure the new functionality is correctly implemented and verified.

src/main/java/org/jabref/gui/integrity/BibLogSettingsPane.java

trag-bot · 2025-04-02T19:41:48Z

src/main/java/org/jabref/gui/integrity/BibLogSettingsPane.java

+
+import javafx.fxml.FXML;
+import javafx.scene.control.TextField;
+import javafx.stage.FileChooser;


The import statement for FileChooser is unnecessary as the dialogService is used for file dialogs, which aligns with the project's architecture guidelines.

I think, the comment of trag bot is correct.

koppor · 2025-04-04T07:54:50Z

Tried out now with a MWE generated by https://github.com/latextemplates/generator-latex-template. Works.

wanling0000 · 2025-04-04T09:48:23Z

Tried out now with a MWE generated by https://github.com/latextemplates/generator-latex-template. Works.

Thanks @koppor

I noticed some of the checks are failing, I’ll take a look and try pushing a fix over this week. Let me know if there’s anything else I should work on :)

src/main/java/org/jabref/gui/integrity/BibLogSettingsViewModel.java

src/main/java/org/jabref/logic/importer/util/MetaDataParser.java

src/test/java/org/jabref/logic/biblog/BibLogPathResolverTest.java

trag-bot · 2025-04-15T11:06:49Z

src/test/java/org/jabref/logic/biblog/BibLogPathResolverTest.java

+    }
+
+    @Test
+    void returnsDefaultBlgPathWhenUserPathIsAbsent() {


The test method names are descriptive enough, so using @DisplayName is unnecessary. The method name should be comprehensive enough to describe the test case.

trag-bot · 2025-04-15T11:06:52Z

src/test/java/org/jabref/logic/biblog/BibLogPathResolverTest.java

+    }
+
+    @Test
+    void returnsEmptyWhenNoUserPathAndNoBibPath() {


The test method names are descriptive enough, so using @DisplayName is unnecessary. The method name should be comprehensive enough to describe the test case.

trag-bot · 2025-04-15T11:08:41Z

src/test/java/org/jabref/logic/importer/util/MetaDataParserTest.java

+     * Ensures that the trailing semicolon (used as separator in .bib metadata) is handled and stripped properly.
+     */
+    @Test
+    void parsesUserSpecificBlgPathSuccessfully() throws Exception {


The method name 'parsesUserSpecificBlgPathSuccessfully' is not comprehensive enough. It should describe the test case more clearly without needing a @DisplayName annotation.

InAnYan · 2025-04-16T15:18:35Z

src/main/java/org/jabref/gui/integrity/BibLogSettingsPane.java

+    private Runnable onBlgPathChanged;
+
+    public void initializeViewModel(BibDatabaseContext context, Runnable onBlgPathChanged) throws JabRefException {
+        if (context == null || context.getMetaData() == null) {


is it necessary to check for nulls here?

InAnYan · 2025-04-16T15:37:20Z

src/main/java/org/jabref/gui/integrity/BibLogSettingsViewModel.java

+
+    public Path getInitialDirectory() {
+        return bibPath.flatMap(path -> Optional.ofNullable(path.getParent()))
+                      .orElse(Path.of(System.getProperty("user.home")));


Is this cross-platform ("user.home")

Not on Windows XP, but this is acceptable: https://stackoverflow.com/q/585534/873282

wanling0000 · 2025-04-17T10:50:10Z

I only meant to leave a single comment here, but I think I accidentally triggered a full review by clicking the wrong button 😅

Also, I noticed that the Trag review has been marked as “in progress” for two days now, not sure if that’s expected or if something got stuck?

subhramit · 2025-04-17T17:20:18Z

Also, I noticed that the Trag review has been marked as “in progress” for two days now, not sure if that’s expected or if something got stuck?

It sometimes does that - pushing another commit refreshes it.

koppor

some small comments towards a really JabRef-high-quality code 😅

koppor · 2025-04-17T22:28:01Z

src/main/java/org/jabref/gui/integrity/BibLogSettingsViewModel.java

+/**
+ * 1. Connects MetaData with the view.
+ * 2. Wraps .blg warnings as IntegrityMessages.
+ * 3. Supports file browsing and reset actions.
+ */


Either html or Markdown - I propose markdown

Suggested change

/**

* 1. Connects MetaData with the view.

* 2. Wraps .blg warnings as IntegrityMessages.

* 3. Supports file browsing and reset actions.

*/

/// 1. Connects MetaData with the view.

/// 2. Wraps .blg warnings as IntegrityMessages.

/// 3. Supports file browsing and reset actions.

koppor · 2025-04-17T22:28:14Z

src/main/java/org/jabref/gui/integrity/BibLogSettingsViewModel.java

+            this.path.set(resolvedPath.toString());
+            if (metaData.getBlgFilePath(user).isEmpty()) {
+                metaData.setBlgFilePath(user, resolvedPath);
+                this.lastResolvedBlgPath = Optional.of(resolvedPath);
+            }


indent seems to be off

koppor · 2025-04-17T22:30:53Z

src/main/java/org/jabref/logic/biblog/BibLogPathResolver.java

+/**
+ * Resolves custom or default .blg path for this library.
+ *
+ * Priority:
+ * 1. User-defined path from MetaData
+ * 2. Default: same name as .bib file with .blg extension
+ */


Suggested change

/**

* Resolves custom or default .blg path for this library.

*

* Priority:

* 1. User-defined path from MetaData

* 2. Default: same name as .bib file with .blg extension

*/

/// Resolves custom or default .blg path for this library.

///

/// Priority:

///

/// 1. User-defined path from MetaData

/// 2. Default: same name as .bib file with .blg extension

koppor · 2025-04-17T22:31:29Z

src/main/java/org/jabref/logic/biblog/BibWarningToIntegrityMessageConverter.java

+/**
+ * Converts {@link BibWarning}s into {@link IntegrityMessage}s for integration with the existing integrity check UI.
+ *
+ * Notes:
+ * - The current IntegrityMessage interface expects a {@link BibEntry} and a {@link Field},
+ *   but .blg warnings come from a different source and may not include a field.
+ * - For now, we map missing fields to a placeholder (InternalField.KEY_FIELD) to make it compatible with the UI.
+ * - This is a minimal MVP solution to reuse the Integrity tab without changing the UI structure.
+ *
+ * Future direction:
+ * - Consider defining a proper interface (e.g., IntegrityMessageWithField / WithoutField)
+ *   to support warnings without fields cleanly.
+ */


Please also convert to Markdown (to have the notes list rendered properly)

koppor · 2025-04-17T22:32:25Z

src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java

+        metaData.getBlgFilePaths().forEach((user, path) -> {
+            stringyMetaData.put(MetaData.BLG_FILE_PATH + "-" + user, Collections.singletonList(path.toString().trim()));
+        });


I think, the other meta data fields do it, too.

I am OK without constant - follow-up can refactor the whole MetaDataParser ^^

koppor · 2025-04-17T22:34:01Z

src/test/java/org/jabref/logic/biblog/BibWarningToIntegrityMessageConverterTest.java

+        assertEquals(2, messages.size());
+
+        IntegrityMessage msg1 = messages.getFirst();
+        assertEquals("empty journal", msg1.message());
+        assertEquals(firstEntry, msg1.entry());
+        assertEquals("journal", msg1.field().getName());
+
+        IntegrityMessage msg2 = messages.get(1);
+        assertEquals("empty year", msg2.message());
+        assertEquals(secondEntry, msg2.entry());
+        assertEquals("year", msg2.field().getName());


Is it possible to compare two lists directly? Or are IntegrityMessage objects not good with equals?

… logic

trag-bot · 2025-04-18T09:59:06Z

src/main/java/org/jabref/gui/integrity/BibLogSettingsViewModel.java

+        BibLogPathResolver.resolve(metaData, bibPath, user)
+                          .ifPresent(resolvedPath -> {
+                              this.path.set(resolvedPath.toString());
+                              if (metaData.getBlgFilePath(user).isEmpty()) {
+                                  metaData.setBlgFilePath(user, resolvedPath);
+                                  this.lastResolvedBlgPath = Optional.of(resolvedPath);
+                              }
+                          });


The code does not follow the fail-fast principle. It should return early if the condition is not met, instead of nesting logic inside an if statement.

trag-bot · 2025-04-18T09:59:07Z

src/main/java/org/jabref/gui/integrity/BibLogSettingsViewModel.java

+    public void resetBlgFilePath() {
+        metaData.clearBlgFilePath(user);
+        userManuallySelectedBlgFile = false;
+        Optional<Path> resolved = BibLogPathResolver.resolve(metaData, bibPath, user);
+        if (resolved.isEmpty()) {
+            path.set("");
+            lastResolvedBlgPath = Optional.empty();
+            return;
+        }
+
+        Path resolvedPath = resolved.get();
+        path.set(resolvedPath.toString());
+        lastResolvedBlgPath = Optional.of(resolvedPath);
+    }


The method does not follow the fail-fast principle. It should return early if the condition is not met, instead of nesting logic inside an if statement.

trag-bot · 2025-04-18T09:59:49Z

src/main/java/org/jabref/logic/biblog/BibWarningToIntegrityMessageConverter.java

+            if (context.getDatabase().getEntryByCitationKey(bibWarning.entryKey()).isEmpty()) {
+                continue;
+            }


The code should follow the fail fast principle by immediately handling invalid states and returning early instead of nesting logic inside else branches. This code should return early if the entry is not found.

trag-bot · 2025-04-18T10:25:48Z

src/test/java/org/jabref/logic/biblog/BibWarningToIntegrityMessageConverterTest.java

+                new IntegrityMessage("empty year", secondEntry, FieldFactory.parseField("year"))
+        );
+
+        assertEquals(expectedMessages, actualMessages);


The test uses assertEquals to compare lists of objects. It's better to assert the contents of objects directly to ensure more precise validation of the test results.

koppor · 2025-04-18T10:47:51Z

@wanling0000 One last thing: submodules src/main/resources/csl-styles were modified - can you follow https://devdocs.jabref.org/code-howtos/faq.html#submodules to fix?

trag-bot · 2025-04-18T11:20:23Z

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

wanling0000 · 2025-04-18T11:24:37Z

@wanling0000 One last thing: submodules src/main/resources/csl-styles were modified - can you follow https://devdocs.jabref.org/code-howtos/faq.html#submodules to fix?

Fixed, thanks for the reminder!

subhramit · 2025-04-18T12:03:56Z

@wanling0000 good work!

Add .blg warning support in IntegrityCheckDialog with Browse/Reset an…

aab2f21

…d path resolution

trag-bot bot reviewed Mar 31, 2025

View reviewed changes

Siedlerchr reviewed Mar 31, 2025

View reviewed changes

koppor reviewed Mar 31, 2025

View reviewed changes

Siedlerchr reviewed Mar 31, 2025

View reviewed changes

trag-bot bot reviewed Mar 31, 2025

View reviewed changes

wanling0000 added 6 commits April 1, 2025 22:04

Apply review suggestions

a4b657f

Merge remote-tracking branch 'upstream/main' into feature/11998-blg-i…

d1e4eb1

…ntegrity

Apply changes based on review feedback

753cadc

add missing localization key for BibTeX log files

a3b6847

apply OpenRewrite suggestions

6760ced

Fix preferences and Reset logic for .blg file settings

b694915

trag-bot bot reviewed Apr 2, 2025

View reviewed changes

src/main/java/org/jabref/gui/integrity/BibLogSettingsViewModel.java Outdated Show resolved Hide resolved

trag-bot bot reviewed Apr 2, 2025

View reviewed changes

src/test/java/org/jabref/logic/exporter/MetaDataSerializerTest.java Show resolved Hide resolved

trag-bot bot reviewed Apr 2, 2025

View reviewed changes

src/test/java/org/jabref/logic/importer/util/MetaDataParserTest.java Show resolved Hide resolved

Apply review suggestions

f86643b

trag-bot bot reviewed Apr 2, 2025

View reviewed changes

Apply review suggestions

64301be

trag-bot bot reviewed Apr 2, 2025

View reviewed changes

src/main/java/org/jabref/gui/integrity/BibLogSettingsPane.java Show resolved Hide resolved

Fix failing unit tests

6f2aab3

trag-bot bot reviewed Apr 2, 2025

View reviewed changes

Merge branch 'main' into feature/11998-blg-integrity

907fb0c

wanling0000 marked this pull request as ready for review April 7, 2025 11:52

wanling0000 marked this pull request as draft April 7, 2025 11:53

trag-bot bot reviewed Apr 7, 2025

View reviewed changes

src/main/java/org/jabref/gui/integrity/BibLogSettingsViewModel.java Outdated Show resolved Hide resolved

trag-bot bot reviewed Apr 15, 2025

View reviewed changes

src/main/java/org/jabref/gui/integrity/BibLogSettingsViewModel.java Outdated Show resolved Hide resolved

trag-bot bot reviewed Apr 15, 2025

View reviewed changes

src/main/java/org/jabref/logic/importer/util/MetaDataParser.java Show resolved Hide resolved

trag-bot bot reviewed Apr 15, 2025

View reviewed changes

src/test/java/org/jabref/logic/biblog/BibLogPathResolverTest.java Show resolved Hide resolved

trag-bot bot reviewed Apr 15, 2025

View reviewed changes

wanling0000 requested a review from koppor April 15, 2025 13:02

InAnYan reviewed Apr 16, 2025

View reviewed changes

koppor requested changes Apr 17, 2025

View reviewed changes

wanling0000 added 2 commits April 17, 2025 23:39

Remove unnecessary @nonnull annotations and fix misleading UI warning…

a75431b

… logic

Apply review suggestions: simplify test and update Javadoc

a629dbb

trag-bot bot reviewed Apr 18, 2025

View reviewed changes

Resolve merge conflict with upstream/main

b1ae65b

trag-bot bot reviewed Apr 18, 2025

View reviewed changes

wanling0000 requested a review from koppor April 18, 2025 10:36

Revert unintended submodule change to csl-styles

19a7c5f

koppor approved these changes Apr 18, 2025

View reviewed changes

koppor added this pull request to the merge queue Apr 18, 2025

Merged via the queue into JabRef:main with commit ccc379c Apr 18, 2025
36 checks passed

github-actions bot mentioned this pull request Apr 18, 2025

Check integrity: Support for BibTeX .blg files #11998

Closed

4 tasks


		List<IntegrityMessage> messages = BibWarningToIntegrityMessageConverter.convert(warnings, context);

		assertEquals(1, messages.size());

Add support for loading .blg warnings in Integrity Check Dialog #12866

Add support for loading .blg warnings in Integrity Check Dialog #12866

Conversation

wanling0000 commented Mar 31, 2025 • edited Loading

Users can now:

Change Overview

Screenshots

❓Limitations of .blg Parsing – I would appreciate feedback

Mandatory checks

trag-bot bot Mar 31, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trag-bot bot Mar 31, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 31, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trag-bot bot Apr 2, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trag-bot bot Apr 2, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 2, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koppor commented Apr 4, 2025

wanling0000 commented Apr 4, 2025

trag-bot bot Apr 15, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 15, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wanling0000 commented Apr 17, 2025

subhramit commented Apr 17, 2025

koppor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trag-bot bot Apr 18, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 18, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 18, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 18, 2025

Choose a reason for hiding this comment

koppor commented Apr 18, 2025

trag-bot bot commented Apr 18, 2025

wanling0000 commented Apr 18, 2025

subhramit commented Apr 18, 2025

wanling0000 commented Mar 31, 2025 •

edited

Loading