ActionHelper to test for present file#6151
Conversation
|
I added some validation. The regular validation mechanics of controlsfx seem somewhat broken. The icon is shown, but behind every other ui element, so instead I just marked the background of the cells with files that do not exist in the filesystem in the warning color.
Theoretically, this one is mergable. |
tobiasdiez
left a comment
There was a problem hiding this comment.
Thanks! Looks good to me in general, but I have a few comments/suggestions.
| } | ||
|
|
||
| public static BooleanExpression isFilePresentForSelectedEntry(StateManager stateManager, PreferencesService preferencesService) { | ||
| List<LinkedFile> files = stateManager.getSelectedEntries().get(0).getFiles(); |
There was a problem hiding this comment.
I think the current code runs into similar problems that we had recently in a PR by @stefan-kolb: if the currently selected entry changes, then the binding is not updated.
Here, this should be relatively easy to fix:
EasyBind.flatMap(stateManager.getSelectedEntries().getValueAt(0), entry -> entry.getFiles())
.map(files -> to boolean)
Here the getFiles methods needs to be changed to return an observable list. This can be done using the current code and a EasyBind.map(getFieldBinding(FILE), ... parse).
There was a problem hiding this comment.
I played a bit around with it. I was able to make getFiles return an ObservableList (by BindingsHelper::map, I had no luck with EasyBind), but EasyBind.flatMap does not exist, so i worked with map. But now the ActionHelper never works. I really don't know how to proceed, what am I doing wrong?
There was a problem hiding this comment.
Sorry, it wasn't in EasyBind but here:
thus BindingsHelper.map(getFieldBinding(FILES), string -> parseFiles()) should work. If not I'll have a closer look at it tomorrow.
There was a problem hiding this comment.
We always think way too complicated.
I just moved the call to getFiles() into the calculation part and added stateManager.getSelectedEntries as an additional dependency, so the list is always recalculated if the currently selected entry changes.
I have tested it and it works.
| } | ||
|
|
||
| public static BooleanExpression isFilePresentForSelectedEntry(StateManager stateManager, PreferencesService preferencesService) { | ||
| List<LinkedFile> files = stateManager.getSelectedEntries().get(0).getFiles(); |
There was a problem hiding this comment.
I played a bit around with it. I was able to make getFiles return an ObservableList (by BindingsHelper::map, I had no luck with EasyBind), but EasyBind.flatMap does not exist, so i worked with map. But now the ActionHelper never works. I really don't know how to proceed, what am I doing wrong?
# Conflicts: # src/main/java/org/jabref/model/entry/BibEntry.java
…ionCaseInsensitive * upstream/master: (25 commits) ActionHelper to test for present file (#6151) Reduce memory footprint (#6298) Add missing abbreviated journal names (#6292) fix l10n again fix checkstyle fix l10n Try to minimize CodeCov "wrong metrics" Showing correct icon on main table linked files column (#6264) Fix labels for outdated dependency issue Change one more line Squashed 'src/main/resources/csl-styles/' changes from c31d9ca..c1793d2 Resolve unit test from failing Add one more change Fix errors RIS import takes the wrong date and duplicates abstract (#6272) Update EntryTypeView.java Change to the old school format Fix XmpExporterTest (#6289) Add checkstyle screenshot (and lint guidelines-...md) Squashed 'src/main/resources/csl-styles/' changes from db54e56..c31d9ca ...
This PR fixes koppor#430
It took a little bit more refactoring than expected, for it does not only check, if StandardField.FILE has text inside, but if the file is also really present in the file system. There are some design decisions i'm unsure of:
There is no alert about that no file is really present. The menu entry is just disabled, although there is a file symbol in the main table (since StandardField.FILE is not empty).
Feedback could be implemented in the files editor in the entry editor as validation.
The other thing is that in theory at last, the table does not yet automatically update, if a file is added or removed (so no file is attached). This would require probably an array of observables to be used as an invalidation alert. I'm unsure where to start with that. However, only in theory. Pracitcally the entry is unselected automatically if I change something about the attached files in the entry editor. So nobody would notice, if this would not be implemented.