Refactor PagesChecker: Extract duplicate regex constants by Zeglow · Pull Request #14567 · JabRef/jabref

Zeglow · 2025-12-10T18:26:51Z

Refactors PagesChecker by extracting duplicated regex patterns for page numbers and separators into named constants (e.g., SINGLE_PAGE_PATTERN).

Steps to test

Please run the tests:

PagesCheckerBibtexTest
PagesCheckerBiblatexTest

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
I manually tested my changes in running JabRef (always required)
[/] I added JUnit tests for changes (if applicable)
[/] I added screenshots in the PR description (if change is visible to the user)
[/] I described the change in CHANGELOG.md in a way that is understandable for the average user (if change is visible to the user)
I checked the user 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 updating file(s) in https://github.com/JabRef/user-documentation/tree/main/en.

Fixes JabRef#14562

calixtus · 2025-12-10T18:32:53Z

+                    "\\A"                       // begin String
+                    + SINGLE_PAGE_PATTERN
                    + "("
-                    + "(\\+|-{2}|\u2013)"   // separator, must contain exactly two dashes
-                    + "[A-Za-z]?\\d*"       // optional prefix and number
+                    + BIBTEX_RANGE_SEPARATOR
+                    + SINGLE_PAGE_PATTERN
                    + ")?"
-                    + "\\z";                // end String
+                    + "\\z";            // end String

    // See https://packages.oth-regensburg.de/ctan/macros/latex/contrib/biblatex/doc/biblatex.pdf#subsubsection.3.15.3 for valid content
    private static final String PAGES_EXP_BIBLATEX =
-            "\\A"               // begin String
-                    + "[A-Za-z]?\\d*"     // optional prefix and number
+                    "\\A"                       // begin String
+                    + SINGLE_PAGE_PATTERN
                    + "("
-                    + "(\\+|-{1,2}|\u2013)" // separator
-                    + "[A-Za-z]?\\d*"     // optional prefix and number
+                    + BIBLATEX_RANGE_SEPARATOR
+                    + SINGLE_PAGE_PATTERN
                    + ")?"
-                    + "\\z";              // end String
+                    + "\\z";            // end String


Comments dont align on indentation. Please fix

Thank you for the feedback.

koppor

This PR is causing effort on our team.

WE DO NOT HAVE ENDLESS TIME.

Please respect our time.

Please try to spend more thoughts on the PRs.

koppor · 2025-12-10T18:37:56Z

-                    + "[A-Za-z]?\\d*"       // optional prefix and number
+                    "\\A"                       // begin String
+                    + SINGLE_PAGE_PATTERN
                    + "("
-                    + "(\\+|-{2}|\u2013)"   // separator, must contain exactly two dashes
-                    + "[A-Za-z]?\\d*"       // optional prefix and number
+                    + BIBTEX_RANGE_SEPARATOR
+                    + SINGLE_PAGE_PATTERN
                    + ")?"
-                    + "\\z";                // end String


Comments ARE NOT KEPT. Comments are deleted.

Refactoring is not about just making the code nicer. IT IS ABOUT KEEPING CODE COMMENTS - meaning: KEEPING THE GUIDANCE FOR DEVELOPERS.

Thank you for the feedback! I totally understand, and sorry for the inconvenience. I have restored the comments as requested. However, I noticed that some unrelated files might have been included during the sync with the upstream branch. I still need some time to figure it out.

… into clean-fix

…low/jabref into clean-fix" This reverts commit 788979c, reversing changes made to a4a0236.

This reverts commit a4a0236.

…sChecker.java

koppor · 2025-12-10T23:04:07Z

+    // optional prefix and number
+    private static final String SINGLE_PAGE_PATTERN = "[A-Za-z]?\\d*";
+
+    // separator, must contain exactly two dashes
+    private static final String BIBTEX_RANGE_SEPARATOR = "(\\+|-{2}|\u2013)";
+    // separator
+    private static final String BIBLATEX_RANGE_SEPARATOR = "(\\+|-{1,2}|\u2013)";


