Skip to content

Commit

Permalink
fix: selection export dialog failing silently, code cleanup (#1227)
Browse files Browse the repository at this point in the history 
* fix: selection export dialog failing silently, code cleanup,

upon pressing the selection export confirm button, the callback responsible
for exporting only took a weak ref to the selected file.
This in turn caused a race condition where the task executing the export raced
against the dialog callback being finished, where the selected file is dropped,
causing the weak ref upgrade to fail.

* chore: improve error toast messages
  • Loading branch information
@flxzt
flxzt authored Sep 28, 2024
1 parent f51a90f commit bbf5417
Showing 1 changed file with 110 additions and 105 deletions.
215 changes: 110 additions & 105 deletions crates/rnote-ui/src/dialogs/export.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,49 +271,51 @@ pub(crate) async fn dialog_export_doc_w_prefs(appwindow: &RnAppWindow, canvas: &
export_doc_button_confirm.connect_clicked(clone!(#[weak] dialog, #[weak] canvas, #[weak] appwindow , move |_| {
dialog.close();

if let Some(file) = selected_file.take() {
glib::spawn_future_local(clone!(#[weak] canvas, #[weak] appwindow , async move {
appwindow.overlays().progressbar_start_pulsing();

let file_title = crate::utils::default_file_title_for_export(
Some(file.clone()),
Some(&canvas::OUTPUT_FILE_NEW_TITLE),
None,
);
if let Err(e) = canvas.export_doc(&file, file_title, None).await {
error!("Exporting document failed, Err: `{e:?}`");

appwindow.overlays().dispatch_toast_error(&gettext("Exporting document failed"));
appwindow.overlays().progressbar_abort();
} else {
appwindow.overlays().dispatch_toast_w_button(
&gettext("Exported document successfully"),
&gettext("View in file manager"),
clone!(#[weak] appwindow , move |_reload_toast| {
let Some(folder_path_string) = file
.parent()
.and_then(|p|
p.path())
.and_then(|p| p.into_os_string().into_string().ok()) else {
error!("Failed to get the parent folder of the output file `{file:?}.");
appwindow.overlays().dispatch_toast_error(&gettext("Exporting document failed"));
return;
};

if let Err(e) = open::that(&folder_path_string) {
error!("Opening the parent folder '{folder_path_string}' in the file manager failed, Err: {e:?}");
appwindow.overlays().dispatch_toast_error(&gettext("Failed to open the file in the file manager"));
}
}
), crate::overlays::TEXT_TOAST_TIMEOUT_DEFAULT);
appwindow.overlays().progressbar_finish();
}
}));
} else {
let Some(file) = selected_file.take() else {
appwindow
.overlays()
.dispatch_toast_error(&gettext("Exporting document failed, no file selected"));
}
return;
};

glib::spawn_future_local(clone!(#[weak] canvas, #[weak] appwindow , async move {
appwindow.overlays().progressbar_start_pulsing();

let file_title = crate::utils::default_file_title_for_export(
Some(file.clone()),
Some(&canvas::OUTPUT_FILE_NEW_TITLE),
None,
);

if let Err(e) = canvas.export_doc(&file, file_title, None).await {
error!("Exporting document failed, Err: `{e:?}`");
appwindow.overlays().dispatch_toast_error(&gettext("Exporting document failed"));
appwindow.overlays().progressbar_abort();
return
}

appwindow.overlays().dispatch_toast_w_button(
&gettext("Exported document successfully"),
&gettext("View in file manager"),
clone!(#[weak] appwindow , move |_reload_toast| {
let Some(folder_path_string) = file
.parent()
.and_then(|p|
p.path())
.and_then(|p| p.into_os_string().into_string().ok()) else {
error!("Failed to get the parent folder of the output file `{file:?}.");
appwindow.overlays().dispatch_toast_error(&gettext("Failed to view the file in the file manager"));
return;
};

if let Err(e) = open::that(&folder_path_string) {
error!("Opening the parent folder '{folder_path_string}' in the file manager failed, Err: {e:?}");
appwindow.overlays().dispatch_toast_error(&gettext("Failed to view the file in the file manager"));
}
}
), crate::overlays::TEXT_TOAST_TIMEOUT_DEFAULT);
appwindow.overlays().progressbar_finish();
}));
}));

