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.

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

Fix journal abbbrev checker for curly braces #9504

Merged
merged 45 commits into from
Jan 2, 2023
Merged

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented Dec 28, 2022

Escape any curly braces
Also use latex free fields

Fixes #9475
Fixes #9503

  • 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.

Escape any curly braces
Also use latex free fields


Fixes #9475
Fixes #9503
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Dec 28, 2022
@HoussemNasri
Copy link
Member

HoussemNasri commented Dec 28, 2022

I was able to reproduce even after this pr. Also, try to hold the up or down keyboard arrow to move past entries and you'll notice a major lag in UI (it wasn't introduced here). Apparently, some heavy operation is being executed on the UI thread whenever an entry is selected selected for the first time.

Edit:
The lag is caused by Pattern.compile

foundKey = abbreviationToFull.keySet().stream()
.filter(s -> Pattern.compile(joined).matcher(s).find())
.collect(Collectors.joining());
}

Exception java.util.regex.PatternSyntaxException: Illegal/unsupported escape sequence near index 48 Proceedings[\.\s]*of[\.\s]*the[\.\s]*4\(\{\}^\{\mbox\{th\}\}\)[\.\s]*Central-European[\.\s]*Workshop[\.\s]*on[\.\s]*Services[\.\s]*and[\.\s]*their[\.\s]*Composition,[\.\s]*ZEUS-2012,[\.\s]*Bamberg,[\.\s]*Germany,[\.\s]*February[\.\s]*23-24,[\.\s]*2012[\.\s]* ^ at java.base/java.util.regex.Pattern.error(Pattern.java:2038) at java.base/java.util.regex.Pattern.escape(Pattern.java:2618) at java.base/java.util.regex.Pattern.atom(Pattern.java:2296) at java.base/java.util.regex.Pattern.sequence(Pattern.java:2169) at java.base/java.util.regex.Pattern.expr(Pattern.java:2079) at java.base/java.util.regex.Pattern.compile(Pattern.java:1793) at java.base/java.util.regex.Pattern.(Pattern.java:1440) at java.base/java.util.regex.Pattern.compile(Pattern.java:1079) at org.jabref@100.0.0/org.jabref.logic.journals.JournalAbbreviationRepository.lambda$findDottedAbbrFromDotless$2(JournalAbbreviationRepository.java:109) at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:178) at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133) at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1845) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921) at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682) at org.jabref@100.0.0/org.jabref.logic.journals.JournalAbbreviationRepository.findDottedAbbrFromDotless(JournalAbbreviationRepository.java:110) at org.jabref@100.0.0/org.jabref.logic.journals.JournalAbbreviationRepository.isAbbreviatedName(JournalAbbreviationRepository.java:83) at org.jabref@100.0.0/org.jabref.logic.integrity.AbbreviationChecker.checkValue(AbbreviationChecker.java:23) at org.jabref@100.0.0/org.jabref.gui.fieldeditors.AbstractEditorViewModel.lambda$new$0(AbstractEditorViewModel.java:43) at de.saxsys.mvvmfx.validation/de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator.lambda$new$1(FunctionBasedValidator.java:74) at de.saxsys.mvvmfx.validation/de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator.validate(FunctionBasedValidator.java:102) at de.saxsys.mvvmfx.validation/de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator.lambda$new$0(FunctionBasedValidator.java:57) at javafx.base@19/com.sun.javafx.binding.ExpressionHelper$Generic.fireValueChangedEvent(ExpressionHelper.java:360) at javafx.base@19/com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:80) at javafx.base@19/javafx.beans.property.StringPropertyBase.fireValueChangedEvent(StringPropertyBase.java:104) at javafx.base@19/javafx.beans.property.StringPropertyBase.markInvalid(StringPropertyBase.java:111) at javafx.base@19/javafx.beans.property.StringPropertyBase.set(StringPropertyBase.java:145) at javafx.base@19/javafx.beans.property.StringPropertyBase.set(StringPropertyBase.java:50) at javafx.base@19/javafx.beans.property.StringProperty.setValue(StringProperty.java:71) at javafx.base@19/javafx.beans.property.StringProperty.setValue(StringProperty.java:57) at org.jabref@100.0.0/org.jabref.gui.util.BindingsHelper.bindBidirectional(BindingsHelper.java:80) at org.jabref@100.0.0/org.jabref.gui.util.BindingsHelper.bindBidirectional(BindingsHelper.java:70) at org.jabref@100.0.0/org.jabref.gui.fieldeditors.AbstractEditorViewModel.bindToEntry(AbstractEditorViewModel.java:62) at org.jabref@100.0.0/org.jabref.gui.fieldeditors.SimpleEditor.bindToEntry(SimpleEditor.java:56) at org.jabref@100.0.0/org.jabref.gui.entryeditor.FieldsEditorTab.setupPanel(FieldsEditorTab.java:122) at org.jabref@100.0.0/org.jabref.gui.entryeditor.FieldsEditorTab.bindToEntry(FieldsEditorTab.java:204) at org.jabref@100.0.0/org.jabref.gui.entryeditor.EntryEditorTab.notifyAboutFocus(EntryEditorTab.java:42) at org.jabref@100.0.0/org.jabref.gui.entryeditor.EntryEditor.setEntry(EntryEditor.java:342) at java.base/java.util.Optional.ifPresent(Optional.java:178) at org.jabref@100.0.0/org.jabref.gui.LibraryTab.lambda$createMainTable$10(LibraryTab.java:517) at javafx.base@19/com.sun.javafx.collections.ListListenerHelper$Generic.fireValueChangedEvent(ListListenerHelper.java:329) at javafx.base@19/com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:73) at javafx.base@19/javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:239) at javafx.base@19/javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482) at javafx.base@19/javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541) at javafx.base@19/javafx.collections.ObservableListBase.endChange(ObservableListBase.java:211) at javafx.controls@19/com.sun.javafx.scene.control.SelectedItemsReadOnlyObservableList.lambda$new$0(SelectedItemsReadOnlyObservableList.java:91) at javafx.base@19/com.sun.javafx.collections.ListListenerHelper$Generic.fireValueChangedEvent(ListListenerHelper.java:329) at javafx.base@19/com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:73) at javafx.base@19/javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:239) at javafx.base@19/javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482) at javafx.base@19/javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541) at javafx.base@19/javafx.collections.ObservableListBase.endChange(ObservableListBase.java:211) at javafx.controls@19/com.sun.javafx.scene.control.ReadOnlyUnbackedObservableList._endChange(ReadOnlyUnbackedObservableList.java:64) at javafx.controls@19/javafx.scene.control.MultipleSelectionModelBase$SelectedIndicesList._endChange(MultipleSelectionModelBase.java:918) at javafx.controls@19/javafx.scene.control.ControlUtils.updateSelectedIndices(ControlUtils.java:218) at javafx.controls@19/javafx.scene.control.TableView$TableViewArrayListSelectionModel.fireCustomSelectedCellsListChangeEvent(TableView.java:3032) at javafx.controls@19/javafx.scene.control.TableView$TableViewArrayListSelectionModel.clearAndSelect(TableView.java:2482) at javafx.controls@19/javafx.scene.control.TableView$TableViewSelectionModel.clearAndSelect(TableView.java:1968) at javafx.controls@19/com.sun.javafx.scene.control.behavior.TableViewBehaviorBase.selectCell(TableViewBehaviorBase.java:875) at javafx.controls@19/com.sun.javafx.scene.control.behavior.TableViewBehaviorBase.selectNextRow(TableViewBehaviorBase.java:838) at javafx.controls@19/com.sun.javafx.scene.control.behavior.TableViewBehaviorBase.lambda$new$12(TableViewBehaviorBase.java:156) at javafx.controls@19/com.sun.javafx.scene.control.inputmap.InputMap.handle(InputMap.java:274) at javafx.base@19/com.sun.javafx.event.CompositeEventHandler$NormalEventHandlerRecord.handleBubblingEvent(CompositeEventHandler.java:247) at javafx.base@19/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:80) at javafx.base@19/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:234) at javafx.base@19/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:191) at javafx.base@19/com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59) at javafx.base@19/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58) at javafx.base@19/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at javafx.base@19/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at javafx.base@19/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at javafx.base@19/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at javafx.base@19/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at javafx.base@19/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at javafx.base@19/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at javafx.base@19/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at javafx.base@19/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at javafx.base@19/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at javafx.base@19/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at javafx.base@19/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at javafx.base@19/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at javafx.base@19/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at javafx.base@19/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at javafx.base@19/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74) at javafx.base@19/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54) at javafx.base@19/javafx.event.Event.fireEvent(Event.java:198) at javafx.graphics@19/javafx.scene.Scene$KeyHandler.process(Scene.java:4113) at javafx.graphics@19/javafx.scene.Scene.processKeyEvent(Scene.java:2159) at javafx.graphics@19/javafx.scene.Scene$ScenePeerListener.keyEvent(Scene.java:2627) at javafx.graphics@19/com.sun.javafx.tk.quantum.GlassViewEventHandler$KeyEventNotification.run(GlassViewEventHandler.java:218) at javafx.graphics@19/com.sun.javafx.tk.quantum.GlassViewEventHandler$KeyEventNotification.run(GlassViewEventHandler.java:150) at java.base/java.security.AccessController.doPrivileged(AccessController.java:399) at javafx.graphics@19/com.sun.javafx.tk.quantum.GlassViewEventHandler.lambda$handleKeyEvent$1(GlassViewEventHandler.java:250) at javafx.graphics@19/com.sun.javafx.tk.quantum.QuantumToolkit.runWithoutRenderLock(QuantumToolkit.java:424) at javafx.graphics@19/com.sun.javafx.tk.quantum.GlassViewEventHandler.handleKeyEvent(GlassViewEventHandler.java:249) at javafx.graphics@19/com.sun.glass.ui.View.handleKeyEvent(View.java:542) at javafx.graphics@19/com.sun.glass.ui.View.notifyKey(View.java:966) at javafx.graphics@19/com.sun.glass.ui.win.WinApplication._runLoop(Native Method) at javafx.graphics@19/com.sun.glass.ui.win.WinApplication.lambda$runLoop$3(WinApplication.java:184) at java.base/java.lang.Thread.run(Thread.java:833)