Do you see why the formatting is consistent?

Thank you for the feedback and your patience!

I realize I was lacking awareness of the formatting conventions. I was uncertain about the comment placement, and after checking other files in the codebase, I noticed some used above-line comments, which is why I went with that style.

I've now updated the formatting. If there are any other issues, please feel free to point them out.

Thank you for your time!

koppor

Even though the comments on line 19 and 25 are not well algined, I approve to be able to focus on other PRs.

Thank you for your time.

* Refactor PagesChecker: Extract duplicate regex constants Fixes JabRef#14562 * fix comments dont align on indentation * Restore regex comments * Revert "Merge branch 'refactor-pages-clean' of https://github.com/Zeglow/jabref into clean-fix" This reverts commit 788979c, reversing changes made to a4a0236. * Revert "Restore regex comments" This reverts commit a4a0236. * restore MainTablePreferences.java, JabRefGuiPreferences.java and PagesChecker.java * fix comment formatting issue

Zeglow and others added 2 commits December 10, 2025 10:15

Refactor PagesChecker: Extract duplicate regex constants

5dfc27f

Fixes JabRef#14562

Merge branch 'main' into refactor-pages-clean

d6f6be7

github-actions Bot added the status: changes-required Pull requests that are not yet complete label Dec 10, 2025

calixtus reviewed Dec 10, 2025

View reviewed changes

koppor requested changes Dec 10, 2025

View reviewed changes

fix comments dont align on indentation

5092f5a

koppor reviewed Dec 10, 2025

View reviewed changes

Zeglow added 5 commits December 10, 2025 11:02

Restore regex comments

a4a0236

Merge branch 'refactor-pages-clean' of https://github.com/Zeglow/jabref…

788979c

… into clean-fix

Revert "Merge branch 'refactor-pages-clean' of https://github.com/Zeg…

a499265

…low/jabref into clean-fix" This reverts commit 788979c, reversing changes made to a4a0236.

Revert "Restore regex comments"

0624ff6

This reverts commit a4a0236.

restore MainTablePreferences.java, JabRefGuiPreferences.java and Page…

1f7704e

…sChecker.java

github-actions Bot removed the status: changes-required Pull requests that are not yet complete label Dec 10, 2025

koppor requested changes Dec 10, 2025

View reviewed changes

github-actions Bot added the status: changes-required Pull requests that are not yet complete label Dec 10, 2025

fix comment formatting issue

1205433

github-actions Bot removed the status: changes-required Pull requests that are not yet complete label Dec 11, 2025

koppor approved these changes Dec 11, 2025

View reviewed changes

koppor added this pull request to the merge queue Dec 11, 2025

Merged via the queue into JabRef:main with commit 7e1ddc6 Dec 11, 2025
49 checks passed

github-actions Bot mentioned this pull request Dec 11, 2025

Refactor PagesChecker: Extract duplicate regex constants #14562

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor PagesChecker: Extract duplicate regex constants#14567

Refactor PagesChecker: Extract duplicate regex constants#14567
koppor merged 9 commits into
JabRef:mainfrom
Zeglow:refactor-pages-clean

Zeglow commented Dec 10, 2025

Uh oh!

calixtus Dec 10, 2025

Uh oh!

Zeglow Dec 10, 2025

Uh oh!

koppor left a comment

Uh oh!

koppor Dec 10, 2025

Uh oh!

Zeglow Dec 10, 2025

Uh oh!

koppor Dec 10, 2025

Uh oh!

Zeglow Dec 11, 2025

Uh oh!

koppor left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Zeglow commented Dec 10, 2025

Steps to test

Mandatory checks

Uh oh!

calixtus Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Zeglow Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

koppor Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Zeglow Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

koppor Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Zeglow Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants