Fix project switching bug - #68
lisham2000 wants to merge 2 commits into
Conversation
520da42 to
09b08b7
Compare
KRLango
left a comment
KRLango
left a comment
There was a problem hiding this comment.
A few changes on the project switching. The scoping change looks good to me though.
| except (AttributeError, RuntimeError): | ||
| return None | ||
|
|
||
| fallback_window: Facade.DocumentWindow | None = None |
There was a problem hiding this comment.
The name fallback_window needs some explanation as to why this is the window that should be used as the fallback.
| if window.target_display is not None: | ||
| try: | ||
| target_display = window.target_display | ||
| window.target_data_item |
There was a problem hiding this comment.
What are you trying to achieve with this line?
| target_display = window.target_display | ||
| window.target_data_item | ||
| except (AttributeError, RuntimeError): | ||
| # Workspace/project changes can temporarily expose PopupWindow objects. |
There was a problem hiding this comment.
Would an explicit check for a popup window be better rather than relying on exceptions?
| # Workspace/project changes can temporarily expose PopupWindow objects. | ||
| continue | ||
|
|
||
| if fallback_window is None: |
There was a problem hiding this comment.
This will always select the first window that passes the previous checks. A comment for why this one is correct should be added.
|
|
||
| if target_display is not None: | ||
| return target_display.data_item | ||
| except (AttributeError, RuntimeError): |
There was a problem hiding this comment.
Would checking the type of the window or getattr calls be better here rather than using exceptions as flow control?
This branch has not been deployed
2 participants
Fixes https://github.com/nion-software/nion-internal/issues/366
Does not currently fix scan axis additional rotation but does fix a bug that occurred giving a traceback when axes were present project switching bug. Swaps all ui functions to being selected display item only