[bug] Chunckbrowser goodie doesnt handle rename

Ian -

I've just noticed that the ChunkBrowser goodie doesn't handle rename refactorings
and show them in its list of restorations. I've got a dodgy laptop (sadly
my last one was stolen - most of it backed up but did lose a few Intelli
updates ;-(

Anyway it keeps hanging so I've been having to use your browser quite a bit
to restore stuff and I've be going crazy trying to figure out why it often
fails on restore with missing Targets etc.

Finally I realised that if I rename a class it doesn't appear in the chunk
browser (renaming a method does). However subsequent operation in the new
class do show up as operations in the new class (however restoring those
operations will fail becuase there isn't anything that creates the new class
and removes the old one).

Tim

Tim,

>Finally I realised that if I rename a class it doesn't appear in the chunk
>browser (renaming a method does).

That's right and, unfortunately, there's not a lot I can do about it.

The CB works by going through the change log and reducing it to
individual chunks.  The chunks are inserted into the change log as
part of the normal operation of Dolphin, adding or removing a class,
renaming a method etc.

However the refactoring browser does some things internally and
doesn't use the "normal" Dolphin operations - for example, it doesn't
rename a class by deleting the existing class and then adding the
class back into the image under a new name.  

Because the normal Dolphin methods are bypassed no entries are made
into the change log and, therefore, the CB is unable to replay the
changes.

The only way to get round this would be for the RB to make it's own
entries into the change log and that would be down to OA to implement.
--
Ian

Use the Reply-To address to contact me (limited validity).
Mail sent to the From address is ignored.

Hello Ian,


> Because the normal Dolphin methods are bypassed no entries are made
> into the change log and, therefore, the CB is unable to replay the
> changes.

Actually that doesn't seem to be the case - I see the following in my chg
file:

   Dummy rename: #DummyClass!


So it would seem that there are enough details to do it. As the Chunkbrowser
has got me out of quite a few scrapes - I would love to see it support this.
(It also seems that renaming methods is ok as it does an add and remove that
appears in the chunk browser).

Tim

Tim,

>Actually that doesn't seem to be the case - I see the following in my chg
>file:

ooooh, so it does - that's new (I think) :-)  

It should be pretty easy to update the CB to handle #rename, I wonder
what other additional refactoring operations I might have missed?

I'll update the CB ASAP.

Many thanks for flagging the problem.
--
Ian

Use the Reply-To address to contact me (limited validity).
Mail sent to the From address is ignored.

Tim et al,

>Actually that doesn't seem to be the case - I see the following in my chg
>file:
>
>   Dummy rename: #DummyClass!

I've uploaded a modified CB to my updates page at

http://www.idb.me.uk/files/updates.html


It adds a new chunk type - Class Rename which behaves slightly
differently to other chunk types

- (minor) Browse attempts to open a browser on the new class name
rather than the original class name.  It just seems more "useful"

- The comparison icon shows green if the original class name is
missing from the current image and the new class name is present.
There is no way of telling if this was a result of the rename but it
seemed more useful than just showing the "missing" icon.

One final point.  Restoring the chunk _only_ restores the rename of
the class, references to the old class name are _not_  changed.  This
shouldn't cause too much of a problem as any methods modified because
of the class rename should also be present in the change log,
immediately after the rename chunk.

If you get a chance could you give it a try and see if it solves the
problems you were having.
--
Ian

Use the Reply-To address to contact me (limited validity).
Mail sent to the From address is ignored.

Hi Ian,

Is the update only for D6?

Dave Wachtel
---
--
Dave Wachtel
Software Developer
Membership Chairman/Webmaster
Mohawk Hudson Region, Sports Car Club of America
http://members.acmenet.net/~dwachtel/
http://www.mohud-scca.org

Dave,

>Is the update only for D6?

No, for D6 only.  I had to make some changes to the CB for D6 (e.g. it
has no resource definition chunks) so the current version is notably
different.   I'm not sure if the #rename chunk was present in D5
either, and I haven't got a working installation to have a look..
--
Ian

Use the Reply-To address to contact me (limited validity).
Mail sent to the From address is ignored.

>>Is the update only for D6?
>
>No, for D6 only

I knew what I meant :-)

Start again.

Yes, for D6 only
--
Ian

Use the Reply-To address to contact me (limited validity).
Mail sent to the From address is ignored.

Hello Ian,


> I've uploaded a modified CB to my updates page at
>
> http://www.idb.me.uk/files/updates.html


The rename chunk works! Many thanks for that - like I said before, you've
saved me out of a few tight spots where I was a bit careless...

I have noticed something that the older version did better however. I have
a chunck where I put an instance variable outside of the instanceVariableNames
string by mistake (see below):

