[Mono-dev] Patches for mono-winforms
Stifu
stifu at free.fr
Mon Jun 4 19:30:11 UTC 2012
I took your test, and turned it into a console application, and did what you
suggested. It seems it just hangs Mono. I tried the same, but making it a
WinForms app this time, and added a message box in the event handler. The
message box shows up on .NET (4 times) but not on Mono.
Attaching the WinForms app here.
http://mono.1490590.n4.nabble.com/file/n4649703/DgwTest.zip DgwTest.zip
Steven Boswell II wrote
>
> Re: patch 1, I have an idea. Take the unit-test code and run it as an
> application, and put a call to "Console.WriteLine
> (Environment.StackTrace)" in the event-handler. If the
> EditingControlShowing event isn't being called from my patched code, I'd
> like to know where it's being called from.
>
> And I'll go ahead and write unit tests for patches 2, 4, 5, and 7, if I
> can figure out some.
>
> Steven Boswell
>
>
>
> ________________________________
> From: Stifu <stifu@>
> To: mono-devel-list at .ximian
> Sent: Sunday, June 3, 2012 11:03 PM
> Subject: Re: [Mono-dev] Patches for mono-winforms
>
> "Also, as far as I can tell, patches 2, 4, and 5 are really small, and
> defy
> the creation of unit tests anyway -- it should be enough for someone
> knowledgeable about WinForms to decide if these changes are correct."
>
> No matter how small these patches are, and no matter if they're obviously
> correct. If something can be tested, it should be tested. :) I've
> committed
> several one-line patches, each of them coming with their unit test, so
> I've
> spent more time writing tests than fixes.
>
> This also ensures that no one breaks the tests / feature later on, by
> working on the code, trying to fix another bug, moving code around,
> refactoring... I guess you could easily come up with various stats showing
> that fixes without tests are XX% more likely to get broken again later.
> Tests also ensure we're on the same level across all OSes or machine type
> (for example, we could imagine patch number X does not fix the bug on Mac
> for some reason, and so this problem would be detected thanks to the
> test).
> To sum things up, tests are what makes a framework mature and reliable.
> Without them, you're just applying temporary patches.
>
> I'll check all of this out when I have more time. And I don't mind coming
> up
> with the tests myself, if I can figure out something. As for patch 1, I'll
> just check if I can reproduce the bug on Linux.
>
>
> Steven Boswell II wrote
>>
>> Yes, the unit test fails for me without my patch -- I made darn sure of
>> that after wasting your time. ;-) The additions to my unit test ensure
>> that the EditingControlShowing event is getting called. Without my
>> patch,
>> Mono never calls that event. So if it's succeeding for you...I'm totally
>> lost.
>>
>> Let me know if you need any more info, or any other changes, so that you
>> can commit patch 3.
>>
>> Also, as far as I can tell, patches 2, 4, and 5 are really small, and
>> defy
>> the creation of unit tests anyway -- it should be enough for someone
>> knowledgeable about WinForms to decide if these changes are correct. I
>> know that it brings Mono's behavior in line with .NET. Please let me
>> know
>> if I should do anything else with these patches.
>>
>> For the time being, I seem to be out of bugs, and am working on
>> annoyances...that's a good sign. :-) Enclosed is my latest change;
>> there's no unit test because it requires user interaction to even see the
>> problem. Before, getting a data-grid-view combo-box to drop down its
>> menu
>> took three mouse-clicks. Now, it only takes one. This brings the
>> behavior in line with .NET. Let me know what you think of this change.
>>
>> Steven Boswell
>>
>>
>> ________________________________
>> From: Stifu <stifu@>
>> To: mono-devel-list at .ximian
>> Sent: Sunday, June 3, 2012 3:20 PM
>> Subject: Re: [Mono-dev] Patches for mono-winforms
>>
>> Nope, I still can't get the tests to fail. :\
>> Does it fail for you (when unpatched)? It's possible the bug doesn't
>> affect
>> Mono on Windows, but I'd like to make sure of it.
>>
>> Can't see anything wrong with patch 3 at first sight.
>>
>>
>> Steven Boswell II wrote
>>>
>>> Sorry about that. I've added code to make sure the event-handler gets
>>> called. Enclosed is another attempt.
>>> Also, do you have any further comments about patch 3? For your
>>> convenience, I've enclosed another copy.
>>>
>>>
>>> ________________________________
>>> From: Stifu <stifu@>
>>> To: mono-devel-list at .ximian
>>> Sent: Sunday, June 3, 2012 8:58 AM
>>> Subject: Re: [Mono-dev] Patches for mono-winforms
>>>
>>> I checked out your unit tests for patch 1.
>>> On Mono on Windows, they already pass even without your patch. I cannot
>>> check Linux right now.
>>>
>>> Have you double checked that your tests fail without your patch?
>>> Thanks for clearing that up.
>>>
>>>
>>> Steven Boswell II wrote
>>>>
>>>> Crap, I didn't notice that InitializeEditingControl() was public.
>>>>
>>>> My whole motivation for splitting up that method was to make sure that
>>>> the
>>>> style modified by the EditingControlShowing event-handler would be
>>>> applied
>>>> to the cell. But that's done a few lines later, by the call to
>>>> "dgvEditingControl.ApplyCellStyleToEditingControl (style)". Argh.
>>>>
>>>> In any case, enclosed is a much simpler patch that doesn't change the
>>>> public API. Hopefully I didn't do anything else stupid -- I'm truly
>>>> not
>>>> trying to waste your time. ;-)
>>>>
>>>> Steven Boswell
>>>>
>>>>
>>>> ________________________________
>>>> From: Stifu <stifu@>
>>>> To: mono-devel-list at .ximian
>>>> Sent: Sunday, June 3, 2012 1:33 AM
>>>> Subject: Re: [Mono-dev] Patches for mono-winforms
>>>>
>>>> This patch does not look good to me, because you're changing the public
>>>> API.
>>>>
>>>> In DataGridViewCell, the InitializeEditingControl method expects 2
>>>> parameters rather than 3 after your patch, which means we're breaking
>>>> the
>>>> API and are no longer compatible with .NET.
>>>>
>>>>
>>>> Steven Boswell II wrote
>>>>>
>>>>> Argh...one more dumb oversight in my change.
>>>>> Enclosed is ANOTHER version of the patch.
>>>>> I wish I had the luxury of working on my hobbies when I was awake and
>>>>> energetic. ;-)
>>>>>
>>>>> Steven Boswell
>>>>>
>>>>>
>>>>> ________________________________
>>>>> From: Steven Boswell II <ulatekh@>
>>>>> To: "mono-devel-list at .ximian" <mono-devel-list at .ximian>
>>>>> Sent: Saturday, June 2, 2012 7:58 PM
>>>>> Subject: Re: [Mono-dev] Patches for mono-winforms
>>>>>
>>>>>
>>>>> Rob, you're my hero. Very few tests in DataGridViewTest.cs create a
>>>>> Form,
>>>>> but most of the ones that do involve data binding.
>>>>> I added a Form to my test, and it succeeded immediately. Apparently,
>>>>> Application.Run() isn't necessary, but that was a good idea.
>>>>> Enclosed is a revised patch, for review by the Powers That Be.
>>>>>
>>>>> Steven Boswell
>>>>>
>>>>>
>>>>> ________________________________
>>>>> From: Rob Wilkens <robwilkens@>
>>>>> To: mono-devel-list at .ximian
>>>>> Sent: Saturday, June 2, 2012 6:46 PM
>>>>> Subject: Re: [Mono-dev] Patches for mono-winforms
>>>>>
>>>>>
>>>>> On 06/02/2012 09:38 PM, Rob Wilkens wrote:
>>>>> On 06/02/2012 08:55 PM, Steven Boswell II wrote:
>>>>>>The EditingControlShowing event has to be called, and it has to be
>> called
>>>> after the control's contents have been initialized properly...that's
>>>> not
>>>> really two separate issues.
>>>>>>>
>>>>>>>
>>>>>>>The enclosed patch is an updated version; in addition to having a
unit
>>>> test, it fixes one additional bug revealed by my testing. Before,
>>>> DataGridViewComboBoxCell.InitializeEditingControl() was setting the
>>>> initial
>>>> value from the FormattedValue property, instead of
>>>> the initialFormattedValue
>>>> parameter.
>>>>>>>
>>>>>>>
>>>>>>>I tried to write an additional unit test that worked with bound data,
>>> but
>>>> for the life of me I can't figure out why it doesn't work. I've done
>>>> data-binding with DataGridView before...it wasn't this mysterious. In
>>>> my
>>>> unit test, after I set the DataGridView's DataSource property, the
>>>> data-grid
>>>> doesn't initialize properly; instead of four rows, it ends up with one
>>>> row,
>>>> and all its cell values are null. After beating my head against the
>>>> wall
>>>> for several hours, I'm perfectly happy to be told what a moron I am, if
>>>> someone will just tell me why the EditingControlShowingTest_Bound test
>>>> doesn't work. :-)
>>>>>>>
>>>>>>>
>>>>>>>Steven Boswell
>>>>>>>
>>>>>>Not writing to call you a moron, I'm a newbie myself and i could
>>>>> be wrong... But i copied and pasted your test code for
>>>>> EditingControlShowingTest_Bound into Visual Studio 2010, but
>>>>> rather than creating the DataGridVIew in code i placed it on the
>>>>> form and modified your code to use the one on the form... And i
>>>>> disabled the asserts... And from what i can tell it runs fine
in
>>>>> both .net and mono (that is, the data grid view populates).
>>>>>>
>>>>>>One thing that I noticed about your code, though, is it depends on
>>>>> a 'showing' event..
>>>>>>
>>>>>>And i wonder if that means your datagridview needs to be placed on
>>>>> a form which is displayed, so that it is actually shown...
>>>>>>
>>>>>>i.e. change your code something like this:
>>>>>>
>>>>>>using (Form Form1=new Form()){
>>>>>> Form1.Controls.Add(_dataGridView);//optionaly set sizeand
>>>>> location of both gridview and form
>>>>>> Form1.Show();
>>>>>>
>>>>>> ... insert the rest of your code here ....
>>>>>>}
>>>>>>
>>>>> Oh, and if i'm right, you might need the equivalent of an
>>>>> Application.Run(Form1) to process the events since you're listening
>>>>> for
>>>>> events, just don't forget to close Form1 or the Run loop might never
>>>>> end
>>>>> and your test can hang up with a displayed window...
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Mono-devel-list mailing list
>>>>> Mono-devel-list at .ximian
>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Mono-devel-list mailing list
>>>>> Mono-devel-list at .ximian
>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>> _______________________________________________
>>>>> Mono-devel-list mailing list
>>>>> Mono-devel-list at .ximian
>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>>
>>>>
>>>> --
>>>> View this message in context:
>>>> http://mono.1490590.n4.nabble.com/Patches-for-mono-winforms-tp4649620p4649670.html
>>>> Sent from the Mono - Dev mailing list archive at Nabble.com.
>>>> _______________________________________________
>>>> Mono-devel-list mailing list
>>>> Mono-devel-list at .ximian
>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>> _______________________________________________
>>>> Mono-devel-list mailing list
>>>> Mono-devel-list at .ximian
>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>
>>>
>>>
>>> --
>>> View this message in context:
>>> http://mono.1490590.n4.nabble.com/Patches-for-mono-winforms-tp4649620p4649674.html
>>> Sent from the Mono - Dev mailing list archive at Nabble.com.
>>> _______________________________________________
>>> Mono-devel-list mailing list
>>> Mono-devel-list at .ximian
>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>> _______________________________________________
>>> Mono-devel-list mailing list
>>> Mono-devel-list at .ximian
>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>
>>
>>
>> --
>> View this message in context:
>> http://mono.1490590.n4.nabble.com/Patches-for-mono-winforms-tp4649620p4649683.html
>> Sent from the Mono - Dev mailing list archive at Nabble.com.
>> _______________________________________________
>> Mono-devel-list mailing list
>> Mono-devel-list at .ximian
>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>> _______________________________________________
>> Mono-devel-list mailing list
>> Mono-devel-list at .ximian
>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>
>
> --
> View this message in context:
> http://mono.1490590.n4.nabble.com/Patches-for-mono-winforms-tp4649620p4649691.html
> Sent from the Mono - Dev mailing list archive at Nabble.com.
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at .ximian
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at .ximian
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
--
View this message in context: http://mono.1490590.n4.nabble.com/Patches-for-mono-winforms-tp4649620p4649703.html
Sent from the Mono - Dev mailing list archive at Nabble.com.
More information about the Mono-devel-list
mailing list