[Cuis-dev] Browser class hierarchy & functionality
Juan Vuletich
juan at jvuletich.org
Thu Dec 5 06:57:41 PST 2019
Hi Phil,
I really like what you are doing here.
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.
Thanks,
--
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
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
> <mailto: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
> <mailto: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 <http://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/7b14d811/attachment-0001.htm>
More information about the Cuis-dev
mailing list