Skip to content

Add consistency check dialog box#13081

Closed
Bha2912 wants to merge 5 commits into
JabRef:mainfrom
Bha2912:main
Closed

Add consistency check dialog box#13081
Bha2912 wants to merge 5 commits into
JabRef:mainfrom
Bha2912:main

Conversation

@Bha2912

@Bha2912 Bha2912 commented May 9, 2025 •
edited
Loading

Copy link
Copy Markdown

Closes #12487
This PR introduce GUI dialog box that will show consistency check result with progress e.g 1 entry out of 123456...

Screenshot

3BC74052-8AEC-4F41-A946-70720DB8ADE5

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.

trag-bot[bot]
trag-bot Bot reviewed May 9, 2025
View reviewed changes
Comment thread jabgui/src/main/java/org/jabref/gui/consistency/ConsistencyCheckAction.java
trag-bot[bot]
trag-bot Bot reviewed May 9, 2025
View reviewed changes
Comment thread jabgui/src/main/java/org/jabref/gui/consistency/ConsistencyCheckAction.java
trag-bot[bot]
trag-bot Bot reviewed May 9, 2025
View reviewed changes
Comment thread jabgui/src/main/java/org/jabref/gui/consistency/ConsistencyCheckAction.java
@Siedlerchr

Siedlerchr commented May 11, 2025

Copy link
Copy Markdown
Member

Looks ok so far. Please add a changelog entry!

@koppor

koppor commented May 11, 2025

Copy link
Copy Markdown
Member

@Bha2912 Please fix localization - see https://devdocs.jabref.org/code-howtos/localization.html for help


> Task :jablib:test
org.jabref.logic.l10n.LocalizationConsistencyTest

  Test findMissingLocalizationKeys() FAILED

  org.opentest4j.AssertionFailedError: 
  DETECTED LANGUAGE KEYS WHICH ARE NOT IN THE ENGLISH LANGUAGE FILE.
  PASTE THESE INTO THE ENGLISH LANGUAGE FILE "JabRef_en.properties".
  Search for a proper place; typically related keys are grouped together.
  If a similar key is already present, please adapt your language instead of adding load to translators by adding a new key.
  
  Checking consistency...=Checking consistency...
  
   ==> expected: <[]> but was: <[Checking consistency... (../jabgui/src/main/java/org/jabref/gui/consistency/ConsistencyCheckAction.java LANG)]>

@Bha2912 Bha2912 marked this pull request as ready for review May 16, 2025 02:08
trag-bot[bot]
trag-bot Bot reviewed May 16, 2025
View reviewed changes
Comment thread jabgui/src/main/java/org/jabref/gui/consistency/ConsistencyCheckAction.java
Task<BibliographyConsistencyCheck.Result> task = new Task<>() {
protected BibliographyConsistencyCheck.Result call() throws Exception {
BibDatabaseContext databaseContext = stateManager.getActiveDatabase()
.orElseThrow(() -> new NullPointerException("Database null"));

@trag-bot trag-bot Bot May 16, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using exceptions for control flow is not recommended. Instead, handle the absence of a database more gracefully without throwing a NullPointerException.

@koppor koppor May 16, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jsut use get() or - even better: stateManager.getActiveDatabase().ifpresent( ...)

@trag-bot

trag-bot Bot commented May 18, 2025

Copy link
Copy Markdown

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

@jabref-machine

jabref-machine commented May 18, 2025

Copy link
Copy Markdown
Collaborator

You committed your code on the main brach of your fork. This is a bad practice. The right way is to branch out from main, work on your patch/feature in that new branch, and then get that branch merged via the pull request (see GitHub flow).

For this pull request, this is OK. For subsequent pull requests, please start with a different branch with a proper branch name. See CONTRIBUTING.md for more details.

koppor
koppor requested changes May 19, 2025
View reviewed changes

@koppor koppor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please re-read the issue description again

image

You should try to make use of org.jabref.logic.util.BackgroundTask#updateProgress(double, double)

See other code how this is done...

I think, the best example is org.jabref.gui.citationkeypattern.GenerateCitationKeyAction#generateKeysInBackground

image

@Bha2912 Bha2912 marked this pull request as draft May 23, 2025 01:15
@Siedlerchr Siedlerchr self-assigned this May 26, 2025
@Siedlerchr Siedlerchr mentioned this pull request May 28, 2025
Merged
4 tasks
@Siedlerchr

Siedlerchr commented May 28, 2025

Copy link
Copy Markdown
Member

Superseded by #13181

@Siedlerchr Siedlerchr closed this May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@trag-bot trag-bot[bot] trag-bot[bot] left review comments

@koppor koppor koppor requested changes

Assignees

@Siedlerchr Siedlerchr

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Show progress in GUI for consistency check

4 participants

@Bha2912 @Siedlerchr @koppor @jabref-machine