GeneralRoundRobinArea subclass: #LeadinRoundRobinArea
        instanceVariableNames: ''leadinDevice
        classVariableNames: ''
        poolDictionaries: ''
        classInstanceVariableNames: ''


The next chunk corrects this - however in the old version it seemed to cope
with this, in the new version I get a walkback (Although only when I start
in my out of date image and try bulk reading in lots of stuff. I unselected
that line and everything worked much better - interestingly when I just tried
to hilight that single line and recreate the walkback I don't get it, its
only when I do the bulk load).

The second point - and I think its a bit harder to work around, and possibly
not worth the trouble, is that renaming an instance variable gives you the
error "the following chunks may have failed to restore". I think this is
becuase it does a variable change in the class (which causes the error in
any methods using the variable" and then fixes all the methods using the
variable.  A quick fix might be to change the error msg and append "... possible
variable renames". It might be possible to notice this if the chunk after
the error you detect is a method with a new variable name.

--- walkback ---

11:50:01, 23 July 2006: 'String does not understand #leadinDevice'
String(Object)>>doesNotUnderstand:
UndefinedObject>>{unbound}doIt
Compiler class>>evaluate:for:evaluationPools:logged:ifFail:
Compiler class>>evaluate:for:evaluationPools:logged:
Compiler class>>evaluate:for:logged:
Compiler class>>evaluate:logged:
[] in ChunkBrowserClassDefineChunk(ChunkBrowserExpressionChunk)>>restoreAndLog:
ExceptionHandler(ExceptionHandlerAbstract)>>markAndTry
[] in ExceptionHandler(ExceptionHandlerAbstract)>>try:
BlockClosure>>ifCurtailed:
BlockClosure>>ensure:
ExceptionHandler(ExceptionHandlerAbstract)>>try:
BlockClosure>>on:do:
ChunkBrowserClassDefineChunk(ChunkBrowserExpressionChunk)>>restoreAndLog:
ChunkBrowserClassDefineChunk>>restore
[] in Chunk Browser>>restore:
Array(ArrayedCollection)>>do:
ChunkBrowser>>restore:
ChunkBrowser>>restoreSelection
Symbol>>forwardTo:
CommandDescription>>performAgainst:
[] in Command>>value
BlockClosure>>ifCurtailed:
BlockClosure>>ensure:
Command>>value
ShellView>>performCommand:
ChunkBrowser(Shell)>>performCommand:
CommandQuery>>perform
DelegatingCommandPolicy(CommandPolicy)>>route:
[] in ShellView(View)>>onCommand:
BlockClosure>>ifCurtailed:
BlockClosure>>ensure:
Cursor>>showWhile:
ShellView(View)>>onCommand:
ShellView(View)>>wmCommand:wParam:lParam:
ShellView(View)>>dispatchMessage:wParam:lParam:
[] in InputState>>wndProc:message:wParam:lParam:cookie:
BlockClosure>>ifCurtailed:
ProcessorScheduler>>callback:evaluate:
InputState>>wndProc:message:wParam:lParam:cookie:
ShellView>>translateAccelerator:
ShellView>>preTranslateKeyboardInput:
ShellView(View)>>preTranslateMessage:
InputState>>preTranslateMessage:
InputState>>pumpMessage:
InputState>>loopWhile:
InputState>>mainLoop
[] in InputState>>forkMain
ExceptionHandler(ExceptionHandlerAbstract)>>markAndTry
[] in ExceptionHandler(ExceptionHandlerAbstract)>>try:

Tim,

Thanks again for the feedback.

>I have noticed something that the older version did better however. I have
>a chunck where I put an instance variable outside of the instanceVariableNames
>string by mistake (see below):
[]
>The second point - and I think its a bit harder to work around, and possibly
>not worth the trouble, is that renaming an instance variable gives you the
>error "the following chunks may have failed to restore".

Both of these are a result of changes I made to try and make the error
handling a bit more precise - in hindsight it's probably not a good
idea.

I've reverted to the previous way of doing things (basically, wimp out
at the slightest sniff of an error) and it seems to make things a bit
smoother.  I've uploaded an update to the update (same address and
filename as before) which you might like to try.

Your first example now generates a "may have failed to restore"
warning for the incorrect chunk but it doesn't interrupt the
restoration process for any other selected chunks.

Your second example doesn't generate any errors at all in the CB but
now results in an entry on the Transcript and a warning beep.  It
might be a bit difficult to prevent that so I'll probably leave that
as is.
--
Ian

Use the Reply-To address to contact me (limited validity).
Mail sent to the From address is ignored.

Ian Bartholomew wrote:

> I've reverted to the previous way of doing things (basically, wimp out
> at the slightest sniff of an error) and it seems to make things a bit
> smoother.  I've uploaded an update to the update (same address and
> filename as before) which you might like to try

