Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Opening a file removes duplicate keywords and triggers lib changed #9216

Merged
merged 8 commits into from
Oct 10, 2022
Merged

Opening a file removes duplicate keywords and triggers lib changed #9216

merged 8 commits into from
Oct 10, 2022

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented Oct 5, 2022 •
edited by ThiloteE
Loading

Refs and I will hopefully find a way to fixes #9187
Fixed #8586

After a while debugging and adding tests everywhere I finally found the culprit. The keyword special field migration calls
removeKeywords on an entry and that internally calls getKeywords which returns only a Set and readds those
=> As this is now a Set: Poof our duplicate keywords are gone and the library has changed

Questions to decide: Remove duplicate keywords on save? Probably easiest solution
Refactor migration to keep duplicates?

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • 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.

@Siedlerchr
Add tests for duplicate keywords
e032096
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into duplicateKeywords
c1da353 
* upstream/main:
  disable conversions for empty fields (#9202)
  Update lucene version to 9.4 (#9213)
  Bump slf4j-api from 2.0.2 to 2.0.3 (#9207)
  Bump slf4j-api from 2.0.2 to 2.0.3 in /buildSrc (#9211)
  Add mappings on ModsImporter.java (#9193)
@Siedlerchr
add another test
3028778
@Siedlerchr
Found the culprit
047d204
@tobiasdiez
Copy link
Member

Nice!

My opinion: the writer should write, not modify anything. All cleanup / migrations / other changes should happen before the actual write and directly (and only) modify the in-memory library.

@Siedlerchr
Copy link
Member Author

I generally agree with you. Writing and reading the bibtex is not the problem. The migration is done on startup. But I will check if I can find a way to make a set of the keywords before writing

koppor and others added 2 commits October 10, 2022 20:58
@koppor
Merge remote-tracking branch 'origin/main' into duplicateKeywords
df37902
@koppor @Siedlerchr
@calixtus @HoussemNasri
Add workaround for set "modification"
6d4157d 
Co-authored-by: Christoph <siedlerkiller@gmail.com>
Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>
Co-authored-by: Houssem Nasri <housi.housi2015@gmail.com>
@Siedlerchr Siedlerchr changed the title WIP: Opening a file removes duplicate keywords and triggers lib changed Opening a file removes duplicate keywords and triggers lib changed Oct 10, 2022
@koppor @Siedlerchr
@calixtus @HoussemNasri @ThiloteE
Adapt test
413b91a 
Co-authored-by: Christoph <siedlerkiller@gmail.com>
Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>
Co-authored-by: Houssem Nasri <housi.housi2015@gmail.com>
Co-authored-by: ThiloteE <73715071+ThiloteE@users.noreply.github.com>
@Siedlerchr Siedlerchr merged commit f63999e into main Oct 10, 2022
@Siedlerchr Siedlerchr deleted the duplicateKeywords branch October 10, 2022 20:41
Siedlerchr added a commit that referenced this pull request Oct 13, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into restapi
607cb2a 
* upstream/main: (42 commits)
  Place subgroups w.r.t. alphabetical ordering (#9228)
  DOAB Fetcher now fetches ISBN and imprint fields where possible (#9229)
  ISSUE-9145: implement isbn fetcher (#9205)
  Remove explicit javafx jmod stuff (#9245)
  New Crowdin updates (#9239)
  Support Arabic language (#9243)
  Process CLI arguments before starting gui (#9217)
  Update README Decision Records link (#9241)
  Opening a file removes duplicate keywords and triggers lib changed (#9216)
  New Crowdin updates (#9237)
  Add Ukrainian as language (#9236)
  Add ADR-0026 on using Swing's FileChooser (#9235)
  Log exception (#9227)
  Fix archunit (#9234)
  Bump tika-core from 2.4.1 to 2.5.0 (#9232)
  Check group type before showing dialog in edit group (#8909)
  Fix unexpected closing of preferences dialog (#9225)
  Fix typo in connection error message (#9226)
  Improve VSCode config (#9218)
  Searching for an entry on the web shouldn't take forever (#9199)
  ...

# Conflicts:
#	build.gradle
#	lib/afterburner.fx.jar
#	src/main/java/org/jabref/cli/Launcher.java
#	src/main/java/org/jabref/gui/EntryTypeView.java
#	src/main/java/org/jabref/gui/auximport/FromAuxDialog.java
#	src/main/java/org/jabref/gui/bibtexextractor/ExtractBibtexDialog.java
#	src/main/java/org/jabref/gui/collab/ExternalChangesResolverDialog.java
#	src/main/java/org/jabref/gui/commonfxcontrols/CitationKeyPatternPanel.java
#	src/main/java/org/jabref/gui/contentselector/ContentSelectorDialogView.java
#	src/main/java/org/jabref/gui/customentrytypes/CustomizeEntryTypeDialogView.java
#	src/main/java/org/jabref/gui/documentviewer/DocumentViewerView.java
#	src/main/java/org/jabref/gui/edit/ManageKeywordsDialog.java
#	src/main/java/org/jabref/gui/entryeditor/EntryEditor.java
#	src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FileAnnotationTabView.java
#	src/main/java/org/jabref/gui/errorconsole/ErrorConsoleView.java
#	src/main/java/org/jabref/gui/exporter/CreateModifyExporterDialogView.java
#	src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogView.java
#	src/main/java/org/jabref/gui/help/AboutDialogView.java
#	src/main/java/org/jabref/gui/importer/ImportCustomEntryTypesDialog.java
#	src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java
#	src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.java
#	src/main/java/org/jabref/gui/libraryproperties/AbstractPropertiesTabView.java
#	src/main/java/org/jabref/gui/libraryproperties/LibraryPropertiesView.java
#	src/main/java/org/jabref/gui/libraryproperties/constants/ConstantsPropertiesView.java
#	src/main/java/org/jabref/gui/libraryproperties/general/GeneralPropertiesView.java
#	src/main/java/org/jabref/gui/libraryproperties/keypattern/KeyPatternPropertiesView.java
#	src/main/java/org/jabref/gui/libraryproperties/saving/SavingPropertiesView.java
#	src/main/java/org/jabref/gui/linkedfile/LinkedFileEditDialogView.java
#	src/main/java/org/jabref/gui/openoffice/ManageCitationsDialogView.java
#	src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java
#	src/main/java/org/jabref/gui/preferences/AbstractPreferenceTabView.java
#	src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java
#	src/main/java/org/jabref/gui/preferences/customexporter/CustomExporterTab.java
#	src/main/java/org/jabref/gui/preferences/externalfiletypes/EditExternalFileTypeEntryDialog.java
#	src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java
#	src/main/java/org/jabref/gui/preferences/keybindings/KeyBindingsTab.java
#	src/main/java/org/jabref/gui/preferences/preview/PreviewTab.java
#	src/main/java/org/jabref/gui/preferences/protectedterms/ProtectedTermsTab.java
#	src/main/java/org/jabref/gui/search/GlobalSearchResultDialog.java
#	src/main/java/org/jabref/gui/shared/SharedDatabaseLoginDialogView.java
#	src/main/java/org/jabref/gui/slr/ManageStudyDefinitionView.java
#	src/main/java/org/jabref/gui/texparser/ParseLatexDialogView.java
#	src/main/java/org/jabref/gui/texparser/ParseLatexResultView.java
@koppor koppor mentioned this pull request Oct 14, 2022
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
hacktoberfest-accepted keywords
Projects
None yet
Milestone
No milestone
4 participants
@Siedlerchr @tobiasdiez @koppor @ThiloteE