[Cuis-dev] Browser class hierarchy & functionality

Thu Jan 9 21:23:42 PST 2020

OK, scratch the previous color idea: how about green (add), blue (change)
and red (delete)?  See attached for a first cut of the proposed color
scheme.   The implementation is ugly and hacky but it works (sort of)...
still thinking about the best approach.

One additional thing I was thinking about was extending the list coloring
to categories so I did that for deleted methods since it was easy.  Not
sure if I'm going to have time right now to go that far as it has more
issues to deal with, but I wanted to play around with the concept.

On Thu, Jan 9, 2020 at 10:04 PM Phil B <pbpublist at gmail.com> wrote:

> If it looks good, I'd say go ahead and integrate it.  I saw your color
> changes so I've been playing around with it and my current thinking is that
> this is going to require some invasive changes in other areas so probably
> better to do the list stuff as a separate changeset.
>
> Also, since we're fixing things: are you OK if I change the diff coloring
> from reds for adds and blue for deletes to blue for adds and red for
> deletes?  This has been bugging me forever: in every diff tool I've ever
> used red means deleted!
>
> On Thu, Jan 9, 2020 at 9:33 PM Juan Vuletich <juan at jvuletich.org> wrote:
>
>> Hi Phil,
>>
>> Your latest code looks great to me. Just say when you want it integrated.
>> Thanks!
>>
>> BTW, today I pushed some updates that, after some cleanup and
>> refactoring, enable colors, bold, italic, etc on ListMorphs. Relevant
>> change set is #4011. It is almost trivial. To try this, add to
>> CodeFileBrowser:
>>
>> messageList
>>     ^ super messageList collect: [ :str | | text |
>>         text _ str asText color: Color random.
>>         Random withDefaultDo: [ :random |
>>             random next > 0.7
>>                 ifTrue: [ text _ text bold ]
>>                 ifFalse: [ random next > 0.7 ifTrue: [ text _ text italic
>> ]]].
>>         text ]
>>
>> Or simply
>>
>> messageList
>>     ^ super messageList collect: [ :str | str blue ]
>>
>> WRT older change sets in the repo, thanks for reminding us. I just added
>> the 3001-4000.zip file, and removed all the changesets in CoreUpdates/ ,
>> except those not yet loaded in the images.
>>
>> Cheers,
>>
>> --
>> Juan Vuletichwww.cuis-smalltalk.orghttps://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Devhttps://github.com/jvuletichhttps://www.linkedin.com/in/juan-vuletich-75611b3
>> @JuanVuletich
>>
>>
>>
>> On 1/9/2020 6:10 PM, Phil B wrote:
>>
>> Juan,
>>
>> (Somewhat related: it looks like it's time to archive the 3xxx series
>> changes in the repo since github cuts web directory listings off at 1000)
>>
>> On Thu, Jan 9, 2020 at 7:21 AM Juan Vuletich <juan at jvuletich.org> wrote:
>>
>>>
>>> If not, why don't we go with a half step of turning it off by default
>>> and see how people react before removing the option entirely?
>>>
>>>
>>> Because current behavior is inconsistent, and keeping it adds unneeded
>>> complexity to the code. Let's make it better and simpler at the same time.
>>>
>>
>> Done.
>>
>>
>>>
>>>
>>>> Besides, in #infoViewContents, when method is not in 'base' it says
>>>> '**NEW** Method not in the system'. It should read something like
>>>> '**ADDED** Method not in base'. Same for class. It says 'Class not in the
>>>> system', and should say 'Class not in base'. All these messages should stay
>>>> like now if base is Smalltalk image, though.
>>>>
>>>
>>> So change 'system' to 'base' if base is not an image?  (keep in mind
>>> that it is possible that base could be a file and case could be the image
>>> or base and case could both be images.  Also, there's no guarantee that
>>> 'image' refers to the currently, locally running image...)
>>>
>>>
>>> Maybe we could have a couple new ivars: 'caseDescription' and
>>> 'baseDescription', and build the user messages with them. This would make
>>> it easier to tweak them until we are happy.
>>>
>>
>> Also done.
>>
>>
>>>
>>>
>>>> Another issue is that in '*** removed methods ***' it shows actively
>>>> methods deleted in a change set, but not missing methods (defined in 'base'
>>>> but not in 'case'). Maybe, when 'base' is not the Smalltalk image, a new
>>>> category with 'methods not included' or such could be added.
>>>>
>>>
>>> The current behavior doesn't show this does it?  I was trying to
>>> minimize breakage or behavior changes (at least for now), but we can change
>>> this if you want.  This is an area I had planned to fill out more later
>>> once it seemed like the core changes are solid.
>>>
>>>
>>> Current behavior doesn't do it because it was thought for ChangeSets. A
>>> ChangeSet doesn't include "all the stuff", just the part it affects.
>>> Therefore, deletions must be explicit to be acknowledged. A packages is
>>> different. And this becomes clearer when base and case can be different
>>> versions of a package, possibly not loaded in the image.
>>>
>>> But we can do this after integrating your first work, checking that
>>> people is happy with them, and fixing any breakage.
>>>
>>
>> OK.
>>
>>
>>>
>>>
>>>
>>>> Finally (I know, I'm asking lots of things!), it would be cool to have,
>>>> in the message list, added methods in red, and removed/missing methods in
>>>> blue. This might not be trivial. Not sure if our lists can do color
>>>> highlighting.
>>>>
>>>
>>> I agree that would be nice to have but I don't think lists can currently
>>> do that.  If you add colored highlighting support to lists, I'll use it :-)
>>>
>>>
>>> Fair enough! I'll do it.
>>>
>>
>> Ready when you are.
>>
>>
>>>
>>>
>>>
>>>> And maybe we could change the colors... Red for new stuff is too
>>>> strong, and usually means 'NO' (like a stop sign). Maybe Green would be
>>>> better. Not sure.
>>>>
>>>
>>> I agree... red for add makes no sense.  But that was consistent with
>>> current behavior so I kept it.  I would prefer something green (or blue)
>>> for adds, yellow for changes and red for deletes.  However, I wanted to
>>> hold off on changing coloring logic until after we're sure that these
>>> changes otherwise make sense and don't break anything.  While I'm
>>> reasonably comfortable with the changes relative to the base image, I was
>>> hoping to get it integrated to see if this breaks anyone else's stuff
>>> before actively trying to change the look/behavior too much to minimize
>>> confusion.
>>>
>>>
>>> Ok. This makes sense. Let's change colors later. Hopefully soon, before
>>> this loses moment.
>>>
>>
>> Agreed.
>>
>>
>>> Cheers,
>>>
>>> --
>>> Juan Vuletichwww.cuis-smalltalk.orghttps://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Devhttps://github.com/jvuletichhttps://www.linkedin.com/in/juan-vuletich-75611b3
>>> @JuanVuletich
>>>
>>>
>> Thanks,
>> Phil
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20200110/5723e275/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 4013-CodeFileBrowser-color-lists-PhilBellalouna-2020Jan09-21h17m-pb.1.cs.st
Type: application/octet-stream
Size: 9016 bytes
Desc: not available
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20200110/5723e275/attachment-0001.obj>