riched20: new selection invalidation logic

Vitaliy Margolen wine-devel at kievinfo.com
Tue Aug 8 19:27:49 CDT 2006


Monday, August 7, 2006, 10:33:02 AM, Krzysztof Foltman wrote:
> 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.
Of course it's good when you don't affect anything else around your code.
If you can make some exception handlers that will catch that assert and print
nice error instead of RTF that would be awesome. And not a single person will be
upset. Users will have their functioning installers and you will have your error
reports.

> But there are valid reasons why those asserts should be avoided. It's 
> just about short term vs long term benefits.
Correct. 2 years ought to be enough?

>> 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).
I don't see how that can be if users report crashes immediately when installer
tries to show licence. Or when user tries to scroll text to the bottom so "Next"
button will become enabled.

> Anyway, the issue is not easy to solve, so I'd be grateful for any ideas 
> that may help in solving this important issue.
_TRY
{
  call riched20
 }
_EXCEPT(handler)
_ENDTRY

Seems like easy to me.

Vitaliy.









More information about the wine-devel mailing list