Repository navigation
Conversation
Removing the entry and inserting the disk version gave it a new identity, so an open entry editor kept showing the removed entry. The fields, type, and comments of the existing entry are now changed instead, recorded as undoable steps, together with the entry's changed flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 tasks done
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Accepting a change to an entry in the "External changes" dialog removed the entry and inserted the version from disk as a new entry. An open entry editor kept showing the removed entry, so it did not update, and edits made in it afterwards were lost on save. The table selection and position were lost too. Now the existing entry is changed in place: type, fields and the comments before the entry are updated as undoable steps, so the entry keeps its identity.
Undo also restores the entry's "changed" flag. Without that, an undone merge would still cause the entry to be re-serialized on the next save instead of being written back as it was read.
This was carved out of #16813, which depends on it. The behaviour on its own is not tied to file synchronization, and the split keeps that PR smaller.
Analogies: This PR is like honey: it stays where it is put instead of being scooped out and poured back in. Like chocolate, it should melt into the existing dialog without changing its shape. And like the moon, the entry keeps showing the same face to the editor, however much changes around it.
jabref-contrib-policy:4.2:reviewed:okSteps to test
Copy
jablib/src/main/resources/Chocolate.bibto a temporary folder and open the copy in JabRef.Double-click
Corti_2009to open the entry editor.In a text editor, change its title to
Cocoa and Cardiovascular Health: Changed in an External Editorand save the file.In JabRef, click the notification "External changes detected" and then "Review changes".
In the "External Changes Resolver", select "Modified entry 'Corti_2009'" and click "Accept".
The entry editor shows the new title, and
Corti_2009stays selected in the table.On
main: the entry editor still shows the old title,Corti_2009moved away from the top of the table, and the selection jumped toCooper_2007:With this PR:
Click "Undo" in the toolbar (or press Ctrl + Z in the table): the title goes back to
Cocoa and Cardiovascular Healthin the table and in the entry editor.Related issues and pull requests
Closes #8901
Split off from #16813
AI usage
Claude Code (model claude-opus-5-5), AIL4: the code was written by the AI under my direction and reviewed by me.
AI CHECKLIST.md walkthrough
1. Code self-review
Nullability and control flow
== null/!= nullchecks — JSpecify annotations (@NullMarked,@Nullable,@NonNull) used instead. (The one!= nullinUndoableChangedFlagreads its own@Nullablerecord component.)Objects.requireNonNull(...)— nullability expressed via JSpecify annotations.@NullMarked(org.jspecify.annotations.NullMarked).@NullMarked(org.jspecify.annotations.NullMarked).Optionalconsumed withifPresent/ifPresentOrElse/map/orElseThrow— neverorElse(unusedValue)nor anisPresent()+get()block. (orElse(null)only whereUndoableFieldChangetakes@Nullablevalues.)StringUtil.isBlank(...)used instead ofs == null || s.isBlank().Exceptions
catch (Exception e)— only specific exceptions are caught.throw new RuntimeException(...)/IllegalStateException(...)— these tear down the whole application.LOGGER.info("...", e)), not concatenated into the message string.Style and idioms
BibEntryobjects built with withers (withField, notsetField).List.of()/Map.of()/Set.of(),Path.of(),SequencedCollection/SequencedSet, text blocks.Pattern.compile(...)constant, notString.matches(...).permitslist is grouped by kind; the new types sit with the other entry changes.)org.jabref.logic.util.BackgroundTask, notnew Thread().///) uses Markdown syntax, not JavaDoc inline tags:`code`instead of{@code},[ClassName]instead of{@link}.User-facing text
Localization.langin Java,%prefix in FXML).!; labels do not end with:."...: %0"), not string concatenation.Security
text/htmlresponse — including exception/error messages, not just the success body (XSS).Tests
EntryChangeTestcovers the case with its own entries: the open entry stays the instance in the library and takes over the disk content.)org.jabref.model/org.jabref.logichave added or updated tests.assertEquals), use plain JUnit asserts (not AssertJ), have no@DisplayName, do not catch exceptions (let them propagate so JUnit reports setup/teardown failures directly), and use@TempDirinstead of manual temp directories.2. Verification commands
./gradlew :jablib:check(or./gradlew checkfor all modules). (Stopped after thejablibtests had run for several minutes without a failure; CI runs the full suite.)./gradlew checkstyleMain checkstyleTest checkstyleJmh../gradlew modernizer../gradlew --no-configuration-cache :rewriteDryRunreports no changes (run./gradlew rewriteRunto fix)../gradlew javadoc.npx markdownlint-cli2 "docs/**/*.md" "*.md"(only if Markdown changed).npm ci && npm run textlintreports no misspellings (only if Markdown changed).rewriteRun:docker run -v $(pwd):/github/workspace ghcr.io/leventebajczi/intellij-format:master "*.java" "" ".idea/codeStyles/Project.xml".3. Documentation
CHANGELOG.mdentry added if the change is visible to the user (end-user wording, no extra blank lines, sorted in next to existing entries about the same component/feature). Link the issue if one exists; link the PR only when no issue exists. UseTODOas the placeholder when neither is known yet — never a fake number. No entry for fixes to changes that were themselves introduced after the last release (feature only in## [Unreleased]) — update the existing unreleased entry instead if needed.TODO(nocloses/fixesfor merely-similar issues).docs/requirements/<area>.mdif the change is a new feature or significant bug fix (skip for refactors, minor fixes, and internal changes). (req~ux.external-library-changes.entry-in-place~1indocs/requirements/ux.md.)docs/updated if behavior or architecture changed.4. Pull request
.github/PULL_REQUEST_TEMPLATE.md, every section filled.[x],[ ], or[/].gh pr create --body-file <file>(not--body).CHANGELOG.mdused aTODOplaceholder (no issue confidently identified yet — an existing issue link always stays), the PR was opened as draft, the placeholder was replaced with the real PR-number link after PR creation, committed and pushed, and only then was the PR marked ready for review. If an issue is identified or created later, the link is switched to the issue.Checklist
CHANGELOG.mddescribing the change from the user's point of view (if the change is visible to the user)🤖 Generated with Claude Code