Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- When adding an entry to a library, a warning is displayed if said entry already exists in an active library. [#13261](https://github.com/JabRef/jabref/issues/13261)
- We added the field `monthfiled` to the default list of fields to resolve BibTeX-Strings for [#13375](https://github.com/JabRef/jabref/issues/13375)
- We added a new ID based fetcher for [EuropePMC](https://europepmc.org/). [#13389](https://github.com/JabRef/jabref/pull/13389)
- We added an initial [cite as you write](https://retorque.re/zotero-better-bibtex/citing/cayw/) endpoint. [#13187](https://github.com/JabRef/jabref/issues/13187)
- We added an initial [cite as you write](https://retorque.re/zotero-better-bibtex/citing/cayw/) endpoint. [#13187](https://github.com/JabRef/jabref/issues/13187)

### Changed

Expand Down Expand Up @@ -56,6 +56,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- When importing a PDF, there is no empty entry column shown in the multi merge dialog. [#13132](https://github.com/JabRef/jabref/issues/13132)
- We added a progress dialog to the "Check consistency" action and progress output to the corresponding cli command. [#12487](https://github.com/JabRef/jabref/issues/12487)
- We made the `check-consistency` command of the toolkit always return an exit code; 0 means no issues found, a non-zero exit code reflects any issues, which allows CI to fail in these cases [#13328](https://github.com/JabRef/jabref/issues/13328).
- Improved file exists warning dialog with clearer options and tooltips [#12565](https://github.com/JabRef/jabref/issues/12565)]

### Fixed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
import javafx.beans.property.SimpleDoubleProperty;
import javafx.beans.property.StringProperty;
import javafx.scene.Node;
import javafx.scene.control.Button;
import javafx.scene.control.ButtonBar;
import javafx.scene.control.ButtonType;
import javafx.scene.control.Dialog;
import javafx.scene.control.Tooltip;

import org.jabref.gui.AbstractViewModel;
import org.jabref.gui.DialogService;
Expand All @@ -37,6 +42,7 @@
import org.jabref.logic.externalfiles.LinkedFileHandler;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.TaskExecutor;
import org.jabref.logic.util.io.FileNameUniqueness;
import org.jabref.logic.util.io.FileUtil;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
Expand Down Expand Up @@ -268,20 +274,54 @@ private void performRenameWithConflictCheck(String targetFileName) {
boolean overwriteFile = false;

if (existingFile.isPresent()) {
overwriteFile = dialogService.showConfirmationDialogAndWait(
Localization.lang("File exists"),
Localization.lang("'%0' exists. Overwrite file?", targetFileName),
Localization.lang("Overwrite"));
// Get existing file path and its directory
Path existingFilePath = existingFile.get();
Path targetDirectory = existingFilePath.getParent();

// Suggest a non-conflicting file name
String suggestedFileName = FileNameUniqueness.getNonOverWritingFileName(targetDirectory, targetFileName);

// Define available dialog options
ButtonType Replace = new ButtonType(Localization.lang("Replace"), ButtonBar.ButtonData.OTHER);
ButtonType keepBoth = new ButtonType(Localization.lang("Keep both"), ButtonBar.ButtonData.OTHER);
ButtonType provideAlternative = new ButtonType(Localization.lang("Provide alternative file name"), ButtonBar.ButtonData.OTHER);
ButtonType cancel = new ButtonType(Localization.lang("Cancel"), ButtonBar.ButtonData.OTHER);
Copy link
Member

Choose a reason for hiding this comment

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

should be cancel close for button data


// Build the dialog window
Dialog<ButtonType> dialog = new Dialog<>();
Copy link
Member

Choose a reason for hiding this comment

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

Please rework. Use org.jabref.gui.JabRefDialogService. Please do not create dialogs on your own. - For example, look at the removed code 😅

MVVM is about testability.

This will also help to test this --> try to craft a test case after you use the JabRefDialogService :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created a custom modal because the current JabRefDialogService methods don’t support tooltips on buttons.
Should I add a new function to JabRefDialogService for this, or is there a better approach you’d recommend?

Copy link
Member

Choose a reason for hiding this comment

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

Yes please add a new function, maybe with a supplier as parameter for the tool tips

dialog.setTitle(Localization.lang("File already exists"));
dialog.setContentText(Localization.lang("File name: \n'%0'", targetFileName));
dialog.getDialogPane().getButtonTypes().addAll(Replace, keepBoth, provideAlternative, cancel);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this should be in viewmodel


// Add tooltip to "Keep both" button with the suggested name
Button keepBothButton = (Button) dialog.getDialogPane().lookupButton(keepBoth);
if (keepBothButton != null) {
keepBothButton.setTooltip(new Tooltip(Localization.lang("New filename: %0", suggestedFileName)));
}

if (!overwriteFile) {
// Show dialog and handle user response
Optional<ButtonType> result = dialog.showAndWait();

if (result.isEmpty() || result.get() == cancel) {
return; // User canceled
} else if (result.get() == Replace) {
overwriteFile = true;
} else if (result.get() == keepBoth) {
targetFileName = suggestedFileName;
} else if (result.get() == provideAlternative) {
askForNameAndRename();
return;
}
}

try {
// Attempt the rename operation
linkedFileHandler.renameToName(targetFileName, overwriteFile);
} catch (IOException e) {
dialogService.showErrorDialogAndWait(Localization.lang("Rename failed"), Localization.lang("JabRef cannot access the file because it is being used by another process."));
// Display an error dialog if file is locked or inaccessible
dialogService.showErrorDialogAndWait(
Localization.lang("Rename failed"),
Localization.lang("JabRef cannot access the file because it is being used by another process."));
}
}

Expand Down
6 changes: 6 additions & 0 deletions jablib/src/main/resources/l10n/JabRef_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,12 @@ Do\ not\ abbreviate\ names=Do not abbreviate names

Do\ not\ wrap\ when\ saving=Do not wrap when saving

File\ already\ exists=File already exists
Copy link
Member

Choose a reason for hiding this comment

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

Pleae group to line 2999

(there is File '%0' already exists. Overwriting.)

File\ name\:\ \n'%0'=File name: \n'%0'
Copy link
Member

Choose a reason for hiding this comment

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

There should be no space before newline

Maybe you can submit a follow-up PR?

New\ filename\:\ %0=New filename: %0
Provide\ alternative\ file\ name=Provide alternative file name


Download\ file=Download file

Download\ '%0'\ was\ a\ HTML\ file.\ Removed.=Download '%0' was a HTML file. Removed.
Expand Down