[Cuis-dev] Temporary to Instance Variable refactor

Eric Brandwein brandweineric at gmail.com
Mon May 27 20:21:35 PDT 2019 

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
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190528/3564ff85/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 3767-TemporaryToInstanceVariableWithMultipleBlocks-EricBrandwein-2019May28-00h17m-EB.1.cs.st
Type: application/octet-stream
Size: 2508 bytes
Desc: not available
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190528/3564ff85/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: TemporaryToInstanceVariableWithMultipleBlocks-BaseImageTests-EB.1.cs.st
Type: application/octet-stream
Size: 1066 bytes
Desc: not available
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190528/3564ff85/attachment-0003.obj>

More information about the Cuis-dev mailing list