Den 08/08/2013 17:32, skrev Christian Dywan: > Review: Needs Fixing > >>> getValidTargets() and synchronizeTargets() hard-code HTML formatting - >> that's not good if we plan to see errors in console or XML and will make those >> cases harder to deal with. The messages should be predictable without using >> markup - if needed, it's still an option to use regex. >>> m_errors.append("Database >> "+index_number+": F >> >> The type of errors here are for the benefit of app devs and app users at >> the UI level, not console etc. The messages are for use in a QML >> application as output from a sync transation to notify re: errors, >> warnings and things that went according to plan. >> >> An alternative approach, with broader and more flexible possibilities, >> is to have the error information available in a model (QVariantMap) that >> includes useful meta data and values (e.g. type of message, message >> text, other?). In that way it can be available to app developers as well >> as easily adapted for other purposes (e.g. log files, console output). > Okay, I better see the reasoning now. And I think you're mixing two concepts into here, UI error messages and error handling API for letting developers distinguish errors. The user must not see verbatim key names. And the app developer needs a way to distinguish a failing sync from a bug in his code. The messages can and should be modified. But the app developer does not need to necessarily use the log/error/warning output from sync in its raw form either. It should be up to the developer to determine how they want to work with the data. > > I had a thought in that direction in respect to Database::setError originally. SQL errors aren't interesting to anyone but U1Db-Qt developers - they need to be simple and pastable. The same is often true for U1Db-Qt errors. Errors about invalid id's or json are indeed relevant for developers - but only in the console. There's no need to "handle" them, wrong use of the API is to be fixed. One kind of error that we should have API for is: no space left. I apparently never filed a bug for that. > > Now for sync, I'd still want the same console output for obvious mistakes like invalid URLs and wrong key names. That is what I was having in mind when I suggested to not have HTML. It's a one-time see the mistake and fix it and not for UI. And it is useful to always log if a connection failed in any way. For the developer's benefit we should have "signal syncFailed". That should suffice to prompt the user appropriately - the developer knows their target audience, what language to use etc. Assuming too much about what the app developer wants, the endless possible types of errors that might be encountered (e.g. faulty app logic, user input variations, API problems), or who their audience is, along rigid lines (and then use that logic for designing the API), may be asking for trouble. What one developer to the next might want (or need) could be different enough that "signal syncFailed" isn't enough. This is why I suggested having a much more flexible and less discriminating approach of making the data vailable using QVarientMap, with meta data included so u1db dev and app dev can use the information in ways they choose, for the ultimate benefit of the end user. The existing approach is indeed immature in that respect, but the suggestion of using a model (QVarientMap) moves beyond current limitations and is flexible enough for a broad set of use cases, some of which we cannot anticipate on our own, but without creating huge overhead. It is intended to provide a rich mechanism for developers to create robust applications for users that do more than simply give feedback that only says "fail". Additionally it can hopefully provide useful information for u1db-qt devs as well (who also might happen to build apps with the plugin...including examples). Kevin