Improvements to error catching and reporting #649
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It is clearly based on a CLI's needs. - remove unused parameter - split Uicommon.uiInit into two and simulate old Uicommon.uiInit by sequencing the two stages Hint: could be reviewed individually or combined with the next commit (the latter may even be better for seeing what's going on) the following git options may help when reviewing --color-moved=blocks
- remove Uicommon.uiInit and call split stages directly Hint: could be reviewed individually or combined with the previous commit (the latter may even be better for seeing what's going on) the following git options may help when reviewing --color-moved=blocks
- eliminate Uicommon.uiInitStage2 completely, moving common parts (only initPrefs) to uitext and uigtk2, respectively; the remaining parts are moved to uitext this commit is intentionally not compiling, to show the changes better Hint: the following git options may help when reviewing --color-moved=blocks --color-moved-ws=ignore-space-change
make the previous commit compile Hint: the following git options may help when reviewing --word-diff=plain
- let each UI (not the common code) decide when to interact with the user and when to exit -- replace explicit callbacks and [exit] calls with an Ok/Error result type - implement uitext first Hint: the following git options may help when reviewing --word-diff=plain
- same as previous, implement uigtk2 Hint: the following git options may help when reviewing --color-moved=blocks --color-moved-ws=ignore-space-change
Previously the changes were kept to a minimum, just to highlight code moves better. Now clean up the resulting code, starting with uicommon... Hint: the following git options may help when reviewing --word-diff=plain (or just -w)
... and continuing with uitext. Hint: the following git options may help when reviewing --word-diff=plain
found in older code; not directly related to the restructuring
Add a Close button to the fatal error message dialog. Clean up and structure the code to either leave the GUI in the neutral state and allow the user to try again or change the profile; or force a profile change if still in the early init process. Clean up global state and guard the code against continuing after an interrupted action. Hint: the following git options may help when reviewing -w --color-moved=blocks --color-moved-ws=ignore-all-space
Previously, an error during initial scanning the profiles would force the user interface to quit without user being able to rectify the situation (indeed, nor even understand what is going on). This would happen at an error in _any_ profile, not just the one the user had requested. Since this scanning is only done to build a list of profiles for the profile manager, we can safely ignore any errors at this stage (while still notifying the user).
By default, this early in the init process, the application would just quit. These warnings are not fatal errors, nor are they relevant at this stage. Just print out the warning and continue normally.
This is to better organise code and not force scanning the profiles before the GUI is built (any error/warning during the scanning would then go lost or force the exit before the GUI is even built). Later, should be updated to update the menu every time profile manager was opened (because profiles may have been added/removed/changed).
Some connection errors were reported twice, first as a warning and then a fatal error. Make them what they really are - fatal errors - and report only once. Hint: the following git options may help when reviewing --word-diff=plain
... and actually report nice errors to the user, rather than just fail with an uncaught exception and a backtrace. While the bulk of this change is in the GUI, the resiliency and error reporting applies to CLI as well. Hint: the following git options may help when reviewing --color-moved=blocks --color-moved-ws=ignore-space-change
Move commit logs processing to the earliest moment after roots have been installed. Moved path expansion closer to prefs propagation, just for logical grouping. Hint: the following git options may help when reviewing --color-moved=plain --color-moved-ws=ignore-all-space
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a collection of bug fixes and improvements to various error situations. There are several independent changes and bug fixes, the major part is the fix to #442, keeping the GUI alive after any "fatal" errors. The bug fixes and improvements are not GUI-specific.
It could be possible to merge several of the commits independently, if needed.
Fixes #442
For reviewers
The number of commits is large to assist in reviewing (the first 8 commits are step-by-step preparations before the big GUI commit). Most of them will be squashed before merging. Each commit indicates git options that may be helpful when reviewing that commit. While it may be easier to review commit-by-commit, reviewing several at a time, or even everything together, should be reasonably simple and effortless (it turns out that GitHub does a mostly decent job displaying the diff).