jabref
jabref copied to clipboard
Add referenced entry's observers to update the entry in the main-table
I change the getField
method so that every time it will bind entry and referenced entry(if exists). Although the behavior now seems correct, I don't think it is an appropriate way to solve the problem. Hope for some suggestions.
Mitigates #7730
- [x] Change in
CHANGELOG.md
described in a way that is understandable for the average user (if applicable) - [ ] Tests created for changes (if applicable)
- [x] Manually tested changed features in running JabRef (always required)
- [ ] Screenshots added in PR description (for UI changes)
- [ ] Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.
Thank you for all your comments. In fact, this was my first attempt to solve the issue and it just worked (ugly indeed). The reason I delete the code about the init cache is that if the value is not null, it just return without considering update. So maybe we can consider update the binding in the corresponding places(crossref
field is changed, etc.). Currently I still don't figure out a proper way to solve this. So appreciate for your further comments and suggestions! 😊
@SuXiChangZhen our thinking is that with the caching re-enabled, this is still an improvement on the previous behavior. Perhaps the caching can be re-enabled, you use bibDatabaseContext.getDatabase().getReferencedEntry(entry)
and we do a re-review -> merge?
Regarding a "proper way to solve this", unless you have an interest in EasyBind/JavaFX in particular, it is probably not going to be worth your time, it might be better to pick a different issue.
If you want to give it an attempt anyway, we'd still propose fixing the other parts we commented on in this PR, merge, and then create a follow-up PR. I can expand on the suggested approach in https://github.com/JabRef/jabref/pull/7754#discussion_r664093093 but I make no guarantees that it will work nor that it is the "proper way" 😛
@k3KAW8Pnf7mkmdSMPHz27 Would you mind taking ove the PR so we can finish it? Just add the caching
Ok. I am very sorry for force-pushing to your repository @SuXiChangZhen, definitively last time I am trying that X) These are the changes we consider sufficient for this PR since it improves on the original JabRef behavior even if it doesn't completely solve the issue.
@Siedlerchr thank you for the git-support X)
The changes in this PR are the following
- When an entry was cross-referencing a different entry, changes in (say
author
) didn't propagate unless JabRef were restarted (or possibly switched back/forth between tabs). Now you'll see the entry being visually updated in real-time when you make changes in the cross-referenced entry.
What remains after this PR,
- If the entry being cross-referenced is changed, you need to restart/switch tabs again to make the updating work properly again
The crossref field is already in the array of the triggers for the CreateStringBinding-method?
Yes, but due to the caching, the dependencies on the cross-refs entry’s fields won’t update when cross-ref does (we need to “depend” on all observables in the cross-referenced entry to know when to update, and that is funky to do)
This was why the caching was removed in the original solution.
Ah, ok. Rebuilding the cache from an invalidation listener?
That would probably work. In “theory”, you shouldn't need to rebuild it.
Anyway, that part is why this issue is a bit of a mess. As is, this PR is imo an improvement on the previous behavior without that part as well.
We shoudd make a decision here: Merge it, as it is an improvement to the current situation, although it does not solve the issue at all, or try to fix it completly. Putting this on line for next dev call.