Issue 5183 in pharo: Problem with PluggableListMorph

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

Issue 5183 in pharo: Problem with PluggableListMorph

pharo
Status: Accepted
Owner: [hidden email]
Labels: Milestone-1.4

New issue 5183 by [hidden email]: Problem with PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

There are cases when Pluggable List Morph has is inner list cache in nil:

getListElementSelector: aSymbol
     "specify a selector that can be used to obtain a single element in the  
underlying list"
     getListElementSelector := aSymbol.
     list := nil.  "this cache will not be updated if getListElementSelector  
has been specified, so go ahead and remove it"

But, there are some methods that send messages to that nil list and blow  
up :D, such as:

backgroundColorFor: aRow

     | anItem return |
     aRow ifNil: [ ^nil ].
     anItem := list at: aRow ifAbsent: [ ^ nil ].
...

Now, two solutions comes to my mind:

put one more ward in:

backgroundColorFor: aRow

     | anItem return |
     (aRow isNil or: [ list isNil ]) ifTrue: [ ^nil ].
     anItem := list at: aRow ifAbsent: [ ^ nil ].
...

Or use #getList, which seems to be the most... cleaner.

backgroundColorFor: aRow

     | anItem return |
     aRow ifNil: [ ^nil ].
     anItem := self getList at: aRow ifAbsent: [ ^ nil ].
...

?



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo

Comment #1 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

The second fix

Name: SLICE-Issue-5183-Problem-with-PluggableListMorph-GuillermoPolito.2
Author: GuillermoPolito
Time: 17 January 2012, 11:53:25 am
UUID: 8e64c18a-c09f-45d1-9977-ef3c8627834b
Ancestors:  
SLICE-Issue-5183-Problem-with-PluggableListMorph-GuillermoPolito.1
Dependencies: Morphic-GuillermoPolito.1070


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo
Updates:
        Status: FixReviewNeeded

Comment #2 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo
Updates:
        Status: Closed
        Cc: [hidden email] [hidden email]

Comment #3 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

in 14290


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo

Comment #4 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

Do you have an example where it hangs ?

Because usually, the list should e calculated before the background or  
selector thingy.
Here calling getList is slow, because it ask the model for the list, which  
means you recalculate the whole list for each item ...

I think it's far too slow


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo

Comment #5 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

Just an example: http://dl.dropbox.com/u/24369478/5183.mov


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo

Comment #6 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

Try loading Omnibrowser :/.

When the browser opens, the category list displays the Red square of  
death :P ...

And this is because OB sends #getListElementSelector: to it's  
PluggableListMorph in here:

listMorphForColumn: aColumn
        ^ (OBPluggableListMorph
                on: aColumn
                list: #list
                selected: #selection
                changeSelected: #selection:
                menu: #menu:
                keystroke: #keystroke:from:)
                        getListElementSelector: #listAt:;
                        getListSizeSelector: #listSize;
                        dragEnabled: aColumn dragEnabled;
                        dropEnabled: aColumn dropEnabled;
                        doubleClickSelector: #doubleClick;
                        alwaysShowScrollBars: false;
                        borderWidth: 0;
                        autoDeselect: false;
                        yourself

To reproduce it just see:

(ConfigurationOfOmniBrowser
        project version: #stable) load: 'Dev Tests'.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo

Comment #7 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

Which, BTW is:

getListElementSelector: aSymbol
        "specify a selector that can be used to obtain a single element in the  
underlying list"
        getListElementSelector := aSymbol.
        list := nil.  "this cache will not be updated if getListElementSelector  
has been specified, so go ahead and remove it"


Just read the second comment :P


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo

Comment #8 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

Argh I did not see the video. I will rollback the changes!!!!!
Argh argh agrh


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo

Comment #9 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

@guillermo, ok I will fix that


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo
Updates:
        Status: Workneeded

Comment #10 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo
Updates:
        Status: FixReviewNeeded

Comment #11 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

Where can be found ConfigurationOfOmniBrowser ?

But by the way, I wrote a fix (mixed with 5208 optimizations) which simply  
check if getListElementSelector is defined or not.

I randomly load OB packages til having a browser, and it seems to work with  
this fix :)


So you can integrate this one, and forget about:
- roll back
- Issue 5208



Attachments:
        FixOptimizationAndGetListElement.1.cs  4.9 KB


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo

Comment #12 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

Thanks ben so I can integrate it now?


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5183 in pharo: Problem with PluggableListMorph

pharo
Updates:
        Status: Closed

Comment #13 on issue 5183 by [hidden email]: Problem with  
PluggableListMorph
http://code.google.com/p/pharo/issues/detail?id=5183

in 14291


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker