[Cuis-dev] Temporary to Instance Variable refactor

Tue May 28 19:53:11 PDT 2019

Another fix: the pipes weren't being removed when removing the last
temporary within a block. Thanks to Hernán for pointing that out to me.
Plus, there was a bug when removing a temporary from inside a block while
there was only one temporary declared in the method, and this ChangeSet
fixes that too.

El mar., 28 may. 2019 a las 16:44, Hernan Wilkinson (<
hernan.wilkinson at 10pines.com>) escribió:

> Cool! It works great!
> On github now.
>
> Thank you.
> Hernan.
>
> On Tue, May 28, 2019 at 12:21 AM Eric Brandwein <brandweineric at gmail.com>
> wrote:
>
>> I noticed that the refactor doesn't work when moving a variable declared
>> inside a block while another block declares it, and that the resulting
>> error isn't very descriptive. I've added a better error message for this
>> case, and the corresponding test.
>>
>> El dom., 26 may. 2019 a las 16:00, Eric Brandwein (<
>> brandweineric at gmail.com>) escribió:
>>
>>> Hi all,
>>>
>>> Here's a ChangeSet that fixes the things Hernán mentioned about showing
>>> references to the variable in other methods on the error message, and about
>>> removing the empty pipes when there are no temporary variables left on the
>>> method, along with the corresponding tests.
>>>
>>> Cheers,
>>> Eric
>>>
>>> El lun., 20 may. 2019 a las 17:13, Hernan Wilkinson (<
>>> hernan.wilkinson at 10pines.com>) escribió:
>>>
>>>> 👍👍
>>>>
>>>> On Mon, May 20, 2019 at 5:02 PM Eric Brandwein <brandweineric at gmail.com>
>>>> wrote:
>>>>
>>>>>
>>>>>
>>>>> El lun., 20 may. 2019 a las 8:10, Hernan Wilkinson (<
>>>>> hernan.wilkinson at 10pines.com>) escribió:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Sun, May 19, 2019 at 9:26 PM Eric Brandwein <
>>>>>> brandweineric at gmail.com> wrote:
>>>>>>
>>>>>>> Thanks Hernán! I'll check your formatting changes so I can apply
>>>>>>> them in future contributions. Is there a 'Smalltalk formatting guidelines'
>>>>>>> or something that I could read to learn them? Maybe in the Terse Guide, I
>>>>>>> haven't checked actually.
>>>>>>>
>>>>>>
>>>>>> There is... not only one convention but there are some basic
>>>>>> guidelines. Formatting conventions are a matter of taste, so it is not easy
>>>>>> for all to share the same conventions and it is always a matter of furious
>>>>>> discussions :-)
>>>>>>
>>>>>>>
>>>>>>> About the comments:
>>>>>>>
>>>>>>> 1) The pipes are currently left empty when the last temporary
>>>>>>> variable is removed because it is not used, so I followed that convention.
>>>>>>> I do think that they should be removed though, but then we should implement
>>>>>>> the same behaviour when removing unused variables.
>>>>>>>
>>>>>>
>>>>>> hmm generally if you don't declare temp var you don't put the || in
>>>>>> the method... I think it is better to remove them if there is no more temp
>>>>>> var
>>>>>>
>>>>>
>>>>> No no, what I meant is that when saving, Cuis asks you if you want to
>>>>> remove unused variables, and when the last temporary variable is removed
>>>>> this way, the pipes are not removed. What I'm saying is that we should
>>>>> change this behaviour too.
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> 2) & 3) I'll look into them!
>>>>>>>
>>>>>>> Cool!
>>>>>>
>>>>>> Cheers!
>>>>>> Hernan.
>>>>>>
>>>>>>> Cheers,
>>>>>>> Eric
>>>>>>>
>>>>>>> El dom., 19 may. 2019 17:52, Hernan Wilkinson <
>>>>>>> hernan.wilkinson at 10pines.com> escribió:
>>>>>>>
>>>>>>>> Hi Eric,
>>>>>>>>  as I said before, great job! Nice implementation and very well
>>>>>>>> tested.
>>>>>>>>  It is on github now.
>>>>>>>>
>>>>>>>>  I made a few changes:
>>>>>>>> 1) Changed from cmd+P to cmd+O (as you said cmd+T is used) (Juan,
>>>>>>>> we will have to talk about shortcuts I guess, we are running out of them
>>>>>>>> :-) )
>>>>>>>> 2) Made some formatting changes to make it more "Smalltalk
>>>>>>>> oriented" :-)
>>>>>>>>
>>>>>>>>  I have a few improvements to suggest that it would be great if you
>>>>>>>> could do them:
>>>>>>>> 1) When the temporal to refactor is the only one, the source code
>>>>>>>> keeps the temp. definition pipes, the | |. It would be great if the were
>>>>>>>> removed from the source code
>>>>>>>> 2) When there is another method that defines the same temporary, it
>>>>>>>> would be great that the error message would say the method and class to
>>>>>>>> easily find it.
>>>>>>>> What I do in this cases is to show the methods as elements in the
>>>>>>>> menu so selecting them I can browse the method if necessary. Or you could
>>>>>>>> show an option like "browse methods" or something like that to browse all
>>>>>>>> those methods and therefore help to change those temporaries if necessary.
>>>>>>>> Look for some examples in other refactorings and appliers...
>>>>>>>> 3) The same thing when there are subclasses that define the inst.
>>>>>>>> var.
>>>>>>>>
>>>>>>>>  Thank you again! this is a great contribution to Cuis. Keep doing
>>>>>>>> more, we want more! :-)
>>>>>>>>
>>>>>>>> Cheers!
>>>>>>>> Hernan.
>>>>>>>>
>>>>>>>> On Fri, May 17, 2019 at 7:27 PM Eric Brandwein <
>>>>>>>> brandweineric at gmail.com> wrote:
>>>>>>>>
>>>>>>>>> I remember trying with Ctrl+shift+t and it being assigned to
>>>>>>>>> something else. I don't have the environment at hand right now, but I'll
>>>>>>>>> confirm later.
>>>>>>>>>
>>>>>>>>> El vie., 17 may. 2019 14:45, Hernan Wilkinson <
>>>>>>>>> hernan.wilkinson at 10pines.com> escribió:
>>>>>>>>>
>>>>>>>>>> Great! I'll take a look at it. It is really useful now that var
>>>>>>>>>> shadowing is not allowed.
>>>>>>>>>> One thing, ctrl+ship+p is used in LiveTyping to show the types of
>>>>>>>>>> the node under the cursor. What about using ctrl+shift+t ?
>>>>>>>>>>
>>>>>>>>>> Hernan
>>>>>>>>>>
>>>>>>>>>> PS:  Thank you for contributing to minimize the missing
>>>>>>>>>> refactorings :-)
>>>>>>>>>>
>>>>>>>>>> On Fri, May 17, 2019 at 1:49 PM Eric Brandwein via Cuis-dev <
>>>>>>>>>> cuis-dev at lists.cuis.st> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>> In my view, Cuis is missing a lot of refactorings. To start with
>>>>>>>>>>> something, here's a ChangeSet that adds the TemporaryToInstanceVariable
>>>>>>>>>>> refactor, along with a ChangeSet that adds tests. I've associated the
>>>>>>>>>>> <ctrl>+<shift>+P shortcut to it.
>>>>>>>>>>>
>>>>>>>>>>> To try it, just make a method with a temporary variable, place
>>>>>>>>>>> the cursor above the temporary, and press the shortcut.
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>> Eric
>>>>>>>>>>> --
>>>>>>>>>>> Cuis-dev mailing list
>>>>>>>>>>> Cuis-dev at lists.cuis.st
>>>>>>>>>>> https://lists.cuis.st/mailman/listinfo/cuis-dev
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>>
>>>>>>>>>> *Hernán WilkinsonAgile Software Development, Teaching & Coaching*
>>>>>>>>>> *Phone: +54-011*-4893-2057
>>>>>>>>>> *Twitter: @HernanWilkinson*
>>>>>>>>>> *site: http://www.10Pines.com <http://www.10pines.com/>*
>>>>>>>>>> Address: Alem 896, Floor 6, Buenos Aires, Argentina
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>>
>>>>>>>> *Hernán WilkinsonAgile Software Development, Teaching & Coaching*
>>>>>>>> *Phone: +54-011*-4893-2057
>>>>>>>> *Twitter: @HernanWilkinson*
>>>>>>>> *site: http://www.10Pines.com <http://www.10pines.com/>*
>>>>>>>> Address: Alem 896, Floor 6, Buenos Aires, Argentina
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> --
>>>>>>
>>>>>> *Hernán WilkinsonAgile Software Development, Teaching & Coaching*
>>>>>> *Phone: +54-011*-4893-2057
>>>>>> *Twitter: @HernanWilkinson*
>>>>>> *site: http://www.10Pines.com <http://www.10pines.com/>*
>>>>>> Address: Alem 896, Floor 6, Buenos Aires, Argentina
>>>>>>
>>>>>
>>>>
>>>> --
>>>>
>>>> *Hernán WilkinsonAgile Software Development, Teaching & Coaching*
>>>> *Phone: +54-011*-4893-2057
>>>> *Twitter: @HernanWilkinson*
>>>> *site: http://www.10Pines.com <http://www.10pines.com/>*
>>>> Address: Alem 896, Floor 6, Buenos Aires, Argentina
>>>>
>>>
>
> --
>
> *Hernán WilkinsonAgile Software Development, Teaching & Coaching*
> *Phone: +54-011*-4893-2057
> *Twitter: @HernanWilkinson*
> *site: http://www.10Pines.com <http://www.10pines.com/>*
> Address: Alem 896, Floor 6, Buenos Aires, Argentina
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190528/b11fd543/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 3778-TemporaryToInstanceVariableRemovesPipesInBlock-EricBrandwein-2019May28-19h16m-EB.1.cs.st
Type: application/octet-stream
Size: 11295 bytes
Desc: not available
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190528/b11fd543/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: TemporaryToInstanceVariableRemovesPipesInBlock-BaseImageTests-EB.1.cs.st
Type: application/octet-stream
Size: 1860 bytes
Desc: not available
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190528/b11fd543/attachment-0003.obj>