image

@Siedlerchr
Copy link
Member Author

I think the key problem is the integrity checker is not using latex free fields. I noticed that lag as well

@Siedlerchr
Copy link
Member Author

I converted the checker to an entry based checker and this also has the advantage that it's not executed when the entry is created. Only for the integrity check.
I first try to use the latex free field, if this still contains illegal chars I continue with ignoring the exception

@Siedlerchr
Copy link
Member Author

the original fix for the conversion is still buggy. the Regex is too greedy and will return too much

@koppor
Copy link
Member

koppor commented Jan 1, 2023

Apparently, some heavy operation is being executed on the UI thread whenever an entry is selected selected for the first time

This surely is #9172

koppor and others added 2 commits January 1, 2023 22:04
Co-authored-by: Christoph <siedlerkiller@gmail.com>
Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>
@koppor
Copy link
Member

koppor commented Jan 1, 2023

META: Working on fixing this clearly shows why code comments are useful. Especially, the usage of Pattern.compile should have been documented (this is a very expensive operation!). -- The general idea should have been documented in the code @K5qu4r3d.

@koppor
Copy link
Member

koppor commented Jan 1, 2023

Current state:

remote: error: File src/main/resources/journals/journalList.mv is 332.14 MB; this exceeds GitHub's file size limit of 100.00 MB  

