<div dir="ltr"><div>Yeah, no comment nowhere and most methods 'as yet unclassified' (we fixed many of those already).</div><div>I guess that the idea was that with a clear separation of concerns and limitation of responsibilities, comments coud eventually become moot.</div><div>Even classification of methods could be seen as unhelpful if there are few enough per class...</div><div>It's been a nice exercise in style.</div><div><br></div><div>But in corners like this, nothing is that obvious, sorry but we're well beyond the limit of what an average Smalltalker can follow and understand...</div><div>The alternative you're proposing does work thanks to an ugly ifError: handling...</div><div>We're sending versionName to a <span class="gmail-im">MCWorkingCopyAncestry which is a MNU.</span></div><div><span class="gmail-im">We have absolutely no idea why a <span class="gmail-im">MCWorkingCopyAncestry</span> would ever be a good candidate for self selectedInfo...<br></span></div><div><span class="gmail-im">We have no idea why a MCVersionInfo has to be a specialization of a MCAncestry.</span></div><div><span class="gmail-im">But then, should we subclass <span class="gmail-im">MCWorkingCopyAncestry and get a <span class="gmail-im">MCWorkingCopyInfo? Inheritance vs composition?<br></span></span></span></div><div><span class="gmail-im"><span class="gmail-im"><span class="gmail-im">We have no idea why we must search a specific version in MCRepositoryGroup default, rather than the more restricted group registered for this package...<br></span></span></span></div>So we end up<span class="gmail-im"> scanning all the repositories in the default repositoryGroup for getting a snapshot of working copy, which is quiet a vain idea, the snapshot lies in no repository, it's right there under our mouse, in the image.<br></span><div><span class="gmail-im">Clearly not such a good example of crystal clear and straightforward code to my eyes...</span></div><div><span class="gmail-im">I'd rather say crooked!</span></div><div><span class="gmail-im"><br></span></div><div><span class="gmail-im">I don't know when and how exactly we got there...</span></div><div><span class="gmail-im">Is it from inception? Or gradual rot due to quick hacking (we often do the EASIEST thing that could possibly work, rather than the SIMPLEST thing that could possibly sustain)...</span></div><div><span class="gmail-im">But at this stage, we need more than comments. We need deeper clean up.</span></div><div><span class="gmail-im">Every piece of code requires such lifting in order to sustain the burden of age, even the nicest pieces...<br></span></div><div><span class="gmail-im"><br></span></div><div><span class="gmail-im">Concerning the lack of comments, I have my own opinion: I think it's a great mistake.</span></div><div><span class="gmail-im">The mental image of the zoo of classes and concepts is certainly obvious to the authors, or the experts, once sufficiently fluent.<br></span></div><div><span class="gmail-im">But the lack of comment or tutorial or even the lightest introduction to the concepts and key implementation choices, will for sure delay the learning curve of newbies.</span></div><div><span class="gmail-im">If you have to deal with such a bug, you're directly thrown into the crooked parts of code, and the austerity of comment is just intimidating. Where to begin the journey in MC?<br></span></div><div><span class="gmail-im"><br></span></div><div><span class="gmail-im">Let's follow my example: why does each and every MCWorkingCopy has a (nil) versionInfo?</span></div><div><span class="gmail-im">My guess when reading code is that such info could be (have been?) a proxy for rebuilding the ancestry (a cheaper memory footprint?).<br></span></div><div><span class="gmail-im">But then what? There no setter but nil... Is it a dead code?<br></span></div><div><span class="gmail-im">When the bug I'm after concerns the selectedInfo of the working copy, the lack of comment and clear intentions just let me lost in a desert.<br></span></div><div><span class="gmail-im">My idea would be to (double) dispatch the search of snapshot to the info, but I have no idea what could be a good candidate implementation.<br></span></div><div><span class="gmail-im"><br></span></div><div><span class="gmail-im">End of rant!<br></span></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le mer. 30 déc. 2020 à 17:52, David T. Lewis <<a href="mailto:lewis@mail.msen.com">lewis@mail.msen.com</a>> a écrit :<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 Wed, Dec 30, 2020 at 02:57:23PM +0000, <a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a> wrote:<br>
> A new version of Monticello was added to project The Inbox:<br>
> <a href="http://source.squeak.org/inbox/Monticello-nice.734.mcz" rel="noreferrer" target="_blank">http://source.squeak.org/inbox/Monticello-nice.734.mcz</a><br>
> <br>
> ==================== Summary ====================<br>
> <br>
> Name: Monticello-nice.734<br>
> Author: nice<br>
> Time: 30 December 2020, 3:57:21.60758 pm<br>
> UUID: 863f642a-7795-4eb6-aa6b-76a5af3a8504<br>
> Ancestors: Monticello-mt.733<br>
> <br>
> Workaround version history browser bug when 'working copy' is selected.<br>
> <br>
> The snapshot is searched in repositoryGroup, and logically, none is found.<br>
> Note that the self selectedInfo isKindOf: MCWorkingCopyAncestry, which does not behave as a regular MCVersionInfo, it does not even answer to versionName...<br>
> This somehow accelerate the failure thru an ifError: [] handling, but at the end, the selectedSnapshot is still nil which makes the 'view changes from...' menu fail.<br>
> <br>
> The workaround is not very nice, it use implicit knowledge that 'working copy' will be at top of list (index = 1).<br>
> <br>
> If you can think of nicer fix, please raise voice.<br>
><br>
<br>
Attached is another way to do it without relying on the list index. I have<br>
not tested very much but it seems to be all right.<br>
<br>
MCVersionHistoryBrowser>>snapshotForInfo: aVersionInfo<br>
"Answer snapshot for aVersionInfo or for the working copy if not found"<br>
^ (self repositoryGroup<br>
versionWithInfo: aVersionInfo<br>
ifNone: [ package ]) snapshot<br>
<br>
<br>
Apologies for violating MC package standards by including a method comment ;-)<br>
<br>
Dave<br>
<br>
<br>
</blockquote></div>