Keep methods package when changing an existing method.
authorStefan Vogel <sv@exept.de>
Fri, 08 Apr 2005 19:23:02 +0200
changeset 6243 ec3b7f54d806
parent 6242 53a88bc35aa5
child 6244 548f6707f59c
Keep methods package when changing an existing method. Fix warning about accepting an already existing method.
NewSystemBrowser.st
Tools__NewSystemBrowser.st
--- a/NewSystemBrowser.st	Fri Apr 08 11:32:20 2005 +0200
+++ b/NewSystemBrowser.st	Fri Apr 08 19:23:02 2005 +0200
@@ -35486,13 +35486,14 @@
     "accept a new method.
      Return false, if NOT accepted (i.e. compilation canceled)"
 
-    |code codeCritics cat rslt returnValue newSelector|
+    |code cat returnValue newSelector existingMethod|
 
     code := codeArg.
     returnValue := false.
 
     "/ a quick parse for the selector ...
     newSelector := self selectorOfMethodFromCode:code in:cls.
+    existingMethod := cls compiledMethodAt:newSelector ifAbsent:[].
     cat := self protocolToAcceptMethod:newSelector class:cls.
 
     AbortOperationRequest handle:[:ex |
@@ -35503,21 +35504,17 @@
 
             answer := SystemBrowser askForPackageChangeFrom:ex oldPackage 
                                                          to:ex newPackage.
-
             (answer ~~ #cancel) ifTrue:[
                 ex proceedWith:answer
             ].
         ] do:[
-            |codeView package prevVersionMethod parser oldSelector change defPackage
-             originalSource changedSource v answer| 
+            |codeView package oldSelector defPackage answer rslt| 
 
             oldSelector := self theSingleSelectedSelector.
 
-            (parser notNil and:[parser ~~ #Error]) ifTrue:[
-                "/ check for overwritten version method
-
-                (cls isMeta and:[newSelector = 'version']) ifTrue:[
-                    (self confirm:'ATTENTION: you are about to accept the classes version method.
+            "/ check for overwritten version method
+            (cls isMeta and:[newSelector = #version]) ifTrue:[
+                (self confirm:'ATTENTION: you are about to accept the classes version method.
 This method is required by the sourceCodeManager and should correctly return
 the classes version as present in the source repository.
 An incorrect version method may lead to failures when accessing/showing/changing
@@ -35525,39 +35522,29 @@
 You have been warned.
 
 Accept anyway ?')
-                    ifFalse:[
+                ifFalse:[
+                    ^ false
+                ]
+            ] ifFalse:[
+                "/ check if accepting a different selector than the selected one,
+                "/ and a method for the new selector exists.
+                (existingMethod notNil and:[oldSelector ~= newSelector]) ifTrue:[
+                    answer := OptionBox 
+                                  request:('You are about to overwrite an existing method.\\Accept anyway ?' withCRs)
+                                  label:(resources string:'Attention')
+                                  image:(WarningBox iconBitmap)
+                                  buttonLabels:(resources array:#('Cancel' 'Compare' 'Yes'))
+                                  values:#(false #compare true)
+                                  default:false
+                                  onCancel:false.
+
+                    answer == false ifTrue:[
                         ^ false
-                    ]
-                ] ifFalse:[
-                    "/ check if accepting a different selector than the selected one,
-                    "/ and a method for the new selector exists.
-                    (oldSelector ~= newSelector) ifTrue:[
-                        (cls includesSelector:newSelector) ifTrue:[
-                            answer := OptionBox 
-                                          request:('You are about to overwrite an existing method.\\Accept anyway ?' withCRs)
-                                          label:(resources string:'Attention')
-                                          image:(WarningBox iconBitmap)
-                                          buttonLabels:(resources array:#('Cancel' 'Compare' 'Yes'))
-                                          values:#(false #compare true)
-                                          default:false
-                                          onCancel:false.
-
-"/                                (self confirm:'You are about to overwrite an existing method.
-"/
-"/Accept anyway ?')
-"/                                ifFalse:[
-"/                                    ^ false
-"/                                ].
-
-                            answer == false ifTrue:[
-                                ^ false
-                            ].
-                            answer == #compare ifTrue:[
-                                self openDiffViewForText:code againstSourceOfMethod:(cls compiledMethodAt:newSelector).
-                                ^ false
-                            ].
-                        ]
-                    ]
+                    ].
+                    answer == #compare ifTrue:[
+                        self openDiffViewForText:code againstSourceOfMethod:existingMethod.
+                        ^ false
+                    ].
                 ]
             ].
 
@@ -35565,42 +35552,46 @@
             codeView cursorMovementWhenUpdating:nil.
             codeView scrollWhenUpdating:nil.
 
-            defPackage := Class packageQuerySignal query.
-
-            "/ if in project-mode,
-            "/ assign the currently selected packageID (or ask, if there is none or multiple)
-            "/ otherwise, use the current project
-
-            (navigationState isProjectBrowser
-            or:[navigationState organizerMode value == #project])
-            ifTrue:[
-                package := self theSingleSelectedProject.
-                package notNil ifTrue:[package := package asSymbol].
+            existingMethod notNil ifTrue:[
+                "keep old package if selector does already exist in class"
+                package := existingMethod package.
+            ] ifFalse:[
+                defPackage := Class packageQuerySignal query.
+
+                "/ if in project-mode,
+                "/ assign the currently selected packageID (or ask, if there is none or multiple)
+                "/ otherwise, use the current project
+
+                (navigationState isProjectBrowser
+                or:[navigationState organizerMode value == #project])
+                ifTrue:[
+                    package := self theSingleSelectedProject.
+                    package isNil ifTrue:[
+                        package := self 
+                                        askForProject:'Method shall be assigned to which project ?'
+                                        initialText:(LastAcceptPackage ? cls package).
+                        package isNil ifTrue:[^ false].
+                        LastAcceptPackage := package.
+                    ] ifFalse:[
+                        package := package asSymbol.
+                        "/ if the current project is different from the selected one
+                        package ~= defPackage ifTrue:[
+                            "/ and the current project is not the default project
+                            (defPackage = Project defaultProject package) ifFalse:[
+                                "/ ask
+                                package := self 
+                                                askForProject:('The browsers selected project is ''%1''\however, your currently active (default) project is ''%2''.\\To which project shall the method be assigned ?'
+                                                               bindWith:package allBold with:defPackage allBold) withCRs
+                                                initialText:package.
+                                package isNil ifTrue:[^ false].
+                                LastAcceptPackage := package.
+                            ]
+                        ]
+                    ].
+                ].
                 package isNil ifTrue:[
-                    package := self 
-                                    askForProject:'Method shall be assigned to which project ?'
-                                    initialText:(LastAcceptPackage ? cls package).
-                    package isNil ifTrue:[^ false].
-                    LastAcceptPackage := package.
-                ] ifFalse:[
-                    "/ if the current project is different from the selected one
-                    package ~= defPackage ifTrue:[
-                        "/ and the current project is not the default project
-                        (defPackage = Project defaultProject package) ifFalse:[
-                            "/ ask
-                            package := self 
-                                            askForProject:('The browsers selected project is ''%1''\however, your currently active (default) project is ''%2''.\\To which project shall the method be assigned ?'
-                                                           bindWith:package allBold with:defPackage allBold) withCRs
-                                            initialText:package.
-                            package isNil ifTrue:[^ false].
-                            LastAcceptPackage := package.
-                        ]
-                    ]
-                ].
-            ].
-
-            package isNil ifTrue:[
-                package := defPackage
+                    package := defPackage
+                ].
             ].
 
             "/ notice: when compiling, the classes change message will already
@@ -35608,6 +35599,8 @@
             "/ to be enqueued.
 
             [
+                |codeCritics|
+
                 code := code asString.
 
                 self enforceCodeStyle ifTrue:[
@@ -35627,10 +35620,9 @@
                 "/ self immediateUpdate value:true.
 
                 "/ Transcript showCR:'accepting in package: ', (package ? '__NoPackage__').
-                Class packageQuerySignal answer:package
-                do:[
+                Class packageQuerySignal answer:package do:[ |change|
                     (self canUseRefactoringSupport) ifTrue:[
-                        change := InteractiveAddMethodChange compile:code asString in:cls classified:cat.
+                        change := InteractiveAddMethodChange compile:code in:cls classified:cat.
                         change controller:codeView.
                         "/ change named:('Accept method ' , newSelector ? '???').
 
@@ -35638,7 +35630,7 @@
                         rslt := cls compiledMethodAt:newSelector.
                     ] ifFalse:[
                         rslt := cls compilerClass 
-                            compile:code asString
+                            compile:code
                             forClass:cls
                             inCategory:cat 
                             notifying:codeView
@@ -35739,18 +35731,13 @@
 !
 
 checkCodeQuality:code
-    |col|
-
     code asCollectionOfLines keysAndValuesDo:[:lineNr :eachLine |
-        |lineString|
+        |lineString column|
 
         lineString := eachLine string.
         (lineString withoutLeadingSeparators startsWith:'^') ifTrue:[
-            col := lineString indexOf:$^.
-"/            col <= 4 ifTrue:[
-"/                ^ (lineNr -> 'bad indentation').
-"/            ].
-            (col-1) \\ 4 ~~ 0 ifTrue:[
+            column := lineString indexOf:$^.
+            (column-1) \\ 4 ~~ 0 ifTrue:[
                 ^ (lineNr -> 'bad indentation').
             ].
         ]
@@ -36487,7 +36474,7 @@
 !NewSystemBrowser class methodsFor:'documentation'!
 
 version
-    ^ '$Header: /cvs/stx/stx/libtool/Attic/NewSystemBrowser.st,v 1.814 2005-03-24 15:33:47 cg Exp $'
+    ^ '$Header: /cvs/stx/stx/libtool/Attic/NewSystemBrowser.st,v 1.815 2005-04-08 17:23:02 stefan Exp $'
 ! !
 
 NewSystemBrowser initialize!
--- a/Tools__NewSystemBrowser.st	Fri Apr 08 11:32:20 2005 +0200
+++ b/Tools__NewSystemBrowser.st	Fri Apr 08 19:23:02 2005 +0200
@@ -35486,13 +35486,14 @@
     "accept a new method.
      Return false, if NOT accepted (i.e. compilation canceled)"
 
-    |code codeCritics cat rslt returnValue newSelector|
+    |code cat returnValue newSelector existingMethod|
 
     code := codeArg.
     returnValue := false.
 
     "/ a quick parse for the selector ...
     newSelector := self selectorOfMethodFromCode:code in:cls.
+    existingMethod := cls compiledMethodAt:newSelector ifAbsent:[].
     cat := self protocolToAcceptMethod:newSelector class:cls.
 
     AbortOperationRequest handle:[:ex |
@@ -35503,21 +35504,17 @@
 
             answer := SystemBrowser askForPackageChangeFrom:ex oldPackage 
                                                          to:ex newPackage.
-
             (answer ~~ #cancel) ifTrue:[
                 ex proceedWith:answer
             ].
         ] do:[
-            |codeView package prevVersionMethod parser oldSelector change defPackage
-             originalSource changedSource v answer| 
+            |codeView package oldSelector defPackage answer rslt| 
 
             oldSelector := self theSingleSelectedSelector.
 
-            (parser notNil and:[parser ~~ #Error]) ifTrue:[
-                "/ check for overwritten version method
-
-                (cls isMeta and:[newSelector = 'version']) ifTrue:[
-                    (self confirm:'ATTENTION: you are about to accept the classes version method.
+            "/ check for overwritten version method
+            (cls isMeta and:[newSelector = #version]) ifTrue:[
+                (self confirm:'ATTENTION: you are about to accept the classes version method.
 This method is required by the sourceCodeManager and should correctly return
 the classes version as present in the source repository.
 An incorrect version method may lead to failures when accessing/showing/changing
@@ -35525,39 +35522,29 @@
 You have been warned.
 
 Accept anyway ?')
-                    ifFalse:[
+                ifFalse:[
+                    ^ false
+                ]
+            ] ifFalse:[
+                "/ check if accepting a different selector than the selected one,
+                "/ and a method for the new selector exists.
+                (existingMethod notNil and:[oldSelector ~= newSelector]) ifTrue:[
+                    answer := OptionBox 
+                                  request:('You are about to overwrite an existing method.\\Accept anyway ?' withCRs)
+                                  label:(resources string:'Attention')
+                                  image:(WarningBox iconBitmap)
+                                  buttonLabels:(resources array:#('Cancel' 'Compare' 'Yes'))
+                                  values:#(false #compare true)
+                                  default:false
+                                  onCancel:false.
+
+                    answer == false ifTrue:[
                         ^ false
-                    ]
-                ] ifFalse:[
-                    "/ check if accepting a different selector than the selected one,
-                    "/ and a method for the new selector exists.
-                    (oldSelector ~= newSelector) ifTrue:[
-                        (cls includesSelector:newSelector) ifTrue:[
-                            answer := OptionBox 
-                                          request:('You are about to overwrite an existing method.\\Accept anyway ?' withCRs)
-                                          label:(resources string:'Attention')
-                                          image:(WarningBox iconBitmap)
-                                          buttonLabels:(resources array:#('Cancel' 'Compare' 'Yes'))
-                                          values:#(false #compare true)
-                                          default:false
-                                          onCancel:false.
-
-"/                                (self confirm:'You are about to overwrite an existing method.
-"/
-"/Accept anyway ?')
-"/                                ifFalse:[
-"/                                    ^ false
-"/                                ].
-
-                            answer == false ifTrue:[
-                                ^ false
-                            ].
-                            answer == #compare ifTrue:[
-                                self openDiffViewForText:code againstSourceOfMethod:(cls compiledMethodAt:newSelector).
-                                ^ false
-                            ].
-                        ]
-                    ]
+                    ].
+                    answer == #compare ifTrue:[
+                        self openDiffViewForText:code againstSourceOfMethod:existingMethod.
+                        ^ false
+                    ].
                 ]
             ].
 
@@ -35565,42 +35552,46 @@
             codeView cursorMovementWhenUpdating:nil.
             codeView scrollWhenUpdating:nil.
 
-            defPackage := Class packageQuerySignal query.
-
-            "/ if in project-mode,
-            "/ assign the currently selected packageID (or ask, if there is none or multiple)
-            "/ otherwise, use the current project
-
-            (navigationState isProjectBrowser
-            or:[navigationState organizerMode value == #project])
-            ifTrue:[
-                package := self theSingleSelectedProject.
-                package notNil ifTrue:[package := package asSymbol].
+            existingMethod notNil ifTrue:[
+                "keep old package if selector does already exist in class"
+                package := existingMethod package.
+            ] ifFalse:[
+                defPackage := Class packageQuerySignal query.
+
+                "/ if in project-mode,
+                "/ assign the currently selected packageID (or ask, if there is none or multiple)
+                "/ otherwise, use the current project
+
+                (navigationState isProjectBrowser
+                or:[navigationState organizerMode value == #project])
+                ifTrue:[
+                    package := self theSingleSelectedProject.
+                    package isNil ifTrue:[
+                        package := self 
+                                        askForProject:'Method shall be assigned to which project ?'
+                                        initialText:(LastAcceptPackage ? cls package).
+                        package isNil ifTrue:[^ false].
+                        LastAcceptPackage := package.
+                    ] ifFalse:[
+                        package := package asSymbol.
+                        "/ if the current project is different from the selected one
+                        package ~= defPackage ifTrue:[
+                            "/ and the current project is not the default project
+                            (defPackage = Project defaultProject package) ifFalse:[
+                                "/ ask
+                                package := self 
+                                                askForProject:('The browsers selected project is ''%1''\however, your currently active (default) project is ''%2''.\\To which project shall the method be assigned ?'
+                                                               bindWith:package allBold with:defPackage allBold) withCRs
+                                                initialText:package.
+                                package isNil ifTrue:[^ false].
+                                LastAcceptPackage := package.
+                            ]
+                        ]
+                    ].
+                ].
                 package isNil ifTrue:[
-                    package := self 
-                                    askForProject:'Method shall be assigned to which project ?'
-                                    initialText:(LastAcceptPackage ? cls package).
-                    package isNil ifTrue:[^ false].
-                    LastAcceptPackage := package.
-                ] ifFalse:[
-                    "/ if the current project is different from the selected one
-                    package ~= defPackage ifTrue:[
-                        "/ and the current project is not the default project
-                        (defPackage = Project defaultProject package) ifFalse:[
-                            "/ ask
-                            package := self 
-                                            askForProject:('The browsers selected project is ''%1''\however, your currently active (default) project is ''%2''.\\To which project shall the method be assigned ?'
-                                                           bindWith:package allBold with:defPackage allBold) withCRs
-                                            initialText:package.
-                            package isNil ifTrue:[^ false].
-                            LastAcceptPackage := package.
-                        ]
-                    ]
-                ].
-            ].
-
-            package isNil ifTrue:[
-                package := defPackage
+                    package := defPackage
+                ].
             ].
 
             "/ notice: when compiling, the classes change message will already
@@ -35608,6 +35599,8 @@
             "/ to be enqueued.
 
             [
+                |codeCritics|
+
                 code := code asString.
 
                 self enforceCodeStyle ifTrue:[
@@ -35627,10 +35620,9 @@
                 "/ self immediateUpdate value:true.
 
                 "/ Transcript showCR:'accepting in package: ', (package ? '__NoPackage__').
-                Class packageQuerySignal answer:package
-                do:[
+                Class packageQuerySignal answer:package do:[ |change|
                     (self canUseRefactoringSupport) ifTrue:[
-                        change := InteractiveAddMethodChange compile:code asString in:cls classified:cat.
+                        change := InteractiveAddMethodChange compile:code in:cls classified:cat.
                         change controller:codeView.
                         "/ change named:('Accept method ' , newSelector ? '???').
 
@@ -35638,7 +35630,7 @@
                         rslt := cls compiledMethodAt:newSelector.
                     ] ifFalse:[
                         rslt := cls compilerClass 
-                            compile:code asString
+                            compile:code
                             forClass:cls
                             inCategory:cat 
                             notifying:codeView
@@ -35739,18 +35731,13 @@
 !
 
 checkCodeQuality:code
-    |col|
-
     code asCollectionOfLines keysAndValuesDo:[:lineNr :eachLine |
-        |lineString|
+        |lineString column|
 
         lineString := eachLine string.
         (lineString withoutLeadingSeparators startsWith:'^') ifTrue:[
-            col := lineString indexOf:$^.
-"/            col <= 4 ifTrue:[
-"/                ^ (lineNr -> 'bad indentation').
-"/            ].
-            (col-1) \\ 4 ~~ 0 ifTrue:[
+            column := lineString indexOf:$^.
+            (column-1) \\ 4 ~~ 0 ifTrue:[
                 ^ (lineNr -> 'bad indentation').
             ].
         ]
@@ -36487,7 +36474,7 @@
 !NewSystemBrowser class methodsFor:'documentation'!
 
 version
-    ^ '$Header: /cvs/stx/stx/libtool/Tools__NewSystemBrowser.st,v 1.814 2005-03-24 15:33:47 cg Exp $'
+    ^ '$Header: /cvs/stx/stx/libtool/Tools__NewSystemBrowser.st,v 1.815 2005-04-08 17:23:02 stefan Exp $'
 ! !
 
 NewSystemBrowser initialize!