Repository navigation
Conversation
…tabs" Old versions stored the default "General" and "Abstract" tabs as if the user had defined them, so upgrading duplicated fields already shown in the "Main" tab. The migration now drops stored tabs whose field set is one of the former defaults and keeps genuinely customized tabs. Users who want the split get it back via "Add classic tabs" in the entry editor preferences. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
PR Summary by QodoDrop legacy default tabs and restore classic entry editor tabs
AI Description
Diagram
High-Level Assessment
Files changed (19)
|
github-actions
Bot
added
component: internationalization
Code Review by Qodo
1. New labels lack locale keys
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Custom tab names are persisted verbatim, so a classic tab added under one language was added again after switching languages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
|
The right side (which is supposed to be about the fields) now looks a little crammed, especially since there is this comment about regex as well.
Two more unrelated things (could be addressed in a follow up or never, as it's low prio):
|
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
|
🤖 Generated with Claude Code Done, thanks:
The two unrelated points (toggling custom tabs, read-only field list of built-in tabs) are left for a follow-up. |
A stored tab is dropped only when its name is a translation of "General" or "Abstract" and its fields are exactly one of the shipped default sets. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
The classic "General"/"Abstract" tabs are stored as "%General" and "%Abstract" and translated on display, so they follow a language change and have a locale-independent identity; user-typed names stay verbatim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
|
🤖 Generated with Claude Code a8b2268 applies ADR 0023 (Localized Preferences) to the classic tabs: their names are stored as |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
koppor
added
the
status: ready-for-review
github-actions
Bot
added
status: changes-required
koppor
added
status: ready-for-review
github-actions
Bot
added
status: changes-required
…nto drop-default-custom-tabs
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
…nto drop-default-custom-tabs
|
🤖 Generated with Claude Code Updated screenshots (the earlier comparison predates the "Comments" classic tab and is marked outdated). Top: JabRef 5.15 (last release) — the classic editor with Required/Optional/… tabs plus Comments, General, Abstract; below its "General" tab.
|
|
OMG, we had "DOI" at "General"? |
| - We added an option to include currently selected entries when creating a new explicit group. [#16588](https://github.com/JabRef/jabref/pull/16588) | ||
| - We added an offer to initialize a Git repository when committing a library that is not yet version controlled. [#16722](https://github.com/JabRef/jabref/pull/16722) | ||
| - We added OCR language selection to the OCR preferences, allowing users to configure which Tesseract languages to use. [#16618](https://github.com/JabRef/jabref/issues/16618) | ||
| - We added a button "Add classic 5.x tabs" to the entry editor preferences, which adds the tabs "General", "Abstract", and "Comments" known from JabRef 5. [#16672](https://github.com/JabRef/jabref/pull/16672) |
There was a problem hiding this comment.
3. Changelog entry exceeds limit 📘 Rule violation ⚙ Maintainability
The added changelog sentence is approximately 25 words before its link, exceeding the mandated maximum of 20 words. This violates the strict unreleased changelog format.
Agent Prompt
## Issue description
The new user-visible changelog sentence exceeds the 20-word maximum required by the changelog compliance rule.
## Issue Context
Keep one sentence under `Unreleased` → `Added`, retain the `We added` prefix, describe only the visible outcome, and preserve an appropriate reference.
## Fix Focus Areas
- CHANGELOG.md[68-68]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
🤖 Generated with Claude Code
Fixed in 73489fa: shortened to 17 words before the link.
|
Code review by qodo was updated up to the latest commit 7c5a93e |
With a Turkish default locale, "FILE".toLowerCase() is "fıle" and the duplicate detection (and the migration default-set match) missed equivalent field names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
A user-typed %-prefixed tab name is shown verbatim instead of being treated as a localization key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
An "Abstract"-named tab holding the General field set (or vice versa) was never a shipped default and is kept as a customization. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
| This\ tab\ has\ no\ fields\ yet.=Cet onglet n'a pas encore de champs. | ||
| The\ field\ is\ contained\ in\ multiple\ tabs.=Le champ apparaît dans plusieurs onglets. | ||
| Remove\ field\ %0\ from\ currently\ selected\ tab=Supprimer le champ %0 de l'onglet actuellement sélectionné | ||
| A\ field\ name\ can\ also\ be\ a\ regular\ expression,\ e.g.\ "comment-.*".=Un nom de champ peut également être une expression régulière, par exemple "comment-.*". |
There was a problem hiding this comment.
2. Translated bundles edited manually 📘 Rule violation ⚙ Maintainability
The PR manually removes an entry from the French and Italian translation bundles. JabRef's Crowdin workflow requires localization-source changes to remain confined to JabRef_en.properties.
Agent Prompt
## Issue description
The PR directly edits translated localization bundles by deleting the obsolete regular-expression help key.
## Issue Context
PR Compliance ID 20 requires translated property files to remain unchanged so Crowdin can manage them.
## Fix Focus Areas
- jablib/src/main/resources/l10n/JabRef_fr.properties[2225-2225]
- jablib/src/main/resources/l10n/JabRef_it.properties[2222-2222]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
🤖 Generated with Claude Code
Fixed in 73489fa: the hand-removed fr/it lines are restored; only JabRef_en.properties is edited, Crowdin will retire the obsolete translations.
| Locale defaultLocale = Locale.getDefault(); | ||
| try { | ||
| Locale.setDefault(Locale.of("tr", "TR")); |
There was a problem hiding this comment.
3. Locale mutation lacks resource lock 📘 Rule violation ☼ Reliability
The new test changes the process-wide default Locale without acquiring the repository's Localization.lang resource lock. Under parallel execution it can race localization-dependent tests and cause nondeterministic results despite restoring the locale afterward.
Agent Prompt
## Issue description
The test mutates the JVM-wide default locale without coordinating with other parallel tests.
## Issue Context
Existing localization tests use JUnit's `@ResourceLock("Localization.lang")`; apply the same lock to this test method or class while retaining restoration in `finally`.
## Fix Focus Areas
- jabgui/src/test/java/org/jabref/gui/preferences/entryeditor/EntryEditorTabViewModelTest.java[73-90]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
🤖 Generated with Claude Code
Fixed in 73489fa: the locale-mutating test is annotated @ResourceLock(Resources.LOCALE).
| assertFalse(names.contains(general.displayName())); | ||
| assertTrue(names.contains(Localization.lang("Abstract"))); |
There was a problem hiding this comment.
4. Tab test asserts predicates 📘 Rule violation ⚙ Maintainability
The new test validates tab contents through assertFalse(names.contains(...)) and assertTrue(names.contains(...)) instead of directly comparing expected values. This produces less useful diagnostics and violates the prescribed object-content assertion style.
Agent Prompt
## Issue description
The test asserts computed `contains` predicates rather than the expected tab-name contents.
## Issue Context
PR Compliance ID 24 requires direct content or value assertions such as `assertEquals` when validating object state. Build the relevant actual tab-name collection and compare it to the expected names.
## Fix Focus Areas
- jabgui/src/test/java/org/jabref/gui/preferences/entryeditor/EntryEditorTabViewModelTest.java[88-90]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
🤖 Generated with Claude Code
Fixed in 73489fa: the test now asserts the complete expected tab-name list via assertEquals.
|
Code review by qodo was updated up to the latest commit d77c616 |
|
|
? |
|
Koppor asked me to review. |
|
I just require information what it is you want to talk about. Just screenshots are very mysterious, but not very helpful. And I won't be in the devcall tonight, im on vacation. |
- Locate the "Main" tab by built-in type, not display name (a custom tab may be named "Main") - Assert the full tab list instead of contains-checks; guard the locale mutation with @ResourceLock(Resources.LOCALE) - @NullMarked on the new test class; shorten the CHANGELOG entry to the 20-word limit; restore the Crowdin-managed fr/it lines removed by hand Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rutjr6iPghPfrMLgDzNLFL
|
DevCall: User whish wrongly understood. #16598 introduced the issue. |
|
🤖 Generated with Claude Code Superseded by #16780: only the migration dropping the former default tabs is kept (now also covering JabRef ≤ 4.x defaults), without the "Add classic 5.x tabs" button — users upgrading from a default configuration get the reworked entry editor as-is. |
Summary
🤖 Upgrading from JabRef 5.x or 6.0-alpha showed the former default tabs "General" and "Abstract" next to "Main", although "Main" already contains their fields. Upgrading now drops these former default tabs while keeping genuinely customized tabs. A new "Add classic 5.x tabs" button in the entry editor preferences brings back "General", "Abstract", and "Comments"; fields shown on a custom tab are omitted from "Main", while fields of the entry type always stay there.
jabref-contrib-policy:4.2:reviewed:okAnalogies: Like honey, this only keeps what was actually made by the user. Like chocolate, the classic tabs are still there for those who crave them — one click away. Like the moon, "Main" reflects all those fields already, so the duplicates can wane.
Steps to test
Mine: keywords;comment-.*) stays.A side-by-side comparison with the JabRef 5.15 entry editor is in this comment.
Related issues and pull requests
Follow-up to #16598; supersedes #16669.
AI usage
Claude Code (model claude-fable-5), AIL4: code, tests, and this description were generated by the AI under my guidance and reviewed by me.
AI CHECKLIST.md walkthrough
1. Code self-review
!= nullon a nullable backing-store read, pre-existing pattern in that method).JabRef_en.properties).2. Verification commands
./gradlew :jabgui:testfor the touched test classes../gradlew :jabgui:checkstyleMain :jabgui:checkstyleTest../gradlew rewriteRun— no changes.3. Documentation
CHANGELOG.md"Added" entry for the button; the migration fixes an unreleased regression, so no "Fixed" entry.4. Pull request
--body-file.TODOplaceholder inCHANGELOG.md.Checklist
CHANGELOG.mdin a way that can be understood by the average user (if change is visible to the user)🤖 Generated with Claude Code