Ideas:

  1. Make JabRef distribution size to 500 MB ^^(and enforce git-lfs)
  2. Re-add *.csv in the repository and have JabRef generate the .mv files if the .mv file is older than any *.csv file
  3. Switch back to "String" and have the Abbreviations in-memory

@koppor
Copy link
Member

koppor commented Jan 2, 2023

I implemented another idea - store complete object once in MVStore and create the HashMaps on load

  • Old size: ~15MB
  • New size: ~38MB

@koppor
Copy link
Member

koppor commented Jan 2, 2023

New menu (MEDLINE -> dotless)

grafik

@koppor
Copy link
Member

koppor commented Jan 2, 2023

For custom abbreviations working, JabRef needs a restart. Also an issue at v5.8.

@Siedlerchr Siedlerchr merged commit 88c9f56 into main Jan 2, 2023
@Siedlerchr Siedlerchr deleted the fixJournalAbbrevChecker branch January 2, 2023 20:34
Siedlerchr added a commit to NiD133/jabref that referenced this pull request Jan 4, 2023
* upstream/main: (29 commits)
  Use Set instead of List for custom journal abbreviations (JabRef#9515)
  Delete snap.yml (JabRef#9526)
  New Crowdin updates (JabRef#9523)
  Fix not on fx thread error when downloading file (JabRef#9522)
  Fix downloading in case of circular redirect (JabRef#9519)
  Typo: sucess -> success (JabRef#9520)
  New Crowdin updates (JabRef#9517)
  Add CHANGELOG.md entry for 9472
  Fix journal abbbrev checker for curly braces (JabRef#9504)
  Remove journal_abbreviations_ieee_strings.csv from journal lists (JabRef#9511)
  Bump mockito-core from 4.10.0 to 4.11.0 (JabRef#9513)
  Bump lucene-core from 9.4.1 to 9.4.2 (JabRef#9512)
  Bump checkstyle from 10.5.0 to 10.6.0 (JabRef#9514)
  Create a better solution for define multi line fields (JabRef#9456)
  Check links once a month
  Really ignore CHANGELOG.md in link check
  Fix spaces
  Add missing dots
  Squashed 'buildres/csl/csl-styles/' changes from 84dba23..43566f2
  try to repack deb file (JabRef#9506)
  ...

# Conflicts:
#	CHANGELOG.md
#	src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java
Siedlerchr added a commit to JabRef/jabref-koppor that referenced this pull request Jan 5, 2023
* upstream/main: (244 commits)
  Use Set instead of List for custom journal abbreviations (JabRef#9515)
  Delete snap.yml (JabRef#9526)
  New Crowdin updates (JabRef#9523)
  Fix not on fx thread error when downloading file (JabRef#9522)
  Fix downloading in case of circular redirect (JabRef#9519)
  Typo: sucess -> success (JabRef#9520)
  New Crowdin updates (JabRef#9517)
  Add CHANGELOG.md entry for 9472
  Fix journal abbbrev checker for curly braces (JabRef#9504)
  Remove journal_abbreviations_ieee_strings.csv from journal lists (JabRef#9511)
  Bump mockito-core from 4.10.0 to 4.11.0 (JabRef#9513)
  Bump lucene-core from 9.4.1 to 9.4.2 (JabRef#9512)
  Bump checkstyle from 10.5.0 to 10.6.0 (JabRef#9514)
  Create a better solution for define multi line fields (JabRef#9456)
  Check links once a month
  Really ignore CHANGELOG.md in link check
  Fix spaces
  Add missing dots
  Squashed 'buildres/csl/csl-styles/' changes from 84dba23..43566f2
  try to repack deb file (JabRef#9506)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
3 participants