Can someone review my fix?
https://pharo.fogbugz.com/default.asp?13063& Stephan _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
Stupid question maybe, but how can we see the changes?
Johan > On 21 Mar 2014, at 11:44, Stephan Eggermont <[hidden email]> wrote: > > Can someone review my fix? > > https://pharo.fogbugz.com/default.asp?13063& > > Stephan > _______________________________________________ > seaside-dev mailing list > [hidden email] > http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
In reply to this post by Stephan Eggermont-3
Johan wrote:
>Stupid question maybe, but how can we see the changes? Not so stupid, it is a platform specific fix in a somewhat seaside related part. The slice is in the Pharo30 inbox. So, preferably: - download a latest vm from ci - download a latest 30 image (with seaside loaded, or load seaside in a plain image) - open monticello - open the pharo30 inbox repository - type 13063 in the package filter - select merge to see the change - merge it, and try Stephan _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
Ok, your fix seems correct but...
wtf? why is there an explicit check for WARenderCanvas necessary in the NEC package ? That has nothing to do with your fix, of course. Unless there is a better way to specialize this implementation and include the code in the Pharo-Development package of Seaside? Johan On 21 Mar 2014, at 13:56, Stephan Eggermont <[hidden email]> wrote: > Johan wrote: >> Stupid question maybe, but how can we see the changes? > > Not so stupid, it is a platform specific fix in a somewhat seaside > related part. > > The slice is in the Pharo30 inbox. > > So, preferably: > - download a latest vm from ci > - download a latest 30 image (with seaside loaded, or load seaside in a plain image) > - open monticello > - open the pharo30 inbox repository > - type 13063 in the package filter > - select merge to see the change > - merge it, and try > > Stephan _______________________________________________ > seaside-dev mailing list > [hidden email] > http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
Hi Stephan,
I don't know if now is the time for it, but we might want to take the opportunity to improve the code of NEC on this. I have the impression the check for the presence of WAHtmlCanvas|WARenderCanvas should actually occur in NECVarTypeGuesser>>getClassFromTypeSuggestingName: Next, frameworks (like Seaside) would need a place to plugin such type-guessing improvements. For that we could add a dictionary to NECVarTypeGuesser where frameworks can register a 'varname->class' mapping. Afaik, a small change that is worthwhile for future code changes and a chance for frameworks to improve the code completion (I guess this is used for code completion? ) But, as I said, we can also take this to Pharo4 if necessary... thanks for catching the bug ;-) Johan On 21 Mar 2014, at 20:54, Johan Brichau <[hidden email]> wrote: > Ok, your fix seems correct but... > > wtf? why is there an explicit check for WARenderCanvas necessary in the NEC package ? > > That has nothing to do with your fix, of course. > Unless there is a better way to specialize this implementation and include the code in the Pharo-Development package of Seaside? > > Johan > > On 21 Mar 2014, at 13:56, Stephan Eggermont <[hidden email]> wrote: > >> Johan wrote: >>> Stupid question maybe, but how can we see the changes? >> >> Not so stupid, it is a platform specific fix in a somewhat seaside >> related part. >> >> The slice is in the Pharo30 inbox. >> >> So, preferably: >> - download a latest vm from ci >> - download a latest 30 image (with seaside loaded, or load seaside in a plain image) >> - open monticello >> - open the pharo30 inbox repository >> - type 13063 in the package filter >> - select merge to see the change >> - merge it, and try >> >> Stephan _______________________________________________ >> seaside-dev mailing list >> [hidden email] >> http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev > _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
In reply to this post by Stephan Eggermont-3
Johan wrote:
>I don't know if now is the time for it, but we might want to take the opportunity to improve the code of NEC on this. Good idea, and I think this is probably the patch that should be in Pharo 3. I hadn’t looked deeper into the problem, just eliminated class references. >I have the impression the check for the presence of WAHtmlCanvas|WARenderCanvas should actually occur in >NECVarTypeGuesser>>getClassFromTypeSuggestingName: Yes, I agree. And taking a look at some Seaside code, canvas seems to be an alias for html too. >Next, frameworks (like Seaside) would need a place to plugin such type-guessing improvements. For that we could add a >dictionary to NECVarTypeGuesser where frameworks can register a 'varname->class' mapping. That would allow simple mappings from html and canvas to WAHtmlCanvas, yes. Would something more complex be needed to guess based on the receiver? >Afaik, a small change that is worthwhile for future code changes and a chance for frameworks to improve the code >completion (I guess this is used for code completion? ) I guess so too... >But, as I said, we can also take this to Pharo4 if necessary… AFAIK it’s code most recently handled by Esteban and Marcus. They are on the critical path to releasing Pharo 3, and this is a design improvement that needs a bit documentation. On the other hand it has a decent set of tests, so we can just resolve 13063 and open a new issue to do the design improvement. >thanks for catching the bug ;-) Yeah, nice one. Stephan _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
I continued working on the plugins and extension menus I noticed that there were no NautiliusIcons anymore, but Smalltalk ui icons.
All of these plugin and extensions mechanisms are quite powerful :-) Phil On Fri, Mar 21, 2014 at 10:59 PM, Stephan Eggermont <[hidden email]> wrote: Johan wrote: _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
Free forum by Nabble | Edit this page |