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

ActionHelper to test for present file #6151

Merged
merged 10 commits into from
Apr 17, 2020
Merged

ActionHelper to test for present file #6151

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c6d0812
Added ActionHelper to test for present file
calixtus Mar 18, 2020
b0ffa0d
Merge remote-tracking branch 'upstream/master' into fix_koppor_430
calixtus Mar 24, 2020
d16ded3
Added Validation
calixtus Mar 24, 2020
bc21be7
Refactored Validation
calixtus Mar 28, 2020
0c2c4c7
Refactored unsuccessfully ActionHelper
calixtus Mar 28, 2020
4c4a6dd
Merge remote-tracking branch 'upstream/master' into fix_koppor_430
calixtus Apr 6, 2020
feb8998
Fixed issues about opening an online link and applied some suggestion…
calixtus Apr 6, 2020
0ad24f6
Fixed recalculation of selected entry
calixtus Apr 13, 2020
cee5bee
Merge remote-tracking branch 'upstream/master' into fix_koppor_430
calixtus Apr 13, 2020
45ae2e6
Merge remote-tracking branch 'upstream/master' into fix_koppor_430
calixtus Apr 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/JabRefFrame.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -833,7 +833,7 @@ private MenuBar createMenu() {

new SeparatorMenuItem(),

factory.createMenuItem(StandardActions.SHOW_PDF_VIEWER, new ShowDocumentViewerAction()),
factory.createMenuItem(StandardActions.SHOW_PDF_VIEWER, new ShowDocumentViewerAction(stateManager, prefs)),
factory.createMenuItem(StandardActions.EDIT_ENTRY, new OpenEntryEditorAction(this, stateManager)),
factory.createMenuItem(StandardActions.OPEN_CONSOLE, new OpenConsoleAction(stateManager))
);
Expand Down
18 changes: 18 additions & 0 deletions src/main/java/org/jabref/gui/actions/ActionHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@

import org.jabref.gui.StateManager;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.LinkedFile;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.util.FileHelper;
import org.jabref.preferences.PreferencesService;

public class ActionHelper {
public static BooleanExpression needsDatabase(StateManager stateManager) {
Expand Down Expand Up @@ -36,4 +40,18 @@ public static BooleanExpression isAnyFieldSetForSelectedEntry(List<Field> fields
entry.getFieldsObservable(),
stateManager.getSelectedEntries());
}

public static BooleanExpression isFilePresentForSelectedEntry(StateManager stateManager, PreferencesService preferencesService) {
List<LinkedFile> files = stateManager.getSelectedEntries().get(0).getFiles();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it wasn't in EasyBind but here:

jabref/src/main/java/org/jabref/gui/util/BindingsHelper.java

Line 68 in 093a6fb

public static <T, U> ObservableList<U> map(ObservableValue<T> source, Function<T, List<U>> mapper) {

thus BindingsHelper.map(getFieldBinding(FILES), string -> parseFiles()) should work. If not I'll have a closer look at it tomorrow.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return Bindings.createBooleanBinding(() -> {
if ((files.size() > 0) && stateManager.getActiveDatabase().isPresent()) {
return FileHelper.expandFilename(
stateManager.getActiveDatabase().get(),
files.get(0).getLink(),
preferencesService.getFilePreferences()).isPresent();
} else {
return false;
}
}, stateManager.getSelectedEntries().get(0).getFieldBinding(StandardField.FILE));
}
}
8 changes: 7 additions & 1 deletion src/main/java/org/jabref/gui/documentviewer/ShowDocumentViewerAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
package org.jabref.gui.documentviewer;

import org.jabref.gui.StateManager;
import org.jabref.gui.actions.ActionHelper;
import org.jabref.gui.actions.SimpleCommand;
import org.jabref.preferences.PreferencesService;

public class ShowDocumentViewerAction extends SimpleCommand {

public ShowDocumentViewerAction(StateManager stateManager, PreferencesService preferences) {
this.executable.bind(ActionHelper.isFilePresentForSelectedEntry(stateManager, preferences));
}

@Override
public void execute() {
new DocumentViewerView().show();
}

}
52 changes: 35 additions & 17 deletions src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,13 @@
import org.jabref.model.entry.LinkedFile;
import org.jabref.model.metadata.FilePreferences;
import org.jabref.model.strings.StringUtil;
import org.jabref.model.util.FileHelper;
import org.jabref.model.util.OptionalUtil;

