I think Issue482 requires a post load doit when loaded into an existing Seaside3.0.x installation. The contents of the table class instance variable were changed to effect the speedup, but if you are updating an existing installation the class initialize methods won't be re-executed automatically, so the new code will be running with the old table contents.... Now it's true that I've seen the tests pass when run in an updated issue, but I am wondering if there might be trouble lurking somewhere... A naive comparison of the WAEncoder code in 3.0.3: encoded isString ifTrue: [ stream nextPutAll: encoded ] ifFalse: [ stream nextPut: encoded ] and in 3.0.4: encoded notNil ifTrue: [ stream nextPutAll: encoded ] ifFalse: [ stream nextPut: encoded ] makes me think that if the encode variable is a Character then it will be passed into the #nextPutAll:, which is expecting a String ... now it is possible that the Pharo implementation doesn't care whether the arg to nextPutAll: is a Character or a String, or maybe I'm missing something ... Anyway, I guess this is a long winded way of saying that I think an explicit post load doit is _required_ that explicitly rebuilds the tables with new contents: WAUrlEncoder initialize. WAXmlEncoder initialize. I expect to add this to the Metacello config. I ran into this in GemStone, because we have a primitive that encodes the string based on the encoding table and of course the primitive choked when the table contents changed. This happens to be one of the reasons that I've branched Seaside-Core for GemStone ... I don't expect to be able to eliminate all of the places where I've found it necessary to create a GemStone branch of the common Seaside code, but perhaps this _is_ an instance where some creative application of Grease would improve the portability? Dale _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
2011/2/15 Dale Henrichs <[hidden email]>:
> > I think Issue482 requires a post load doit when loaded into an existing > Seaside3.0.x installation. > > The contents of the table class instance variable were changed to effect the > speedup, but if you are updating an existing installation the class > initialize methods won't be re-executed automatically, so the new code will > be running with the old table contents.... > > Now it's true that I've seen the tests pass when run in an updated issue, > but I am wondering if there might be trouble lurking somewhere... > > A naive comparison of the WAEncoder code in 3.0.3: > > encoded isString > ifTrue: [ stream nextPutAll: encoded ] > ifFalse: [ stream nextPut: encoded ] > > and in 3.0.4: > > encoded notNil > ifTrue: [ stream nextPutAll: encoded ] > ifFalse: [ stream nextPut: encoded ] > > makes me think that if the encode variable is a Character then it will be > passed into the #nextPutAll:, which is expecting a String ... now it is > possible that the Pharo implementation doesn't care whether the arg to > nextPutAll: is a Character or a String, or maybe I'm missing something ... > > Anyway, I guess this is a long winded way of saying that I think an explicit > post load doit is _required_ that explicitly rebuilds the tables with new > contents: > > WAUrlEncoder initialize. > WAXmlEncoder initialize. > > I expect to add this to the Metacello config. Hmm. I changed the class side #initialize methods of WAUrlEncoder and WAXmlEncoder in the fix. The way I understand how Monticello works is that in this case it should send these messages to the class again. At least on Pharo/Squeak. Cheers Philippe _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
On 02/15/2011 10:01 PM, Philippe Marschall wrote:
> 2011/2/15 Dale Henrichs<[hidden email]>: >> >> I think Issue482 requires a post load doit when loaded into an existing >> Seaside3.0.x installation. >> >> The contents of the table class instance variable were changed to effect the >> speedup, but if you are updating an existing installation the class >> initialize methods won't be re-executed automatically, so the new code will >> be running with the old table contents.... >> >> Now it's true that I've seen the tests pass when run in an updated issue, >> but I am wondering if there might be trouble lurking somewhere... >> >> A naive comparison of the WAEncoder code in 3.0.3: >> >> encoded isString >> ifTrue: [ stream nextPutAll: encoded ] >> ifFalse: [ stream nextPut: encoded ] >> >> and in 3.0.4: >> >> encoded notNil >> ifTrue: [ stream nextPutAll: encoded ] >> ifFalse: [ stream nextPut: encoded ] >> >> makes me think that if the encode variable is a Character then it will be >> passed into the #nextPutAll:, which is expecting a String ... now it is >> possible that the Pharo implementation doesn't care whether the arg to >> nextPutAll: is a Character or a String, or maybe I'm missing something ... >> >> Anyway, I guess this is a long winded way of saying that I think an explicit >> post load doit is _required_ that explicitly rebuilds the tables with new >> contents: >> >> WAUrlEncoder initialize. >> WAXmlEncoder initialize. >> >> I expect to add this to the Metacello config. > > Hmm. I changed the class side #initialize methods of WAUrlEncoder and > WAXmlEncoder in the fix. The way I understand how Monticello works is > that in this case it should send these messages to the class again. At > least on Pharo/Squeak. Ahh, you are correct! Sorry about that. It works that way in GemStone, too....the primitive was blowing up precisely because the new initialize methods _had_ been run... As Emily Littella used to say: "Never mind" Dale _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
Free forum by Nabble | Edit this page |