[Cuis-dev] Browser class hierarchy & functionality

[Phil B]

<bump>

On Thu, Dec 12, 2019 at 2:27 AM Phil B <pbpublist at gmail.com> wrote:

> OK, I think this version addresses the items from the previous version[1]
> and your feedback.  There were about half a dozen methods with breaking
> changes (i.e. either needed to be redefined or renamed) to align
> CodeFileBrowser->SystemDictionary and PseudoClass->ClassDescription but I
> didn't see any obvious users of them other than CodeFileBrowser so I don't
> think this impacts anything else.
>
> [1] The one exception was item 2... this can't be 'fixed' without doing a
> rather convoluted 3-way diff with the image which may, or may not, make
> sense depending on what you're looking at.  So for now at least, I'm
> leaving it as is.
>
> On Thu, Dec 5, 2019 at 2:59 PM Phil B <pbpublist at gmail.com> wrote:
>
>> Juan,
>>
>> On Thu, Dec 5, 2019, 9:57 AM Juan Vuletich <juan at jvuletich.org> wrote:
>>
>>> Hi Phil,
>>>
>>> I really like what you are doing here.
>>>
>>
>> Thanks... it's taken a couple attempts to get something that didn't turn
>> into a mess
>>
>>
>>> However there is one detail I think can be improved. You call the stuff
>>> that can be compared 'target' and 'source'. The problem is that 'source'
>>> means 'source code' in many places and this could be confusing. Besides,
>>> 'target' doesn't obviously mean 'the reference we are comparing against'.
>>> DifferenceFinder calls them 'base' and 'case'. This sounds clearer to me.
>>> Or may be there are better names to be found.
>>>
>>
>> I wasn't wild about the naming either.  So baseCodeSource and
>> caseCodeSource?  (I'm not crazy about the CodeSource part of the name
>> either but wanted to come up with a convention that would hint at how
>> future implementations/interfaces might be named)
>>
>> Also, are you OK with the conditional logic on the menu items?  I wanted
>> to check on this before doing something similar with the button bar (i.e.
>> when an image isn't the target/base (or is it case?) most of the items
>> should probably be disabled)
>>
>>
>>> Thanks,
>>>
>>> --
>>> 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
>>
>>
>>>
>>> On 12/5/2019 6:34 AM, Phil B wrote:
>>>
>>> I finally had some time to work on this and got something working.  I
>>> wanted to throw it out there for feedback before going any further with it
>>> as there are a few outstanding issues.
>>>
>>> What this changeset should allow for is any pairing of Smalltalk
>>> image/changeset/package in a CodeFileBrowserWindow (though most of the
>>> changes are actually in CodeFileBrowser)  By default, you can still open a
>>> browser window from a FileListWindow as always.  But now when you want a
>>> non-standard browser you can 'CodeFileBrowser browseCodeSource:
>>> (CodePackageFile newFromFile: someFileEntry) target: (CodePackageFile
>>> newFromFile: someOtherFileEntry)' (you can also either leave one of the
>>> arguments as nil or pass Smalltalk to use the image as your
>>> source/target... see 3 below)  You should also be able to add arbitrary
>>> other code sources (i.e. remote images as sources/targets are a
>>> possibility) as well.
>>>
>>> Here are a few notes on where things stand:
>>>
>>> 1) One of the things that I ran into doing this that I really liked was
>>> disabling the 'false color' diff formatting of the browser so I made this
>>> an optional flag.  This is handy when you're looking at a piece of code and
>>> want to see missing methods/classes in the image.  It's far from perfect
>>> (i.e. it is very dumb as methods defined in the source aren't taken into
>>> account), but I still find it handy and if it makes sense this could be
>>> made a UI toggle.
>>>
>>> 2) Related to 1 is that the existing diffing code doesn't know what to
>>> do in non-standard configurations (i.e. when target is not the Smalltalk
>>> image) so view as source always appears to be running with 'false color'
>>> off (this is a bug)
>>>
>>> 3) Currently the image can only be a target (work in progress)
>>>
>>> 4) There was one breaking method change:  PseudoClass>>includesSelector:
>>> so if I've missed any senders that depend on the old behavior (I only saw
>>> one obvious one) they will need minor changes.
>>>
>>> 5) Pane menus are a bit more complex since some of the options don't
>>> make sense if either the image isn't the target or a changeset/package
>>> isn't the source.
>>>
>>> 6) Related to 5, the button pane needs conditional work (still thinking
>>> about how to best deal with this)
>>>
>>> So what do you think?  Anything obvious I missed?
>>>
>>> Phil
>>>
>>>
>>> On Sat, Nov 2, 2019 at 2:50 PM Phil B <pbpublist at gmail.com> wrote:
>>>
>>>> Juan,
>>>>
>>>> OK, that's basically what I was thinking.   I just wanted to see if you
>>>> were open to it first since it will require some rework (I'm not currently
>>>> thinking a rewrite is in order... we'll see) of the existing model
>>>> classes.  I'll start with the model and if the direction makes sense then
>>>> can enhance the views to expose the functionality.
>>>>
>>>> Thanks,
>>>> Phil
>>>>
>>>> On Sat, Nov 2, 2019 at 1:35 PM Juan Vuletich <juan at jvuletich.org>
>>>> wrote:
>>>>
>>>>> On 10/29/2019 2:17 PM, Phil B via Cuis-dev wrote:
>>>>> > I was looking into adding the ability to easily analyze/diff two
>>>>> > sources of Smalltalk code (i.e. basically the same image vs.
>>>>> changeset
>>>>> > or image vs. package functionality we currently have but expanding
>>>>> the
>>>>> > capability to allow for any two arbitrary sources whether changesets
>>>>> > vs. changeset, package vs. package, package vs. changeset or some
>>>>> > other pairing entirely) and it seems a bit harder than it needs to
>>>>> be
>>>>> > since the existing implementation assumes you will always be
>>>>> comparing
>>>>> > the image vs. something else.
>>>>> >
>>>>> > If this already exists and I'm looking in the wrong place, pointers
>>>>> > are appreciated.  If it doesn't, would there be any objection to
>>>>> > modifying/cleaning up this code so that we could optionally set an
>>>>> > 'other' Code*Browser that would be used rather than always assuming
>>>>> > the image?  A related enhancement could be to make the Code*Browser
>>>>> > code a bit smarter so that we don't need to explicitly determine if
>>>>> > 'this is a package' or 'this is a changeset' unless you want to.
>>>>> > Basically I'm asking if we would be willing to trade a (slight)
>>>>> > increase in implementation complexity for increased code source
>>>>> > flexibility here?
>>>>>
>>>>> Hi Phil,
>>>>>
>>>>> What you want is not yet done. I think it is a good idea. I suggest
>>>>> writing new (or improving existing) Model classes, for code already
>>>>> loaded in the image (in base image or in packages), and for code in
>>>>> files (CodePackageFile, etc). Ideally, the same View classes (i.e. the
>>>>> Morphic Windows) should be the same and work on whatever model. Models
>>>>> should be polymorphic enough so that comparing works irrespectively of
>>>>> the models involved.
>>>>>
>>>>> This could be a nice improvement for the system.
>>>>>
>>>>> Cheers,
>>>>>
>>>>> --
>>>>> Juan Vuletich
>>>>> www.cuis-smalltalk.org
>>>>> https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev
>>>>> https://github.com/jvuletich
>>>>> https://www.linkedin.com/in/juan-vuletich-75611b3
>>>>> @JuanVuletich
>>>>>
>>>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20200106/243ba9b5/attachment.htm>