[Cuis-dev] Browser class hierarchy & functionality

[Phil B]

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/20191205/cc1137e5/attachment.htm>