import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
import de.saxsys.mvvmfx.utils.validation.Validator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -68,6 +73,8 @@ public class LinkedFileViewModel extends AbstractViewModel {
private final LinkedFileHandler linkedFileHandler;
private final ExternalFileTypes externalFileTypes;

private final Validator fileExistsValidator;

public LinkedFileViewModel(LinkedFile linkedFile,
BibEntry entry,
BibDatabaseContext databaseContext,
Expand All @@ -87,6 +94,14 @@ public LinkedFileViewModel(LinkedFile linkedFile,
this.externalFileTypes = externalFileTypes;
this.xmpPreferences = xmpPreferences;

fileExistsValidator = new FunctionBasedValidator<>(
linkedFile.linkProperty(),
link -> {
Optional<Path> path = FileHelper.expandFilename(databaseContext, link, filePreferences);
return path.isPresent() && Files.exists(path.get());
},
ValidationMessage.warning(Localization.lang("Could not find file '%0'.", linkedFile.getLink())));

downloadOngoing.bind(downloadProgress.greaterThanOrEqualTo(0).and(downloadProgress.lessThan(1)));
canWriteXMPMetadata.setValue(!linkedFile.isOnlineLink() && linkedFile.getFileType().equalsIgnoreCase("pdf"));
}
Expand Down Expand Up @@ -161,8 +176,18 @@ public Observable[] getObservables() {

public void open() {
try {
Optional<Path> resolvedPath = FileHelper.expandFilename(
databaseContext,
linkedFile.getLink(),
filePreferences);

Optional<ExternalFileType> type = ExternalFileTypes.getInstance().fromLinkedFile(linkedFile, true);
boolean successful = JabRefDesktop.openExternalFileAnyFormat(databaseContext, linkedFile.getLink(), type);

boolean successful = false;
if (resolvedPath.isPresent()) {
successful = JabRefDesktop.openExternalFileAnyFormat(databaseContext, resolvedPath.get().toString(), type);
}

if (!successful) {
dialogService.showErrorDialogAndWait(Localization.lang("File not found"), Localization.lang("Could not find file '%0'.", linkedFile.getLink()));
}
Expand All @@ -173,22 +198,13 @@ public void open() {

public void openFolder() {
try {
Path path = null;
// absolute path
if (Paths.get(linkedFile.getLink()).isAbsolute()) {
path = Paths.get(linkedFile.getLink());
} else {
// relative to file folder
for (Path folder : databaseContext.getFileDirectoriesAsPaths(filePreferences)) {
Path file = folder.resolve(linkedFile.getLink());
if (Files.exists(file)) {
path = file;
break;
}
}
}
if (path != null) {
JabRefDesktop.openFolderAndSelectFile(path);
Optional<Path> resolvedPath = FileHelper.expandFilename(
databaseContext,
linkedFile.getLink(),
filePreferences);

if (resolvedPath.isPresent()) {
JabRefDesktop.openFolderAndSelectFile(resolvedPath.get());
} else {
dialogService.showErrorDialogAndWait(Localization.lang("File not found"));
}
Expand Down Expand Up @@ -455,4 +471,6 @@ private Optional<ExternalFileType> inferFileTypeFromURL(String url) {
public LinkedFile getFile() {
return linkedFile;
}

public ValidationStatus fileExistsValidationStatus() { return fileExistsValidator.getValidationStatus(); }
}
3 changes: 2 additions & 1 deletion src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ public LinkedFilesEditor(Field field, DialogService dialogService, BibDatabaseCo
.withOnMouseClickedEvent(this::handleItemMouseClick)
.setOnDragDetected(this::handleOnDragDetected)
.setOnDragDropped(this::handleOnDragDropped)
.setOnDragOver(this::handleOnDragOver);
.setOnDragOver(this::handleOnDragOver)
.withValidation(LinkedFileViewModel::fileExistsValidationStatus);

listView.setCellFactory(cellFactory);

Expand Down
3 changes: 1 addition & 2 deletions src/main/java/org/jabref/gui/maintable/OpenExternalFileAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import org.jabref.gui.fieldeditors.LinkedFileViewModel;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.StandardField;
import org.jabref.preferences.PreferencesService;

public class OpenExternalFileAction extends SimpleCommand {
Expand All @@ -25,7 +24,7 @@ public OpenExternalFileAction(DialogService dialogService, StateManager stateMan
this.stateManager = stateManager;
this.preferencesService = preferencesService;

this.executable.bind(ActionHelper.isFieldSetForSelectedEntry(StandardField.FILE, stateManager)
this.executable.bind(ActionHelper.isFilePresentForSelectedEntry(stateManager, preferencesService)
.and(ActionHelper.needsEntriesSelected(1, stateManager)));
}

Expand Down
3 changes: 1 addition & 2 deletions src/main/java/org/jabref/gui/maintable/OpenFolderAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import org.jabref.gui.actions.SimpleCommand;
import org.jabref.gui.externalfiletype.ExternalFileTypes;
import org.jabref.gui.fieldeditors.LinkedFileViewModel;
import org.jabref.model.entry.field.StandardField;
import org.jabref.preferences.PreferencesService;

public class OpenFolderAction extends SimpleCommand {
Expand All @@ -21,7 +20,7 @@ public OpenFolderAction(DialogService dialogService, StateManager stateManager,
this.stateManager = stateManager;
this.preferencesService = preferencesService;

this.executable.bind(ActionHelper.isFieldSetForSelectedEntry(StandardField.FILE, stateManager));
this.executable.bind(ActionHelper.isFilePresentForSelectedEntry(stateManager, preferencesService));
}

@Override
Expand Down
19 changes: 17 additions & 2 deletions src/main/java/org/jabref/gui/util/ViewModelListCellFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.jabref.gui.icon.JabRefIcon;
import org.jabref.model.strings.StringUtil;

import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
import org.fxmisc.easybind.Subscription;

/**
Expand All @@ -43,6 +44,7 @@ public class ViewModelListCellFactory<T> implements Callback<ListView<T>, ListCe
private BiConsumer<T, ? super DragEvent> toOnDragExited;
private BiConsumer<T, ? super DragEvent> toOnDragOver;
private Map<PseudoClass, Callback<T, ObservableValue<Boolean>>> pseudoClasses = new HashMap<>();
private Callback<T, ValidationStatus> validationStatusProperty;

public ViewModelListCellFactory<T> withText(Callback<T, String> toText) {
this.toText = toText;
Expand Down Expand Up @@ -134,6 +136,11 @@ public ViewModelListCellFactory<T> withPseudoClass(PseudoClass pseudoClass, Call
return this;
}

public ViewModelListCellFactory<T> withValidation(Callback<T, ValidationStatus> validationStatusProperty) {
this.validationStatusProperty = validationStatusProperty;
return this;
}

public void install(ComboBox<T> comboBox) {
comboBox.setButtonCell(this.call(null));
comboBox.setCellFactory(this);
Expand All @@ -146,7 +153,7 @@ public void install(ListView<T> listView) {
@Override
public ListCell<T> call(ListView<T> param) {

return new ListCell<T>() {
return new ListCell<>() {

List<Subscription> subscriptions = new ArrayList<>();

Expand Down Expand Up @@ -178,7 +185,12 @@ protected void updateItem(T item, boolean empty) {
getStyleClass().setAll(toStyleClass.call(viewModel));
}
if (toTooltip != null) {
setTooltip(toTooltip.call(viewModel));
Tooltip tooltip = toTooltip.call(viewModel);
if (validationStatusProperty != null) {
validationStatusProperty.call(viewModel).getHighestMessage().ifPresent(
message -> tooltip.setText(message.getMessage()));
}
setTooltip(tooltip);
}
if (toContextMenu != null) {
setContextMenu(toContextMenu.call(viewModel));
Expand All @@ -203,6 +215,9 @@ protected void updateItem(T item, boolean empty) {
Subscription subscription = BindingsHelper.includePseudoClassWhen(this, pseudoClassWithCondition.getKey(), condition);
subscriptions.add(subscription);
}
if ((validationStatusProperty != null) && validationStatusProperty.call(viewModel).getHighestMessage().isPresent()) {
setStyle("-fx-background-color: -jr-warn");
}
}
}
};
Expand Down