Vitaliy Margolen wrote:

I think this patch takes number of asserts per line to the highest level I've
ever seen. Could you explain why you adding asserts everywhere (only 11 in this
patch alone!!!)? Instead of using proper error handling?

Well, that's the good question. The thing is, those conditions should never happen, and if they do happen, then something is really wrong somewhere with the code and a crash may happen anyway. I think it's better to get an assertion failure early than a random crash when it's too late to diagnose anything. And one can always comment the asserts out.

But there are valid reasons why those asserts should be avoided. It's just about short term vs long term benefits.

Maybe an option to disable those asserts would be a good idea so that people who just need to get the job done aren't messed with - but then we don't get half the bug reports we get now.

As for the enormous (or even insane) amount of asserts - there's no such thing ;) Those asserts are pretty "paranoid-level" - chances of any of them happening is really, really low - and that's totally different to my previous repaint patch which had one assertion I predicted may be risky (which proved something is wrong with repaint, so it kinda served the purpose).

The good solution would be a decent regression test, but it seems to be pretty difficult and/or tedious (simulating real-life conditions for testing repainting is hard because of relative timing/order of window messages).

For the past year or more that was the main source of user problems. And if you
could how many installers have been broken with this technique, you will
probably account for 90% of all the installers that otherwise would work on
Wine.

Have you even tried your code on purposely broken rtf? How does it handle it?

Those asserts are relatively independent on text input, and more on user interaction with the editor, which, as I already said, is hard to simulate and test (that's why I'm kind of shifting responsibility to end users: I may do as much monkey tests as you want, still I may get into some ruts of usage pattern).

Anyway, the issue is not easy to solve, so I'd be grateful for any ideas that may help in solving this important issue.

Krzysztof



Reply via email to