- Pharo-dev - lists.pharo.org

Re: [Pharo-dev] Generate accessors refactoring

by Nicolai Hess

2016-12-07 10:44 GMT+01:00 Denis Kudriashov <dionisiydk(a)gmail.com>: > > 2016-12-06 19:29 GMT+01:00 John Brant <brant(a)refactoryworkers.com>: > >> > We could make it configurable. "Manually" accessors will be generated >> in simplified mode and related refactorings will use intelligent mode >> >> My personal preference would be to ask the user what to do when >> performing the accessor refactoring. If the class hierarchy already has a >> method with the name of the variable, we can look at that method and >> determine what to do. In the case of #name, we could see that it is defined >> in a superclass and ask if it is ok to override the method (likely a good >> choice, but not always so it is good to confirm with the user). If the >> class directly defines the method, and that method has a return that >> returns the variable, then we can ask if they want to use that method as >> the âaccessorâ even though it does some other stuff (and potential returns >> a different value). Finally, if some subclass defines the method, then we >> can ask if they want to create that method even though some subclass is >> overriding it. Of course, we could have any combination of all three too. >> If they want a different method, then we should ask what to name the method >> instead of appending a number to the name. > > > Ok. You prefer tons of questions to user just to perform stupid accessors > generation. And, please, don't say that it is rare cases. It is quite > common which nicely shown here 18880 > <https://pharo.fogbugz.com/f/cases/18880/Autogenerating-accessors-should-be-…> > . > I wonder do you have *real* experience when it protected you from > destroying system? We only talk about simple accessors refactoring. > Possible solution in inbox for case 18880. Needs a test case. As far as I can see, this change does not conflict with application of abstract-variable-refactoring, but needs some testing (exising RB-Tests are green).

Dec. 7, 2016

[pharo-project/pharo-core]

by GitHub

Branch: refs/tags/60319 Home: https://github.com/pharo-project/pharo-core

Dec. 7, 2016

[pharo-project/pharo-core] 44e997: 60319

by GitHub

Branch: refs/heads/6.0 Home: https://github.com/pharo-project/pharo-core Commit: 44e9977b19d5e712cadefd4a3fcdfbf034f5cf9f https://github.com/pharo-project/pharo-core/commit/44e9977b19d5e712cadefd4a… Author: Jenkins Build Server <board(a)pharo-project.org> Date: 2016-12-07 (Wed, 07 Dec 2016) Changed paths: R Glamour-Morphic-Brick.package/extension/RubScrolledTextMorph/instance/cursor.st R Glamour-Morphic-Brick.package/extension/RubScrolledTextMorph/instance/font_.st R Glamour-Morphic-Brick.package/extension/RubScrolledTextMorph/instance/margins_.st M Kernel.package/Delay.class/class/settings/delaySchedulerClass_.st A Kernel.package/ManifestKernel.class/class/code-critics/ruleRBBadMessageRuleV1FalsePositive.st A Morphic-Widgets-Tabs.package/TabBarMorph.class/instance/initialization/addMenuButton.st A Morphic-Widgets-Tabs.package/TabBarMorph.class/instance/initialization/useSortedTabsBy_.st R Morphic-Widgets-Tabs.package/TabBarMorph.class/instance/initialize/addMenuButton.st A Morphic-Widgets-Tabs.package/TabManagerMorph.class/instance/initialization/minExtent.st A Morphic-Widgets-Tabs.package/TabManagerMorph.class/instance/initialization/registerBarActions.st A Morphic-Widgets-Tabs.package/TabManagerMorph.class/instance/initialization/registerShortcuts.st A Morphic-Widgets-Tabs.package/TabManagerMorph.class/instance/initialization/useSortedTabsBy_.st R Morphic-Widgets-Tabs.package/TabManagerMorph.class/instance/initialize/minExtent.st R Morphic-Widgets-Tabs.package/TabManagerMorph.class/instance/initialize/registerBarActions.st R Morphic-Widgets-Tabs.package/TabManagerMorph.class/instance/initialize/registerShortcuts.st M Morphic-Widgets-Tabs.package/TabMorph.class/instance/protocol/close.st A Morphic-Widgets-Tabs.package/TabMorph.class/instance/protocol/okToClose.st A Rubric.package/RubScrolledTextMorph.class/instance/accessing text area/cursor.st A Rubric.package/RubScrolledTextMorph.class/instance/accessing text area/font_.st A Rubric.package/RubScrolledTextMorph.class/instance/accessing text area/margins_.st R ScriptLoader60.package/ScriptLoader.class/instance/pharo - scripts/script60318.st A ScriptLoader60.package/ScriptLoader.class/instance/pharo - scripts/script60319.st R ScriptLoader60.package/ScriptLoader.class/instance/pharo - updates/update60318.st A ScriptLoader60.package/ScriptLoader.class/instance/pharo - updates/update60319.st M ScriptLoader60.package/ScriptLoader.class/instance/public/commentForCurrentUpdate.st Log Message: ----------- 60319 19412 SpecDebugger cannot work without Glamour https://pharo.fogbugz.com/f/cases/19412 19428 Silently ignore loading DelayScheduler setting when its the same as existing https://pharo.fogbugz.com/f/cases/19428 19426 TabsManagerMorph improvements https://pharo.fogbugz.com/f/cases/19426 http://files.pharo.org/image/60/60319.zip