dialog.present(appwindow.root().as_ref());
Expand Down Expand Up @@ -685,41 +687,43 @@ pub(crate) async fn dialog_export_doc_pages_w_prefs(appwindow: &RnAppWindow, can
export_doc_pages_button_confirm.connect_clicked(clone!(#[weak] export_files_stemname_entryrow, #[weak] dialog, #[weak] canvas, #[weak] appwindow, move |_| {
dialog.close();

if let Some(dir) = selected_file.take() {
glib::spawn_future_local(clone!(#[weak] export_files_stemname_entryrow, #[weak] canvas, #[weak] appwindow, async move {
appwindow.overlays().progressbar_start_pulsing();

let file_stem_name = export_files_stemname_entryrow.text().to_string();
if let Err(e) = canvas.export_doc_pages(&dir, file_stem_name, None).await {
error!("Exporting document pages failed, Err: {e:?}");

appwindow.overlays().dispatch_toast_error(&gettext("Exporting document pages failed"));
appwindow.overlays().progressbar_abort();
} else {
appwindow.overlays().dispatch_toast_w_button(
&gettext("Exported document pages successfully"),
&gettext("View in file manager"),
clone!(#[weak] appwindow, move |_reload_toast| {
let Some(folder_path_string) = dir.path().and_then(|p| p.into_os_string().into_string().ok()) else {
error!("Failed to get the path of the parent folder");
appwindow.overlays().dispatch_toast_error(&gettext("Exporting document failed"));
return;
};

if let Err(e) = open::that(&folder_path_string) {
error!("Opening the parent folder '{folder_path_string}' in the file manager failed, Err: {e:?}");
appwindow.overlays().dispatch_toast_error(&gettext("Failed to open the file in the file manager"));
}
}
), crate::overlays::TEXT_TOAST_TIMEOUT_DEFAULT);
appwindow.overlays().progressbar_finish();
}
}));
} else {
let Some(dir) = selected_file.take() else {
appwindow.overlays().dispatch_toast_error(&gettext(
"Exporting document pages failed, no directory selected",
));
}
return;
};

glib::spawn_future_local(clone!(#[weak] export_files_stemname_entryrow, #[weak] canvas, #[weak] appwindow, async move {
appwindow.overlays().progressbar_start_pulsing();

let file_stem_name = export_files_stemname_entryrow.text().to_string();

if let Err(e) = canvas.export_doc_pages(&dir, file_stem_name, None).await {
error!("Exporting document pages failed, Err: {e:?}");
appwindow.overlays().dispatch_toast_error(&gettext("Exporting document pages failed"));
appwindow.overlays().progressbar_abort();
return
}

appwindow.overlays().dispatch_toast_w_button(
&gettext("Exported document pages successfully"),
&gettext("View in file manager"),
clone!(#[weak] appwindow, move |_reload_toast| {
let Some(folder_path_string) = dir.path().and_then(|p| p.into_os_string().into_string().ok()) else {
error!("Failed to get the path of the parent folder");
appwindow.overlays().dispatch_toast_error(&gettext("Failed to view the file in the file manager"));
return;
};

if let Err(e) = open::that(&folder_path_string) {
error!("Opening the parent folder '{folder_path_string}' in the file manager failed, Err: {e:?}");
appwindow.overlays().dispatch_toast_error(&gettext("Failed to view the file in the file manager"));
}
}
), crate::overlays::TEXT_TOAST_TIMEOUT_DEFAULT);
appwindow.overlays().progressbar_finish();
}));
}));

dialog.present(appwindow.root().as_ref());
Expand Down Expand Up @@ -1033,50 +1037,51 @@ pub(crate) async fn dialog_export_selection_w_prefs(appwindow: &RnAppWindow, can
}
));

export_selection_button_confirm.connect_clicked(clone!(#[weak] selected_file, #[weak] dialog, #[weak] canvas, #[weak] appwindow , move |_| {
export_selection_button_confirm.connect_clicked(clone!(#[weak] dialog, #[weak] canvas, #[weak] appwindow , move |_| {
dialog.close();

glib::spawn_future_local(clone!(#[weak] selected_file, #[weak] canvas, #[weak] appwindow , async move {
let Some(file) = selected_file.take() else {
appwindow
.overlays()
.dispatch_toast_error(&gettext("Exporting selection failed, no file selected"));
return;
};
let Some(file) = selected_file.take() else {
appwindow
.overlays()
.dispatch_toast_error(&gettext("Exporting selection failed, no file selected"));
return;
};

glib::spawn_future_local(clone!(#[weak] canvas, #[weak] appwindow , async move {
appwindow.overlays().progressbar_start_pulsing();

if let Err(e) = canvas.export_selection(&file, None).await {
error!("Exporting selection failed, Err: {e:?}");

appwindow
.overlays()
.dispatch_toast_error(&gettext("Exporting selection failed"));
appwindow.overlays().progressbar_abort();
} else {
appwindow.overlays().dispatch_toast_w_button(
&gettext("Exported selection successfully"),
&gettext("View in file manager"),
clone!(#[weak] appwindow , move |_reload_toast| {
let Some(folder_path_string) = file
.parent()
.and_then(|p|
p.path())
.and_then(|p| p.into_os_string().into_string().ok()) else {
error!("Failed to get the parent folder of the output file `{file:?}.");
appwindow.overlays().dispatch_toast_error(&gettext("Exporting document failed"));
return;
};

if let Err(e) = open::that(&folder_path_string) {
error!("Opening the parent folder '{folder_path_string}' in the file manager failed, Err: {e:?}");
appwindow.overlays().dispatch_toast_error(&gettext("Failed to open the file in the file manager"));
}
}),
crate::overlays::TEXT_TOAST_TIMEOUT_DEFAULT,
);
appwindow.overlays().progressbar_finish();
return;
}
}));

appwindow.overlays().dispatch_toast_w_button(
&gettext("Exported selection successfully"),
&gettext("View in file manager"),
clone!(#[weak] appwindow , move |_| {
let Some(folder_path_string) = file
.parent()
.and_then(|p|
p.path())
.and_then(|p| p.into_os_string().into_string().ok()) else {
error!("Failed to get the parent folder of the output file `{file:?}.");
appwindow.overlays().dispatch_toast_error(&gettext("Failed to view the file in the file manager"));
return;
};

if let Err(e) = open::that(&folder_path_string) {
error!("Opening the parent folder '{folder_path_string}' in the file manager failed, Err: {e:?}");
appwindow.overlays().dispatch_toast_error(&gettext("Failed to view the file in the file manager"));
}
}),
crate::overlays::TEXT_TOAST_TIMEOUT_DEFAULT,
);
appwindow.overlays().progressbar_finish();
}));
}));

dialog.present(appwindow.root().as_ref());
Expand Down

0 comments on commit bbf5417

Please sign in to comment.