Yeah, no comment nowhere and most methods 'as yet unclassified' (we fixed
many of those already).
I guess that the idea was that with a clear separation of concerns and
limitation of responsibilities, comments coud eventually become moot.
Even classification of methods could be seen as unhelpful if there are few
enough per class...
It's been a nice exercise in style.

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...
The alternative you're proposing does work thanks to an ugly ifError:
handling...
We're sending versionName to a MCWorkingCopyAncestry which is a MNU.
We have absolutely no idea why a MCWorkingCopyAncestry would ever be a good
candidate for self selectedInfo...
We have no idea why a MCVersionInfo has to be a specialization of a
MCAncestry.
But then, should we subclass MCWorkingCopyAncestry and get a MCWorkingCopyInfo?
Inheritance vs composition?
We have no idea why we must search a specific version in MCRepositoryGroup
default, rather than the more restricted group registered for this
package...
So we end up 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.
Clearly not such a good example of crystal clear and straightforward code
to my eyes...
I'd rather say crooked!

I don't know when and how exactly we got there...
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)...
But at this stage, we need more than comments. We need deeper clean up.
Every piece of code requires such lifting in order to sustain the burden of
age, even the nicest pieces...

Concerning the lack of comments, I have my own opinion: I think it's a
great mistake.
The mental image of the zoo of classes and concepts is certainly obvious to
the authors, or the experts, once sufficiently fluent.
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.
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?

Let's follow my example: why does each and every MCWorkingCopy has a (nil)
versionInfo?
My guess when reading code is that such info could be (have been?) a proxy
for rebuilding the ancestry (a cheaper  memory footprint?).
But then what? There no setter but nil... Is it a dead code?
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.
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.

End of rant!

Le mer. 30 déc. 2020 à 17:52, David T. Lewis <lewis at mail.msen.com> a écrit :

> On Wed, Dec 30, 2020 at 02:57:23PM +0000, commits at source.squeak.org wrote:
> > A new version of Monticello was added to project The Inbox:
> > http://source.squeak.org/inbox/Monticello-nice.734.mcz
> >
> > ==================== Summary ====================
> >
> > Name: Monticello-nice.734
> > Author: nice
> > Time: 30 December 2020, 3:57:21.60758 pm
> > UUID: 863f642a-7795-4eb6-aa6b-76a5af3a8504
> > Ancestors: Monticello-mt.733
> >
> > Workaround version history browser bug when 'working copy' is selected.
> >
> > The snapshot is searched in repositoryGroup, and logically, none is
> found.
> > Note that the self selectedInfo isKindOf: MCWorkingCopyAncestry, which
> does not behave as a regular MCVersionInfo, it does not even answer to
> versionName...
> > 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.
> >
> > The workaround is not very nice, it use implicit knowledge that 'working
> copy' will be at top of list (index = 1).
> >
> > If you can think of nicer fix, please raise voice.
> >
>
> Attached is another way to do it without relying on the list index. I have
> not tested very much but it seems to be all right.
>
>   MCVersionHistoryBrowser>>snapshotForInfo: aVersionInfo
>      "Answer snapshot for aVersionInfo or for the working copy if not
> found"
>      ^ (self repositoryGroup
>            versionWithInfo: aVersionInfo
>               ifNone: [ package ]) snapshot
>
>
> Apologies for violating MC package standards by including a method comment
> ;-)
>
> Dave
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20201230/18552f0c/attachment-0001.html>