Dec. 7, 2016

Re: [Pharo-dev] Generate accessors refactoring

by Denis Kudriashov

2016-12-06 19:29 GMT+01:00 John Brant <brant(a)refactoryworkers.com>: > > We could make it configurable. "Manually" accessors will be generated in > simplified mode and related refactorings will use intelligent mode > > My personal preference would be to ask the user what to do when performing > the accessor refactoring. If the class hierarchy already has a method with > the name of the variable, we can look at that method and determine what to > do. In the case of #name, we could see that it is defined in a superclass > and ask if it is ok to override the method (likely a good choice, but not > always so it is good to confirm with the user). If the class directly > defines the method, and that method has a return that returns the variable, > then we can ask if they want to use that method as the âaccessorâ even > though it does some other stuff (and potential returns a different value). > Finally, if some subclass defines the method, then we can ask if they want > to create that method even though some subclass is overriding it. Of > course, we could have any combination of all three too. If they want a > different method, then we should ask what to name the method instead of > appending a number to the name. Ok. You prefer tons of questions to user just to perform stupid accessors generation. And, please, don't say that it is rare cases. It is quite common which nicely shown here 18880 <https://pharo.fogbugz.com/f/cases/18880/Autogenerating-accessors-should-be-…> . I wonder do you have *real* experience when it protected you from destroying system? We only talk about simple accessors refactoring.

Dec. 7, 2016

Re: [Pharo-dev] Fogbugz admin - please add Pharo 7 milestone

by stepharong

we will :) On Wed, 07 Dec 2016 06:15:09 +0100, Ben Coman <btc(a)openinworld.com> wrote: > Just taking a browse through open tickets for Pharo 6, and maybe some > can be bumped to Pharo 7 (rather than just "Later"). Could we have a > Fogbugz milestone for it? > > cheers -ben > -- Using Opera's mail client: http://www.opera.com/mail/

Dec. 7, 2016

Fogbugz admin - please add Pharo 7 milestone

by Ben Coman

Just taking a browse through open tickets for Pharo 6, and maybe some can be bumped to Pharo 7 (rather than just "Later"). Could we have a Fogbugz milestone for it? cheers -ben

Dec. 7, 2016

Re: [Pharo-dev] How to report website text bugs?

by Ben Coman

