-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fix adding files with illegal characters #10205
Conversation
Fixes #10182 Display a dialog with auto renaming option
src/main/java/org/jabref/gui/externalfiles/ExternalFilesEntryLinker.java
Show resolved
Hide resolved
src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java
Show resolved
Hide resolved
src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java
Show resolved
Hide resolved
Your code currently does not meet JabRef's code guidelines. The tool reviewdog already placed comments on GitHub to indicate the places. See the tab "Files" in you PR. Please carefully follow the setup guide for the codestyle. Afterwards, please run checkstyle locally and fix the issues. More information on code quality in JabRef is available at https://devdocs.jabref.org/getting-into-the-code/development-strategy.html. |
src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java
Show resolved
Hide resolved
The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build. |
I suggest rephrasing the right button "Rename and add" and remove the two sentences describing sentences in the dialog text. |
private List<Path> getValidFileNames(List<Path> filesToAdd) { | ||
List<Path> validFileNames = new ArrayList<>(); | ||
|
||
for (Path fileToAdd : filesToAdd) { | ||
if (FileUtil.detectBadFileName(fileToAdd.toString())) { | ||
String newFilename = FileNameCleaner.cleanFileName(fileToAdd.getFileName().toString()); | ||
|
||
boolean correctButtonPressed = dialogService.showConfirmationDialogAndWait(Localization.lang("File \"%0\" cannot be added!", fileToAdd.getFileName()), | ||
Localization.lang("Illegal characters in the file name detected.\nFile will be renamed to \"%0\" and added.", newFilename), | ||
Localization.lang("Rename and add")); | ||
|
||
if (correctButtonPressed) { | ||
Path correctPath = fileToAdd.resolveSibling(newFilename); | ||
try { | ||
Files.move(fileToAdd, correctPath); | ||
validFileNames.add(correctPath); | ||
} catch (IOException ex) { | ||
LOGGER.error("Error moving file", ex); | ||
dialogService.showErrorDialogAndWait(ex); | ||
} | ||
} | ||
} else { | ||
validFileNames.add(fileToAdd); | ||
} | ||
} | ||
return validFileNames; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of code duplication to LinkedFilesEditorViewModel, but fine for a hotfix. Maybe an followup PR should clean this up.
Fixes #10182
Also supported for drag and drop
(The FilenameCleanerTest can be converted to parameterized test, bu this can be an easy student task)
Display a dialog with auto-renaming option
Mandatory checks
CHANGELOG.md
described in a way that is understandable for the average user (if applicable)