Update entry for every change in the source panel

[tobiasdiez]

In #3352 (comment), @lenhard suggested to update the entry as soon as the source code is changed (instead of only on focus lost). This is accomplished in this PR.
Jörg mentioned that the most direct solution does not work since

the store operation will move the cursor to the end of the source tab.

This problem is circumvented by implementing a proper bidirectional binding between the entry and the source code, which prevents update cycles.

Moreover, in case of a parse error, the message is no longer shown in a dialog but inline as a notification:

• Change in CHANGELOG.md described
• Tests created for changes
• Screenshots added (for bigger UI changes)
• Manually tested changed features in running JabRef
• Check documentation status (Issue created for outdated help page at help.jabref.org?)
• If you changed the localization: Did you run gradle localizationUpdate?

[halirutan]

I have several issues with this on my machine.

1. Open Entry go to source tab and invoke Generate Key button or Quality -> Autogenerate BibTeX keys throws an exception and leaves everything in an undefined state where the source editor doesn't work anymore (likewise each external change like Clean Entry, Mark Entry, will do this)
2. Using Change Entry Type button doesn't update the source view
3. Changing source to something invalid and moving to a different tab or entry silently reverts the entry to its original content (which would be OK for me and which makes life so much easier)
4. Undo doesn't work correctly. It kind of mixes reverting things I did earlier and throws an exception. In the end, it clears the source editor

Some of the things might be older issues but leaving the source editor unusable should be introduced by this PR.

[tobiasdiez]

@halirutan Thank you for the intensive testing.

• 1+4: Oh yes, again one of these "not on JavaFX thread"-related issues. Should be fixed now
• 2: Changing the entry type indeed does not update the source code. However, this also didn't worked before if I'm not mistaken.
• 3: I don't want to store invalid input. This would really complicate things.

[lenhard]

As far as I've tested, this resolves #3352 #3363 #3375 #3369 and maybe also a few other of the entry editor issues that I haven't tested.

1. Seems to work again as far as I can see
2. Seems to be broken since moving to JavaFX
3. Agreed
4. Undo in the source tab is essentially broken since changing the entry editor to JavaFX. The CodeArea listens to undo events on its own and that is not integrated into the undo framework for the remaining field editors. That is the reason for the strange behavior (which is still present in this PR).

When adding a new entry of type article and moving to the source tab, I get the exception below. After that, the entire entry editor seems to be broken, regardless of the tab.

12:04:25.997 [JavaFX Application Thread] ERROR org.jabref.FallbackExceptionHandler - Uncaught exception occurred in Thread[JavaFX Application Thread,5,main]
java.lang.IndexOutOfBoundsException: [-2147483648, 8) is not a valid range within [0, 8)
        at org.reactfx.util.Lists.checkRange(Lists.java:128) ~[reactfx-2.0-M5.jar:?]
        at org.reactfx.util.Lists.checkRange(Lists.java:123) ~[reactfx-2.0-M5.jar:?]
        at org.reactfx.collection.MemoizationListImpl.forget(MemoizationList.java:184) ~[reactfx-2.0-M5.jar:?]
        at org.fxmisc.flowless.CellListManager.cropTo(CellListManager.java:71) ~[flowless-0.5.2.jar:?]
        at org.fxmisc.flowless.CellPositioner.cropTo(CellPositioner.java:24) ~[flowless-0.5.2.jar:?]
        at org.fxmisc.flowless.Navigator.cropToNeighborhoodOf(Navigator.java:209) ~[flowless-0.5.2.jar:?]
        at org.fxmisc.flowless.Navigator.placeStartAtMayCrop(Navigator.java:182) ~[flowless-0.5.2.jar:?]
        at org.fxmisc.flowless.Navigator.visit(Navigator.java:111) ~[flowless-0.5.2.jar:?]
        at org.fxmisc.flowless.StartOffStart.accept(TargetPosition.java:49) ~[flowless-0.5.2.jar:?]
        at org.fxmisc.flowless.Navigator.layoutChildren(Navigator.java:67) ~[flowless-0.5.2.jar:?]
        at javafx.scene.Parent.layout(Parent.java:1087) ~[jfxrt.jar:?]
        at org.fxmisc.flowless.VirtualFlow.layoutChildren(VirtualFlow.java:165) ~[flowless-0.5.2.jar:?]
        at javafx.scene.Parent.layout(Parent.java:1087) ~[jfxrt.jar:?]
        at javafx.scene.Parent.layout(Parent.java:1093) ~[jfxrt.jar:?]
        at javafx.scene.Parent.layout(Parent.java:1093) ~[jfxrt.jar:?]
        at javafx.scene.Parent.layout(Parent.java:1093) ~[jfxrt.jar:?]
        at javafx.scene.Parent.layout(Parent.java:1093) ~[jfxrt.jar:?]
        at javafx.scene.Parent.layout(Parent.java:1093) ~[jfxrt.jar:?]
        at javafx.scene.Scene.doLayoutPass(Scene.java:552) ~[jfxrt.jar:?]
        at javafx.scene.Scene.preferredSize(Scene.java:1646) ~[jfxrt.jar:?]
        at javafx.scene.Scene.impl_preferredSize(Scene.java:1720) ~[jfxrt.jar:?]

        at javafx.stage.Window$9.invalidated(Window.java:846) ~[jfxrt.jar:?]
        at javafx.beans.property.BooleanPropertyBase.markInvalid(BooleanPropertyBase.java:109) ~[jfxrt.jar:?]
        at javafx.beans.property.BooleanPropertyBase.set(BooleanPropertyBase.java:144) ~[jfxrt.jar:?]
        at javafx.stage.Window.setShowing(Window.java:922) ~[jfxrt.jar:?]
        at javafx.stage.Window.show(Window.java:937) ~[jfxrt.jar:?]
        at com.sun.javafx.stage.EmbeddedWindow.show(EmbeddedWindow.java:58) ~[jfxrt.jar:?]
        at javafx.embed.swing.JFXPanel.lambda$addNotify$49(JFXPanel.java:799) ~[jfxrt.jar:?]
        at com.sun.javafx.application.PlatformImpl.lambda$null$173(PlatformImpl.java:295) ~[jfxrt.jar:?]
        at java.security.AccessController.doPrivileged(Native Method) ~[?:1.8.0_121]
        at com.sun.javafx.application.PlatformImpl.lambda$runLater$174(PlatformImpl.java:294) ~[jfxrt.jar:?]
        at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95) ~[jfxrt.jar:?]
        at com.sun.glass.ui.win.WinApplication._runLoop(Native Method) ~[jfxrt.jar:?]
        at com.sun.glass.ui.win.WinApplication.lambda$null$148(WinApplication.java:191) ~[jfxrt.jar:?]
        at java.lang.Thread.run(Thread.java:745) [?:1.8.0_121]

