[Cuis-dev] Temporary to Instance Variable refactor

Sun May 26 12:00:40 PDT 2019

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/20190526/f2e19f63/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 3766-TemporaryToInstanceVariableBetterErrorMessageAndRemovesPipes-EricBrandwein-2019May24-18h51m-EB.1.cs.st
Type: application/octet-stream
Size: 2238 bytes
Desc: not available
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190526/f2e19f63/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: UnsavedChangesTo-BaseImageTests-EB.1.cs.st
Type: application/octet-stream
Size: 8834 bytes
Desc: not available
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190526/f2e19f63/attachment-0003.obj>