On Thu, Dec 1, 2016 at 2:21 PM, Nicolai Hess <nicolaihess(a)gmail.com> wrote: > Am 01.12.2016 12:30 vorm. schrieb "Ben Coman" <btc(a)openinworld.com>: >> >> On Wed, Nov 30, 2016 at 7:37 PM, Henrik Nergaard <henrik.nergaard(a)uia.no> >> wrote: >> > SystemNavigation new allLocalCallsOn: 'No change' ofClass: ProtoObject . >> > >> > "a Set(Delay class>>#delaySchedulerClass:)" >> >> That is only called from two places: >> * DelayBenchmark>>bench: >> * Delay class >> systemSettingOn: >> so it is curious that #delaySchedulerClass: is being invoked. >> >> > From: Pharo-dev [mailto:pharo-dev-bounces@lists.pharo.org] On Behalf Of >> > Esteban Lorenzano >> > Sent: Wednesday, November 30, 2016 12:14 PM >> > To: Pharo Development List <pharo-dev(a)lists.pharo.org> >> > Subject: Re: [Pharo-dev] How to report website text bugs? >> > >> > >> > On 30 Nov 2016, at 12:09, Esteban Lorenzano <estebanlm(a)gmail.com> wrote: >> > On 30 Nov 2016, at 11:26, Tim Mackinnon <tim(a)testit.works> wrote: >> > >> > I just tried the new example on the website (factorial is a great >> > example >> > from the command line - I like it). >> > >> > The output is: >> > >> > $ ./pharo Pharo.image eval "42 factorial" >> > 'No change' >> > 1405006117752879898543142606244511569936384000000000 >> > >> > Out of curiosity, What is the âNo Changeâ message above the answer? >> > no idea :) >> > obviouslyâ¦ a remaining error :P >> > and btwâ¦ thatâs obviously a transcript message. >> >> actually an #inform: >> >> >> >> >> On Thu, Dec 1, 2016 at 12:14 AM, Sven Van Caekenberghe <sven(a)stfx.eu> >> wrote: >> > Tim, >> > >> >> On 30 Nov 2016, at 11:26, Tim Mackinnon <tim(a)testit.works> wrote: >> >> >> >> I just tried the new example on the website (factorial is a great >> >> example from the command line - I like it). >> >> >> >> The output is: >> >> >> >> $ ./pharo Pharo.image eval "42 factorial" >> >> 'No change' >> >> 1405006117752879898543142606244511569936384000000000 >> >> $ >> >> >> >> Out of curiosity, What is the âNo Changeâ message above the answer? >> > >> > What version are you using, I just tried (on macOS 10.12.1), in a >> > terminal: >> > >> > prometheus:pharo6 sven$ curl get.pharo.org/60+vm | bash >> > % Total % Received % Xferd Average Speed Time Time Time >> > Current >> > Dload Upload Total Spent Left >> > Speed >> > 100 2901 100 2901 0 0 2565 0 0:00:01 0:00:01 --:--:-- >> > 2571 >> > Downloading the latest 60 Image: >> > http://files.pharo.org/get-files/60/pharo.zip >> > Pharo.image >> > Downloading the latest pharoVM: >> > http://files.pharo.org/get-files/60/pharo-mac-stable.zip >> > pharo-vm/Pharo.app/Contents/MacOS/Pharo >> > Downloading PharoV60.sources: >> > http://files.pharo.org/get-files/60/sources.zip >> > Creating starter scripts pharo and pharo-ui >> > >> > prometheus:pharo6 sven$ ./pharo Pharo.image printVersion >> > [version] 6.0 #60311 >> > >> > prometheus:pharo6 sven$ ./pharo Pharo.image eval '42 factorial' >> > 1405006117752879898543142606244511569936384000000000 >> > >> > Works like a charm. I assume it works on Linux too. >> >> I just tried the same with new download of Pharo 5 and it worked >> without outputting 'No change' >> cheers -ben >> > > It depends on your user settings. If and what set of user settings were > exported. And maybe, if there is a set if user settings in the old format. Nice pick up. Better to be less verbose... https://pharo.fogbugz.com/default.asp?19428 cheers -ben

Dec. 7, 2016

Re: [Pharo-dev] Generate accessors refactoring

by Ben Coman

