-
Notifications
You must be signed in to change notification settings - Fork 137
Description
Suggest moving show_log_as_dialog() to MultiFrame.py
In commit cf79a3e @t0b3 created silhouette/MultiFrame.py with lots of code moved from silhouette_multi.py. wrapup() was one of the functions moved over, but without any modification to it's call of show_log_as_dialog(). This function call is now broken, as identified by @TapuCosmo in #308. However, the call should not be removed, as Tapu suggests, but repaired.
In silhoutte_multi.py, show_log_as_dialog() is called 1 time during exception handling BUT ONLY IF unblock_inkscape is set to true? Why? It doesn't care if the exception happened during instantiation or .run() so really no indication of if unblocking has anything to do with the exception or not. It is clearly not critical to create this extra dialog here, and it's primary purpose was always to create these dialogues as part of wrapup().
In MultiFrame.py, show_log_as_dialog() is called 1 time and that is in wrapup(). MultiFrame.wrapup() is called 3 times, but 2 of them are in silhouette_multi. So silhouette_multi calls MultiFrame.wrapup() which calls show_log_as_dialog() which is instead in the class where the function call just came from? Why? This could probably be done better, or at least commented better.
At the moment, it might be that anytime there is a wrapup() with unblock_inkscape == TRUE it's causing an exception when it tries to call show_log_as_dialog() with then gets caught by the exception handling and just calls show_log_as_dialog() anyway, but from a different scope, possibly masking and/or conflating problems and confusing the hell out of everyone. I'm certainly confused.
Can we move show_log_as_dialog() to Multiframe and remove lines 325-328? https://github.com/fablabnbg/inkscape-silhouette/blob/1e8261eeac70e753b5df2415cc0e896fd67ae994/silhouette_multi.py#L325C1-L328C33 It would clean up the code and maybe also solve some problems. Including #213