Review: Approve
Code LGTM, still needs testing.
I have also added a reply to one of your comments.
Diff comments:
>
> === modified file 'src/wui/load_or_save_game.cc'
> --- src/wui/load_or_save_game.cc 2018-10-26 07:09:29 +0000
> +++ src/wui/load_or_save_game.cc 2018-11-14 12:37:16 +0000
> @@ -269,12 +276,69 @@
> table_.focus();
> }
> if (do_delete) {
> + // Failed deletions aren't a serious problem, we just catch the
> errors
> + // and keep track to notify the player.
> + std::set<uint32_t> failed_selections;
> + bool failed;
> for (const uint32_t index : selections) {
> + failed = false;
> const std::string& deleteme = get_filename(index);
> - g_fs->fs_unlink(deleteme);
> - if (filetype_ == FileType::kReplay) {
> - g_fs->fs_unlink(deleteme + kSavegameExtension);
> - }
> + try {
> + g_fs->fs_unlink(deleteme);
> + } catch (const FileError& e) {
> + log("player-requested file deletion failed:
> %s", e.what());
> + failed = true;
> + }
> + if (filetype_ == FileType::kReplay) {
> + try {
> + g_fs->fs_unlink(deleteme +
> kSavegameExtension);
> + // If at least one of the two relevant
> files of a replay are
> + // successfully deleted then count it
> as success.
> + // (From the player perspective the
> replay is gone.)
> + failed = false;
> + // If it was a multiplayer replay, also
> delete the synchstream. Do
> + // it here, so it's only attempted if
> replay deletion was successful.
> + if (g_fs->file_exists(deleteme +
> kSyncstreamExtension)) {
> + g_fs->fs_unlink(deleteme +
> kSyncstreamExtension);
> + }
> + } catch (const FileError& e) {
> + log("player-requested file deletion
> failed: %s", e.what());
> + }
> + }
> + if (failed) {
> + failed_selections.insert(index);
> + }
> + }
> + if (!failed_selections.empty()) {
> + // Notify the player.
> + std::string caption = ngettext("Error Deleting File!",
Those are sins of the past ;)
> + "Error Deleting Files!", failed_selections.size());
> + if (filetype_ == FileType::kReplay) {
> + if (selections.size() == 1) {
> + header = _("The replay could not be
> deleted.");
> + } else {
> + header =
> + (boost::format(ngettext("%s replay
> could not be deleted.",
> + "%s replays could not be deleted.",
> failed_selections.size()))
> + % failed_selections.size()).str();
> + }
> + } else {
> + if (selections.size() == 1) {
> + header = _("The game could not be
> deleted.");
> + } else {
> + header =
> + (boost::format(ngettext("%s game
> could not be deleted.",
> + "%s games could not be deleted.",
> failed_selections.size()))
> + % failed_selections.size()).str();
> + }
> + }
> + std::string message = (boost::format("%s\n%s") % header
> + % filename_list_string(failed_selections)).str();
> + UI::WLMessageBox msgBox(
> + parent_->get_parent()->get_parent(),
> + caption, message,
> + UI::WLMessageBox::MBoxType::kOk);
> + msgBox.run<UI::Panel::Returncodes>();
> }
> fill_table();
>
--
https://code.launchpad.net/~widelands-dev/widelands/handling-various-fs-errors/+merge/358767
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/handling-various-fs-errors.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
