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

Add "Do not ask again" for empty entry confirmation #8297

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
13 changes: 13 additions & 0 deletions src/main/java/org/jabref/gui/DialogService.java
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,19 @@ boolean showConfirmationDialogWithOptOutAndWait(String title, String content,
Optional<ButtonType> showCustomButtonDialogAndWait(Alert.AlertType type, String title, String content,
ButtonType... buttonTypes);

/**
* This will create and display a new dialog of the specified
* {@link Alert.AlertType} but with user defined buttons as optional
* {@link ButtonType}s.
* Moreover, the dialog contains a opt-out checkbox with the given text to support "Do not ask again"-behaviour.
*
* @return Optional with the pressed Button as ButtonType
*/
Optional<ButtonType> showCustomButtonDialogWithOptOutAndWait(Alert.AlertType type, String title, String content,
String optOutMessage, Consumer<Boolean> optOutAction,
ButtonType... buttonTypes);


/**
* This will create and display a new dialog showing a custom {@link DialogPane}
* and using custom {@link ButtonType}s.
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/org/jabref/gui/JabRefDialogService.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import javafx.print.PrinterJob;
import javafx.scene.Group;
import javafx.scene.Node;
import javafx.scene.control.Alert;
import javafx.scene.control.Alert.AlertType;
import javafx.scene.control.Button;
import javafx.scene.control.ButtonBar;
Expand Down Expand Up @@ -259,6 +260,15 @@ public Optional<ButtonType> showCustomButtonDialogAndWait(AlertType type, String
return alert.showAndWait();
}

@Override
public Optional<ButtonType> showCustomButtonDialogWithOptOutAndWait(Alert.AlertType type, String title, String content,
String optOutMessage, Consumer<Boolean> optOutAction,
ButtonType... buttonTypes) {
FXDialog alert = createDialogWithOptOut(AlertType.CONFIRMATION, title, content, optOutMessage, optOutAction);
alert.getButtonTypes().setAll(buttonTypes);
return alert.showAndWait();
}

Comment on lines +263 to +271
Copy link
Member

Choose a reason for hiding this comment

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

There still seems to be some code duplication with showConfirmationDialogWithOptOutAndWait, which is basically now a subset of this method here. Should call this instead.
Also the method argument type in the method signature, but is not used in the method.

@Override
public Optional<ButtonType> showCustomDialogAndWait(String title, DialogPane contentPane,
ButtonType... buttonTypes) {
Expand Down
46 changes: 37 additions & 9 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,16 @@ public boolean quit() {
final BibDatabaseContext context = libraryTab.getBibDatabaseContext();

if (context.hasEmptyEntries()) {
if (!confirmEmptyEntry(libraryTab, context)) {
return false;
if (prefs.getGeneralPreferences().shouldConfirmEmptyEntries()) {
Optional<ButtonType> response = confirmEmptyEntry(libraryTab);
if (!deleteEmptyEntry(libraryTab, context, response)) {
return false;
}
} else if (prefs.getGeneralPreferences().shouldDeleteEmptyEntries()) {
Optional<ButtonType> response = Optional.of(new ButtonType(Localization.lang("Delete empty entries"), ButtonBar.ButtonData.YES));
if (!deleteEmptyEntry(libraryTab, context, response)) {
return false;
}
}
}

Expand Down Expand Up @@ -1169,8 +1177,10 @@ private boolean confirmClose(LibraryTab libraryTab) {

/**
* Ask if the user really wants to remove any empty entries
*
* @return user response from the confirmation dialog
*/
private Boolean confirmEmptyEntry(LibraryTab libraryTab, BibDatabaseContext context) {
private Optional<ButtonType> confirmEmptyEntry(LibraryTab libraryTab) {
String filename = libraryTab.getBibDatabaseContext()
.getDatabasePath()
.map(Path::toAbsolutePath)
Expand All @@ -1181,11 +1191,22 @@ private Boolean confirmEmptyEntry(LibraryTab libraryTab, BibDatabaseContext cont
ButtonType keepEmptyEntries = new ButtonType(Localization.lang("Keep empty entries"), ButtonBar.ButtonData.NO);
ButtonType cancel = new ButtonType(Localization.lang("Return to JabRef"), ButtonBar.ButtonData.CANCEL_CLOSE);

Optional<ButtonType> response = dialogService.showCustomButtonDialogAndWait(Alert.AlertType.CONFIRMATION,
Optional<ButtonType> response = dialogService.showCustomButtonDialogWithOptOutAndWait(Alert.AlertType.CONFIRMATION,
Copy link
Member

Choose a reason for hiding this comment

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

Why don't you use the showConfirmationDialogWithOptOutAndWait? The AlertType.CONFIRMATION is used there too.
This looks to me like unnecessary code duplication, or didn't I see anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I added showCustomButtonDialogWithOptOutAndWait is because I would like to include the 'Return to JabRef' option as it has now. I believe showConfirmationDialogWithOptOutAndWait only allows for Yes/No choice, but not the third option.

Localization.lang("Empty entries"),
Localization.lang("Library '%0' has empty entries. Do you want to delete them?", filename),
Localization.lang("Do not ask again"),
optOut -> prefs.getGeneralPreferences().setConfirmEmptyEntries(!optOut),
deleteEmptyEntries, keepEmptyEntries, cancel);
if (response.isPresent() && response.get().equals(deleteEmptyEntries)) {
if (response.isPresent() && response.get().getButtonData().equals(ButtonBar.ButtonData.NO)) {
prefs.getGeneralPreferences().setDeleteEmptyEntries(false);
} else if (response.isPresent() && response.get().getButtonData().equals(ButtonBar.ButtonData.YES)) {
prefs.getGeneralPreferences().setDeleteEmptyEntries(true);
}
return response;
}

private Boolean deleteEmptyEntry(LibraryTab libraryTab, BibDatabaseContext context, Optional<ButtonType> response) {
if (response.isPresent() && response.get().getButtonData() == ButtonBar.ButtonData.YES) {
// The user wants to delete.
try {
for (BibEntry currentEntry : new ArrayList<>(context.getEntries())) {
Expand All @@ -1206,19 +1227,26 @@ private Boolean confirmEmptyEntry(LibraryTab libraryTab, BibDatabaseContext cont
// Save was cancelled or an error occurred.
return false;
}
return !response.get().equals(cancel);
return !(response.get().getButtonData() == ButtonBar.ButtonData.CANCEL_CLOSE);
}

private void closeTab(LibraryTab libraryTab) {
// empty tab without database
if (libraryTab == null) {
return;
}

final BibDatabaseContext context = libraryTab.getBibDatabaseContext();
if (context.hasEmptyEntries()) {
if (!confirmEmptyEntry(libraryTab, context)) {
return;
if (prefs.getGeneralPreferences().shouldConfirmEmptyEntries()) {
Optional<ButtonType> response = confirmEmptyEntry(libraryTab);
if (!deleteEmptyEntry(libraryTab, context, response)) {
return;
}
} else if (prefs.getGeneralPreferences().shouldDeleteEmptyEntries()) {
Optional<ButtonType> response = Optional.of(new ButtonType(Localization.lang("Delete empty entries"), ButtonBar.ButtonData.YES));
if (!deleteEmptyEntry(libraryTab, context, response)) {
return;
}
}
}

Expand Down
60 changes: 60 additions & 0 deletions src/main/java/org/jabref/preferences/GeneralPreferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ public class GeneralPreferences {
private final ObjectProperty<BibDatabaseMode> defaultBibDatabaseMode;
private final BooleanProperty warnAboutDuplicatesInInspection;
private final BooleanProperty confirmDelete;
private final BooleanProperty confirmEmptyEntries;
private final BooleanProperty deleteEmptyEntries;

private final BooleanProperty memoryStickMode;
private final BooleanProperty showAdvancedHints;
Expand All @@ -22,12 +24,16 @@ public GeneralPreferences(Charset defaultEncoding,
BibDatabaseMode defaultBibDatabaseMode,
boolean warnAboutDuplicatesInInspection,
boolean confirmDelete,
boolean confirmEmptyEntries,
boolean deleteEmptyEntries,
boolean memoryStickMode,
boolean showAdvancedHints) {
this.defaultEncoding = new SimpleObjectProperty<>(defaultEncoding);
this.defaultBibDatabaseMode = new SimpleObjectProperty<>(defaultBibDatabaseMode);
this.warnAboutDuplicatesInInspection = new SimpleBooleanProperty(warnAboutDuplicatesInInspection);
this.confirmDelete = new SimpleBooleanProperty(confirmDelete);
this.confirmEmptyEntries = new SimpleBooleanProperty(confirmEmptyEntries);
this.deleteEmptyEntries = new SimpleBooleanProperty(deleteEmptyEntries);

this.memoryStickMode = new SimpleBooleanProperty(memoryStickMode);
this.showAdvancedHints = new SimpleBooleanProperty(showAdvancedHints);
Expand Down Expand Up @@ -81,6 +87,60 @@ public void setConfirmDelete(boolean confirmDelete) {
this.confirmDelete.set(confirmDelete);
}

/**
* Get the preference of whether to give a confirmation dialog when empty entries are detected in the library
*
* @return true if the preference is set as yes
*/
public boolean shouldConfirmEmptyEntries() {
return confirmEmptyEntries.get();
}

/**
* Get the BooleanProperty of the preference of whether to give a confirmation dialog when empty entries are detected in the library
*
* @return BooleanProperty
*/
public BooleanProperty confirmEmptyEntriesProperty() {
return confirmEmptyEntries;
}

/**
* Set the preference of whether to give a confirmation dialog when empty entries are detected in the library
*
* @param confirmEmptyEntries boolean for the preference
*/
public void setConfirmEmptyEntries(boolean confirmEmptyEntries) {
this.confirmEmptyEntries.set(confirmEmptyEntries);
}

/**
* Get the preference of whether to delete the empty entries detected in the library if confirmation dialog is opted out
*
* @return true if the preference is set as yes (delete)
*/
public boolean shouldDeleteEmptyEntries() {
return deleteEmptyEntries.get();
}

/**
* Get the BooleanProperty of the preference of whether to delete the empty entries detected in the library if confirmation dialog is opted out
*
* @return BooleanProperty
*/
public BooleanProperty deleteEmptyEntriesProperty() {
return deleteEmptyEntries;
}

/**
* Set the preference of whether to delete the empty entries detected in the library if confirmation dialog is opted out
*
* @param deleteEmptyEntries boolean for the preference
*/
public void setDeleteEmptyEntries(boolean deleteEmptyEntries) {
this.deleteEmptyEntries.set(deleteEmptyEntries);
}
Copy link
Member

Choose a reason for hiding this comment

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

This javadoc is superfluous. Please comment on the the variable or the property, but not on every getter and setter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry about this. I removed these comments. Let me know if you would like have any other changes.


public boolean isMemoryStickMode() {
return memoryStickMode.get();
}
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/org/jabref/preferences/JabRefPreferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,8 @@ public class JabRefPreferences implements PreferencesService {
public static final String DEFAULT_CITATION_KEY_PATTERN = "defaultBibtexKeyPattern";
public static final String UNWANTED_CITATION_KEY_CHARACTERS = "defaultUnwantedBibtexKeyCharacters";
public static final String CONFIRM_DELETE = "confirmDelete";
public static final String CONFIRM_EMPTY_ENTRIES = "confirmEmptyEntries";
public static final String DELETE_EMPTY_ENTRIES = "deleteEmptyEntries";
public static final String WARN_BEFORE_OVERWRITING_KEY = "warnBeforeOverwritingKey";
public static final String AVOID_OVERWRITING_KEY = "avoidOverwritingKey";
public static final String AUTOLINK_EXACT_KEY_ONLY = "autolinkExactKeyOnly";
Expand Down Expand Up @@ -626,6 +628,8 @@ private JabRefPreferences() {
defaults.put(AVOID_OVERWRITING_KEY, Boolean.FALSE);
defaults.put(WARN_BEFORE_OVERWRITING_KEY, Boolean.TRUE);
defaults.put(CONFIRM_DELETE, Boolean.TRUE);
defaults.put(CONFIRM_EMPTY_ENTRIES, Boolean.TRUE);
defaults.put(DELETE_EMPTY_ENTRIES, Boolean.TRUE);
defaults.put(DEFAULT_CITATION_KEY_PATTERN, "[auth][year]");
defaults.put(UNWANTED_CITATION_KEY_CHARACTERS, "-`ʹ:!;?^+");
defaults.put(DO_NOT_RESOLVE_STRINGS_FOR, StandardField.URL.getName());
Expand Down Expand Up @@ -1330,13 +1334,17 @@ public GeneralPreferences getGeneralPreferences() {
getBoolean(BIBLATEX_DEFAULT_MODE) ? BibDatabaseMode.BIBLATEX : BibDatabaseMode.BIBTEX,
getBoolean(WARN_ABOUT_DUPLICATES_IN_INSPECTION),
getBoolean(CONFIRM_DELETE),
getBoolean(CONFIRM_EMPTY_ENTRIES),
getBoolean(DELETE_EMPTY_ENTRIES),
getBoolean(MEMORY_STICK_MODE),
getBoolean(SHOW_ADVANCED_HINTS));

EasyBind.listen(generalPreferences.defaultEncodingProperty(), (obs, oldValue, newValue) -> put(DEFAULT_ENCODING, newValue.name()));
EasyBind.listen(generalPreferences.defaultBibDatabaseModeProperty(), (obs, oldValue, newValue) -> putBoolean(BIBLATEX_DEFAULT_MODE, (newValue == BibDatabaseMode.BIBLATEX)));
EasyBind.listen(generalPreferences.isWarnAboutDuplicatesInInspectionProperty(), (obs, oldValue, newValue) -> putBoolean(WARN_ABOUT_DUPLICATES_IN_INSPECTION, newValue));
EasyBind.listen(generalPreferences.confirmDeleteProperty(), (obs, oldValue, newValue) -> putBoolean(CONFIRM_DELETE, newValue));
EasyBind.listen(generalPreferences.confirmEmptyEntriesProperty(), (obs, oldValue, newValue) -> putBoolean(CONFIRM_EMPTY_ENTRIES, newValue));
EasyBind.listen(generalPreferences.deleteEmptyEntriesProperty(), (obs, oldValue, newValue) -> putBoolean(DELETE_EMPTY_ENTRIES, newValue));
EasyBind.listen(generalPreferences.memoryStickModeProperty(), (obs, oldValue, newValue) -> putBoolean(MEMORY_STICK_MODE, newValue));
EasyBind.listen(generalPreferences.showAdvancedHintsProperty(), (obs, oldValue, newValue) -> putBoolean(SHOW_ADVANCED_HINTS, newValue));

Expand Down