[Mono-dev] Patches for mono-winforms

Stifu stifu at free.fr
Mon Jun 4 21:57:58 UTC 2012 

I've just pushed patch 3
(https://github.com/mono/mono/commit/bcb49c60cdb9e9797fa91473955fe71828805643).
Thanks.

By the way, I have noticed a few remaining subtle differences with .NET, but
I guess we have enough on our hands as is...


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-tp4649620p4649709.html
Sent from the Mono - Dev mailing list archive at Nabble.com.

More information about the Mono-devel-list mailing list