<div dir="auto"><div>Juan,<br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Dec 5, 2019, 9:57 AM Juan Vuletich <<a href="mailto:juan@jvuletich.org">juan@jvuletich.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><u></u>

  
    
  
  <div bgcolor="#ffffff" text="#000000">
    Hi Phil,<br>
    <br>
    I really like what you are doing here.<br></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Thanks... it's taken a couple attempts to get something that didn't turn into a mess</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#ffffff" text="#000000">
    <br>
    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.<br></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">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)</div><div dir="auto"><br></div><div dir="auto">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)</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#ffffff" text="#000000">
    <br>
    Thanks,<br>
    <pre cols="72">-- 
Juan Vuletich
<a href="http://www.cuis-smalltalk.org" target="_blank" rel="noreferrer">www.cuis-smalltalk.org</a>
<a href="https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev" target="_blank" rel="noreferrer">https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev</a>
<a href="https://github.com/jvuletich" target="_blank" rel="noreferrer">https://github.com/jvuletich</a>
<a href="https://www.linkedin.com/in/juan-vuletich-75611b3" target="_blank" rel="noreferrer">https://www.linkedin.com/in/juan-vuletich-75611b3</a>
@JuanVuletich</pre></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Thanks,</div><div dir="auto">Phil</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#ffffff" text="#000000">
    <br>
    <br>
    On 12/5/2019 6:34 AM, Phil B wrote:
    <blockquote type="cite">
      <div dir="ltr">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.
        <div><br>
        </div>
        <div>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.</div>
        <div><br>
        </div>
        <div>Here are a few notes on where things stand:</div>
        <div><br>
        </div>
        <div>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.</div>
        <div><br>
        </div>
        <div>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)</div>
        <div><br>
        </div>
        <div>3) Currently the image can only be a target (work in
          progress)</div>
        <div><br>
        </div>
        <div>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.</div>
        <div><br>
        </div>
        <div>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.</div>
        <div><br>
        </div>
        <div>6) Related to 5, the button pane needs conditional work
          (still thinking about how to best deal with this)</div>
        <div><br>
        </div>
        <div>So what do you think?  Anything obvious I missed?</div>
        <div><br>
        </div>
        <div>Phil</div>
        <div><br>
        </div>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr" class="gmail_attr">On Sat, Nov 2, 2019 at 2:50 PM
          Phil B <<a href="mailto:pbpublist@gmail.com" target="_blank" rel="noreferrer">pbpublist@gmail.com</a>>
          wrote:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
          <div dir="ltr">
            <div dir="ltr">Juan,</div>
            <div><br>
            </div>
            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.
            <div>
              <div>
                <div><br>
                </div>
                <div>Thanks,</div>
                <div>Phil</div>
                <div><br>
                  <div class="gmail_quote">
                    <div dir="ltr" class="gmail_attr">On Sat, Nov 2,
                      2019 at 1:35 PM Juan Vuletich <<a href="mailto:juan@jvuletich.org" target="_blank" rel="noreferrer">juan@jvuletich.org</a>>
                      wrote:<br>
                    </div>
                    <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 10/29/2019 2:17
                      PM, Phil B via Cuis-dev wrote:<br>
                      > I was looking into adding the ability to
                      easily analyze/diff two <br>
                      > sources of Smalltalk code (i.e. basically the
                      same image vs. changeset <br>
                      > or image vs. package functionality we
                      currently have but expanding the <br>
                      > capability to allow for any two arbitrary
                      sources whether changesets <br>
                      > vs. changeset, package vs. package, package
                      vs. changeset or some <br>
                      > other pairing entirely) and it seems a bit
                      harder than it needs to be <br>
                      > since the existing implementation assumes you
                      will always be comparing <br>
                      > the image vs. something else.<br>
                      ><br>
                      > If this already exists and I'm looking in the
                      wrong place, pointers <br>
                      > are appreciated.  If it doesn't, would there
                      be any objection to <br>
                      > modifying/cleaning up this code so that we
                      could optionally set an <br>
                      > 'other' Code*Browser that would be used
                      rather than always assuming <br>
                      > the image?  A related enhancement could be to
                      make the Code*Browser <br>
                      > code a bit smarter so that we don't need to
                      explicitly determine if <br>
                      > 'this is a package' or 'this is a changeset'
                      unless you want to.  <br>
                      > Basically I'm asking if we would be willing
                      to trade a (slight) <br>
                      > increase in implementation complexity for
                      increased code source <br>
                      > flexibility here?<br>
                      <br>
                      Hi Phil,<br>
                      <br>
                      What you want is not yet done. I think it is a
                      good idea. I suggest <br>
                      writing new (or improving existing) Model classes,
                      for code already <br>
                      loaded in the image (in base image or in
                      packages), and for code in <br>
                      files (CodePackageFile, etc). Ideally, the same
                      View classes (i.e. the <br>
                      Morphic Windows) should be the same and work on
                      whatever model. Models <br>
                      should be polymorphic enough so that comparing
                      works irrespectively of <br>
                      the models involved.<br>
                      <br>
                      This could be a nice improvement for the system.<br>
                      <br>
                      Cheers,<br>
                      <br>
                      -- <br>
                      Juan Vuletich<br>
                      <a href="http://www.cuis-smalltalk.org" rel="noreferrer noreferrer" target="_blank">www.cuis-smalltalk.org</a><br>
                      <a href="https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev" rel="noreferrer noreferrer" target="_blank">https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev</a><br>
                      <a href="https://github.com/jvuletich" rel="noreferrer noreferrer" target="_blank">https://github.com/jvuletich</a><br>
                      <a href="https://www.linkedin.com/in/juan-vuletich-75611b3" rel="noreferrer noreferrer" target="_blank">https://www.linkedin.com/in/juan-vuletich-75611b3</a><br>
                      @JuanVuletich<br>
                      <br>
                    </blockquote>
                  </div>
                </div>
              </div>
            </div>
          </div>
        </blockquote>
      </div>
    </blockquote>
    <br>
  </div>

</blockquote></div></div></div>