Code-wise the PR looks fine. I have to say that I do not like the bindings, since in my point of view they make the code harder to read and much harder to debug. They also provide for more seamless updates, though, so maybe I just still need time to get more comfortable with them.

Because it fixes so many things, I am for eventually merging it when all exceptions are fixed and the localizations are synchronized. @tobiasdiez @halirutan You should probably synchronize on how does what when it comes to repairing the source tab. Just to avoid effort duplication.

[tobiasdiez]

@lenhard Also thanks to you for the intensive testing! I've not realized how many bugs this PR actually fixes. The exception you get is also present in the current master #3375. I actually cannot reproduce it, so its not possible for me to solve or analyze it (however: it seems to be a problem with the code editor itself, flowless is the library that provides the editor).
Thus I'm taking the liberty to merge the PR after the build is successful.

Sidenote: you are right, debugging these bindings is a nightmare...on the other hand, they are really powerful and make things easier (as the 2-way binding in this PR shows; setting up something similar with our event system would be way harder).

@halirutan I'm really sorry that my changes overlapped with your's. I read it to late that you already had a solution for some of these issues. Feel free to work on the editor, I'll stay put and work at other things now. (A more detailed communication on these matters is hard at the moment since my skype is not working due to the great firewall and vpn issues...)

[sambo57u]

I just tried testing the latest build which I believe includes this change. Changes now a properly saved
in the entry editor except when one tries to put a curly bracket, e.g. for capital letters, I am getting an
what seems to be an inconsequential error message saying that "unable to parse the entry ....".
I think as soon as you type in a { if this is going to be stored then there is a mismatch in brackets.

[jonasstein]

I tried this release too, here (#3383) it even "destroys" the source field until I restart Jabref. In the Example I documented there is also a { perhaps this is the root of evil.

[tobiasdiez]

@sambo57u Thank you very much for trying the new build. If you add an additional { jabref can no longer parse the entry successful and thus does not stores the changes. But as soon as you close the bracket } JabRef should start to update the entry again. I just pushed a fix to the newest development version that makes the error message at the bottom disappear as soon as the code can be parsed again.

[sambo57u]

Trying 4.1dev build 2017-11-01 04:42
There seems to be other problems creeping into the entry editor:

1. Changes are not saved at all
2. When the source editor is invoked by double clicking on an entry the clicking the mouse cursor somewhere in the source has no effect of moving the cursor (remains at the top left position). But actually it seems to have moved because if you type a letter it types it where you clicked the cursor and
   at that point cursor also moves.

[lenhard]

@tobiasdiez It seems that several other people are experiencing problems with the source tab that are affecting the complete entry editor, e.g. also #3217 (comment) I guess the binding as it stands here does not work properly.

I think the merge of this might have been a little too early. Could you please investigate this issue (sort of soon, sorry), because the master branch is kind of broken at the moment. I am not sure, but maybe an alternative would be to revert the merge, reopen the closed issues linked above, and go for a new PR. After all, it might be better to have no sync between source tab and other tabs than to have no entry editor at all. But maybe the whole problem is easily fixed soon in a follow-up PR and then reverting would not be necessary.

[tobiasdiez]

@lenhard Yes, sorry for merging a bit to early. I thought that since the index out of bounds exception was already a known issue, I did not introduce a new bug and thus everything is fine. But, as we learned now, the fixes in this PR actually make it so that users hit the IndexOutOfBounds bug way more often (since now the external updates actually work...).

Hopefully #3390 provides a temporary fix until FXMisc/RichTextFX#637 is solved.

[lenhard]

@tobiasdiez Thanks for the quick action :) I'll try to review the new PR as soon as possible and then we will hopefully be back in a working state soon.

[sambo57u]

I am only seeing the 11-01 12;11 build on master. Testing that two problems remain, the mouse movement and nothing is saved. Even after making a change one does ctrl+s and sees a message that the bib file is saved, it is really not saved.

[sambo57u]

As of 4.1dev 2017-11-03 07:17 version no change in the source editor is saved, plus the cursor issue.

[lenhard]

@sambo57u As of now, the fix is not yet merged into master, so this is expected. If you want to try out the current version of the fix that seems to repair the saving, you can get it here: http://builds.jabref.org/storeSourceFix/

[sambo57u]

Hi, unfortunately the version from http://builds.jabref.org/storeSourceFix/ have the same problem. This was fixed at some point except the issue of getting an error message when one tried to enter a curly bracket. At this point no changes are saved in my test.

[lenhard]

Ok, I also have some update problems in that left. Looks like they won't get fixed as part of that PR. Well have to see if someone addresses them.