Backups: handle discarded changes v1 #9441
Conversation
|
Hello! Welcome to JabRef! I have read through the changes made in this PR and I'm not entirely sure what is the underlying issue you are addressing here. If you could provide more information or context, that would be great. If this PR is in response to an issue that's already been reported, please include a link to the issue in the PR description. If this is a new issue, please describe it here and mention your motivations for addressing it. |
calixtus
added
the
status: waiting-for-feedback
ThiloteE
added
component: export-or-save
component: backup
and removed
status: waiting-for-feedback
| @Article{, | ||
| author = {Test1-abc}, | ||
| title = {Test1}, | ||
| month = may, | ||
| } | ||
|
|
||
| @Comment{jabref-meta: databaseType:bibtex;} |
There was a problem hiding this comment.
The path of the file is very strange. I think you accidently commited the file and it should not be part of the PR?
| String backUpPath = BackupFileUtil.getPathOfLatestExisingBackupFile(directory, BackupFileType.BACKUP).map(Path::toString). | ||
| orElse(Localization.lang("File not found")); |
There was a problem hiding this comment.
There should not be a file File not found be created.
| @@ -189,7 +198,8 @@ private Optional<Path> determineBackupPathForNewBackup() { | |||
| * @param backupPath the path where the library should be backed up to | |||
| */ | |||
| private void performBackup(Path backupPath) { | |||
| if (!needsBackup) { | |||
|
|
|||
| if (!needsBackup || discardedFileExists) { | |||
There was a problem hiding this comment.
Why the check here? A backup should be performed regardless of the discared file. The discarded file was of the last JabRef run.
|
|
||
| Path directory = Path.of(filename); | ||
| try { | ||
| SaveDatabaseAction saveAction = new SaveDatabaseAction(libraryTab, prefs, Globals.entryTypesManager); |
There was a problem hiding this comment.
Do NOT Call UI methods for saving. The logic for creating a --discared file should be in the BackupManager. You can see the saving at org.jabref.logic.autosaveandbackup.BackupManager#performBackup.
As written in #9361 (comment), it really should be done in BackupManager.
| if (saveAction.saveAs(Path.of(backUpPath))) { | ||
| BackupManager.setDiscardedFileExists(true); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
I think, you misunderstood the user interaction of JabRef.
- User starts JabRef
- User opens a file
demo.bib - User changes file
demo.bibfor 10 minutes (and does not save) - User closes file.
- JabRef diplays "File changed dialog"
- User selects "Discard changes"
- User opens the file
demo.bibagain - JabRef should NOT prompt for restoring backup
In the code at hand, sets the flag after step 7, but not at step 8, where it is needed.
|
Superseeded by #9457 |
ThiloteE
changed the title
ThiloteE
changed the title
Fix #9361
CHANGELOG.mddescribed in a way that is understandable for the average user (if applicable)