-
Notifications
You must be signed in to change notification settings - Fork 52
Description
Turns out several places in the raw ninja code, called by ninjaTools style functions, are returning error messages in a way that are not properly caught by our ninjaComMessageHandler methods, leading to a loss of our more specific error messages in the GUI. We currently get a bunch of generic error messages sent to the GUI instead.
Specifically, the raw ninja code currently returns error codes alongside CPLError() or std::cout printed error messages to the CLI, rather than using throw. There are times when using error codes instead of throws is the preferred behavior, sometimes the code needs to continue with just a warning rather than doing a full stop, but the ninjaTools level ninjaComMessageHandler currently can only catch thrown messages. Also, the error codes do not currently match specific error messages in a 1 to 1 manner, meaning we currently can't just assume a specific error code corresponds to a specific error message.
So the plan is as follows:
- Propagate the
ninjaToolslevelninjaComMessageHandlerto the various underlying rawninjacode methods. - If an
error codewith anerror messageis still the preferred behavior (warn and continue rather than a full stop), replace theCPLError()andstd::coutprintederror messageswithninjaComMessageHandlercalls. If the preferred behavior is to have a full stop, replace withthrow.
We might need to better define the error codes that correspond to specific error messages, but I think that can wait till later code cleanup for this.
There are a few other things that will need to be worked out as well. When using the try/catch and throw method, an error is passed around at each step that keeps various parts of the error together. The error code, error type, and error message are kept together. But when using the error code while printing error messages method, whether with CPLError(), std::cout, std::cerr, or with ninjaComMessageHandler to print the error message, these three parts of an error get separated out. Plus, at each level of the code an error gets passed through, try/catch and throw can be used to accumulate and contort the error message.
The ninjaComMessageHandler methods SHOULD be able to indirectly keep these three error parts together, to mimic this behavior, using getLastMessage(), but it will likely require a bit of work. We can wait to update ninjaCom to have better defined error types, but we will need to add some kind of message regulator methods to the GUI to better aid in this message accumulation process.
So, some additional steps that seem to be coming up are as follows:
3. Improve the message regulator methods in the Qt6 GUI. We already parse the error message to get the error type, but we need to add message regulation instead of just printing each and every error message as it is received. Let the user stop and process each error message as it comes up, rather than continuing on to the next message.
4. It makes more sense to send the error messages to a QMessageBox rather than to the QProgressDialog, leave the QProgressDialog stopped at whatever progress it had rather than setting the progress to 100% when an error message is shown, probably close the QProgressDialog once the error messages are fully processed by the user.
There is something up with the look of default QMessageBoxes in Qt6, might need to look into methods to make our QMessageBox instances look better.
This issue is related to issue #639, and slightly to issues #666 and #667.