How about mostly keeping the existing behaviour, but default to collisions being deselected? Then at least you don't need to search for it. It minimises the chance of missing it by mistake. The deselected item easily stands out to be reselected if required. P.S. An additional side idea: If collision is from a superclass then append (Superclass>>name) to the list item, and clicking on it shows a code dialog to the right similar to Spotter code review pane, to make a review easier. cheers -ben On Tue, Dec 6, 2016 at 10:09 PM, Yuriy Tymchuk <yuriy.tymchuk(a)me.com> wrote: > It shouldnât be too intelligent. Keep the default behavior, and for each > method add a 3-state checkbox: create (default), skip, force (override). > Also having a usage recorder would be nice to know if developers are > actually checking the other state more often than the default one. > > Uko > > On 6 Dec 2016, at 13:03, Thierry Goubier <thierry.goubier(a)gmail.com> wrote: > > > > 2016-12-06 11:34 GMT+01:00 Denis Kudriashov <dionisiydk(a)gmail.com>: >> >> >> 2016-12-06 11:28 GMT+01:00 Yuriy Tymchuk <yuriy.tymchuk(a)me.com>: >>> >>> Additionally it was annoying if a superclass has a method with the same >>> name. For example if you have a name var and you create accessors Iâd like >>> to have actually a ânameâ getter and not âname1â >> >> >> Yes >> > > If you start to make it so intelligent it requires half a dozen click to > generate a simple accessor, then people will stop using the refactoring and > write the code by hand... > > Thierry > > >

Dec. 6, 2016

Re: [Pharo-dev] Generate accessors refactoring

by stepharong

>> That isnât a refactoring. The existing methods arenât accessors. The >> refactoring will find accessors for the variables if they exist (even >> if they have a different names). >> >> We all know what is refactoring. Usually we don't care about safe >> aspect of it (and we have tests). We just want simple code >> transformation. I think it is exactly such case. > > âWe allâ probably donât agree on much. In fact, I think many people > would argue that the âsafe aspectâ is a very important part of a > refactoring. Otherwise, they shouldnât be called refactorings, but > should instead be called transformations. If you call something a > ârefactoringâ but donât do basic checks on its validity then that is > just wrong. Yes this is why we are working on proposing refactorings and transformations. We are currently refactoring the refactorings to have both transformations and refactorings.

Dec. 6, 2016

Re: [Pharo-dev] Generate accessors refactoring

by John Brant

> On Dec 6, 2016, at 10:53 AM, Denis Kudriashov <dionisiydk(a)gmail.com> wrote: > > > 2016-12-06 17:16 GMT+01:00 John Brant <brant(a)refactoryworkers.com>: > > For me it is too intelligent now because it is tried to create new version of accessors instead of existing one. > > But simple logic should just analyze method name and if accessor already exists it should skip it > > That isnât a refactoring. The existing methods arenât accessors. The refactoring will find accessors for the variables if they exist (even if they have a different names). > > We all know what is refactoring. Usually we don't care about safe aspect of it (and we have tests). We just want simple code transformation. I think it is exactly such case. âWe allâ probably donât agree on much. In fact, I think many people would argue that the âsafe aspectâ is a very important part of a refactoring. Otherwise, they shouldnât be called refactorings, but should instead be called transformations. If you call something a ârefactoringâ but donât do basic checks on its validity then that is just wrong. > Also after applying override browser shows it with special icon. And user could see it. Assuming that you didnât override something that is an essential part of the image. For example, try overriding the class side method #name to return nil and tell me how the special icon works. > > New accessors canât override an existing method, for example #name, since that would change the behavior of the #name method for that object. If you are suggesting not creating the accessor method for the âmyInstVar ^myInstVar ifNil: [#someValue]â method, then youâll need to change the abstract variable refactorings so that they no longer use the modified create accessors refactoring. > > We could make it configurable. "Manually" accessors will be generated in simplified mode and related refactorings will use intelligent mode My personal preference would be to ask the user what to do when performing the accessor refactoring. If the class hierarchy already has a method with the name of the variable, we can look at that method and determine what to do. In the case of #name, we could see that it is defined in a superclass and ask if it is ok to override the method (likely a good choice, but not always so it is good to confirm with the user). If the class directly defines the method, and that method has a return that returns the variable, then we can ask if they want to use that method as the âaccessorâ even though it does some other stuff (and potential returns a different value). Finally, if some subclass defines the method, then we can ask if they want to create that method even though some subclass is overriding it. Of course, we could have any combination of all three too. If they want a different method, then we should ask what to name the method instead of appending a number to the name. Currently, the extract method works similarly. If you enter an existing hierarchy method name, it warns you that you could be changing the behavior and asks if that is what you want to do. By asking the user, we can confirm that is what they really want to do, and also let them know of the potential problems that they may not have been aware of. John Brant

Dec. 6, 2016