Ian - I had another crash (its a combination of a suspect laptop and a
some work on sockets) and got to try out another scenario. I have come
accross a few more problems.

I suspect that the ChunkBrowser is not correctly updating category
information (e.g. public/private). When I recover and compare against a
prevoius STS edition, all recovered methods appear as public? (Even
though the chg file seems to write out this information)?

I appear to have a slightly corrupted method in the chg file that looks
like the following:

aString in parseMethod:in: = 'stop
    serverMonitor terminate.
    serverSocket close.!PlcPort categoriesFor: #warehouse:'

I don't know how it happed - I recall this random text appearing in a
method after an image save and I corrected it. Now having crashed and
between my last version I am trying to bring in my changes and I get a
walkback when scrolling through the changes.

I believe the fix might be in isMatch: (it seems to call
parseMethod:in: without an error block - however I couldn't seem to get
a fix there to work).

It may also be you can't correct this (I have editted my chg file and
fixed it that way but though any aditional error cases my be helpful).

Tim

08:05:24, 26 July 2006: 'Error: invalid expression start'

SmalltalkParser>> parserError:range:
parserError:
parsePrimitiveObject
parseUnaryMessage
parseBinaryMessage
parseKeywordMessage
parseCascadeMessage
parseAssignment
parseStatementList:into:
parseStatements:
parseMethod
parseMethod:
parseMethod:in:errorBlock:
SmalltalkParser class>>parseMethod:in:errorBlock:
SmalltalkParser class>>parseMethod:in:onError:
SmalltalkParser class>>parseMethod:in:
ChunkBrowserMethodDefineChunk>> isMatch


ChunkBrowserMethodDefineChunk(ChunkBrowserChunk)>>chunkIconIndex

Message>> forwardTo:
value:
ListViewColumn>> imageFromRow:

ListView>> onDisplayDetailsRequired:


ListView(IconicListAbstract)>>nmGetDispInfo:

ListView>> nmNotify:


ContainerView(View)>>wmNotify:wParam:lParam:
ContainerView(View)>>dispatchMessage:wParam:lParam: [] in
InputState>>wndProc:message:wParam:lParam:cookie:

BlockClosure>> ifCurtailed:

ProcessorScheduler>> callback:evaluate:

InputState>> wndProc:message:wParam:lParam:cookie:

ListView(ControlView)>>defaultWindowProcessing:wParam:lParam:
ListView(ControlView)>>wmPaint:wParam:lParam:
ListView(View)>>dispatchMessage:wParam:lParam:
[] in InputState>>wndProc:message:wParam:lParam:cookie:
BlockClosure>> ifCurtailed:

ProcessorScheduler>> callback:evaluate:

InputState>> wndProc:message:wParam:lParam:cookie:
pumpMessage:
loopWhile:
mainLoop

[] in InputState>>forkMain
ExceptionHandler(ExceptionHandlerAbstract)>>markAndTry [] in
ExceptionHandler(ExceptionHandlerAbstract)>>try:

BlockClosure>> ifCurtailed:
ensure:

ExceptionHandler(ExceptionHandlerAbstract)>>try:

BlockClosure>> on:do:
 

[] in BlockClosure>>newProcess

Tim,

Thanks again for the feedback.

>I suspect that the ChunkBrowser is not correctly updating category
>information (e.g. public/private). When I recover and compare against a
>prevoius STS edition, all recovered methods appear as public? (Even
>though the chg file seems to write out this information)?

Yes, I can see that problem.  It occurs when you try to restore
methods already restored using the CB i.e.

1) You use the CB to restore some methods.
2) The CB restores the methods and categories but doesn't put the new
category information back into the change log
3) You use the CB again to re-restore those incomplete methods saved
in step 2

I've changed the CB so that after updating a category it forces the
change to be saved in the change log.

FWIW, in some circumstances Dolphin itself has the same problem (see a
later post)

>I believe the fix might be in isMatch: (it seems to call
>parseMethod:in: without an error block - however I couldn't seem to get
>a fix there to work).

Yes, the default of passing #nil as the error block doesn't seem to
work.  I've added an #errorBlock and restored the check for
CompilerErrorNotification in the #restore method - between them it
seems (nb seems) to cover all the possibilities.

Updated ClassBrowser package uploaded to the normal web page

http://www.idb.me.uk/files/updates.html
--
Ian

Use the Reply-To address to contact me (limited validity).
Mail sent to the From address is ignored.

Hello Ian,

>
> Thanks again for the feedback.
>

No way - thank you, its definitely a labor of love you have going on, but
its certainly saved me some effor in manually doing from the chg log...  
You're in the UK right? Maybe one day we should all get together and do a
little mini dolphin conference and buy some well deserved beers... I certainly
owe a fair number.

Tim