[squeak-dev] The Trunk: Monticello-tfel.637.mcz

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Mon Jul 25 22:06:19 UTC 2016


2016-07-25 23:56 GMT+02:00 Chris Muller <asqueaker at gmail.com>:

> Why interesting?  I tend to think that people should not be committing
> changes in which only the formatting was changed.  So, maybe better to
> take measures to minimize that from happening in the first place than
> introduce a button to "undo" it later....?
>
>
Well, it happens that such change are interesting...
For example some literal arrays are table that can suffer from misalignment.
Or some constant has been expressed in decimal when the intention would
have been more clear with hexa.

Also the feature concerns comment conflicts.

It's also interesting because it happens that we compare a very distant
branch like Squeak vs Pharo
and we don't want to be perturbated by cosmetic conflicts
(ok this happens less and less as the divergence increase).


> On Mon, Jul 25, 2016 at 4:43 PM, Nicolas Cellier
> <nicolas.cellier.aka.nice at gmail.com> wrote:
> > Whatever the flow of critics, I find the feature interesting.
> >
> >
> >
> > 2016-07-25 23:17 GMT+02:00 Chris Cunningham <cunningham.cb at gmail.com>:
> >>
> >>
> >>
> >> On Mon, Jul 25, 2016 at 10:03 AM, Nicolas Cellier
> >> <nicolas.cellier.aka.nice at gmail.com> wrote:
> >>>
> >>> Hi both,
> >>>
> >>> 2016-07-25 17:55 GMT+02:00 Chris Muller <asqueaker at gmail.com>:
> >>>>
> >>>> Please do not use abbreviations.  What is AST?  Abstract Syntax Tree?
> >>>> This takes the Monticello tool to a new level of technical
> >>>> intimidation and confusion.
> >>>>
> >>>> Why do we need a button for this?
> >>>
> >>>
> >>> I would say, why only one? Why a lack of symmetry?
> >>> The same AST means that some formatting has been performed.
> >>> I think AST currently contains comments, but see below...
> >>>
> >>> Of course, with auto-format that we currently apply in the UI, we can't
> >>> even visualize those diff (grrr! I hate those auto-format)
> >>> So anyway we are blind to these changes, we could as well throw a dice
> ;)
> >>>
> >>>>
> >>>>
> >>>> On Mon, Jul 25, 2016 at 8:24 AM,  <commits at source.squeak.org> wrote:
> >>>> > Tim Felgentreff uploaded a new version of Monticello to project The
> >>>> > Trunk:
> >>>> > http://source.squeak.org/trunk/Monticello-tfel.637.mcz
> >>>> >
> >>>> > ==================== Summary ====================
> >>>> >
> >>>> > Name: Monticello-tfel.637
> >>>> > Author: tfel
> >>>> > Time: 25 July 2016, 3:24:24.996828 pm
> >>>> > UUID: cf0d6af1-c703-5044-9c57-e798b0cf3abf
> >>>> > Ancestors: Monticello-cmm.636
> >>>> >
> >>>> > add a button for rejecting all incoming conflicts that only change
> AST
> >>>> >
> >>>> > =============== Diff against Monticello-cmm.636 ===============
> >>>> >
> >>>> > Item was added:
> >>>> > + ----- Method: MCConflict>>chooseSameAST (in category 'as yet
> >>>> > unclassified') -----
> >>>> > + chooseSameAST
> >>>> > +       | fromSrc toSrc |
> >>>> > +       (self definition isNil or: [self definition
> isMethodDefinition
> >>>> > not])
> >>>> > +               ifTrue: [^ self].
> >>>> > +       fromSrc := (Parser new parse: operation fromSource class:
> nil
> >>>> > class)
> >>>> > +               generate decompile asString.
> >>>> > +       toSrc := (Parser new parse: operation toSource class: nil
> >>>> > class)
> >>>> > +               generate decompile asString.
> >>>> > +       fromSrc = toSrc ifTrue: [self chooseLocal].!
> >>>> >
> >>>
> >>>
> >>> OK, so this does not even take the comments in account, because this is
> >>> decompiled code (reconstructed AST) not original AST.
> >>> There's another problem here: decompile: sometimes fail, so we should
> >>> protect ourselves...
> >>>
> >>>
> >>>>
> >>>> > Item was changed:
> >>>> >   ----- Method: MCMergeBrowser>>buttonSpecs (in category 'as yet
> >>>> > unclassified') -----
> >>>> >   buttonSpecs
> >>>> >         ^ #((Merge merge 'Proceed with the merge' canMerge)
> >>>> >                  (Cancel cancel 'Cancel the merge')
> >>>> >                 ('All Newer' chooseAllNewerConflicts 'Choose all
> newer
> >>>> > conflict versions')
> >>>> >                 ('All Older' chooseAllOlderConflicts 'Choose all
> older
> >>>> > conflict versions')
> >>>> >                 ('Rest Reject' chooseAllUnchosenLocal 'Choose local
> >>>> > versions of all remaining conflicts')
> >>>> >                 ('Rest Accept' chooseAllUnchosenRemote 'Choose
> remote
> >>>> > versions of all remaining conflicts')
> >>>> > +               ('Accept same source' chooseAllSameAST 'Choose all
> >>>> > local conflicting versions that have essentially the same code')
> >>>> >   )!
> >>>> >
> >>>
> >>>
> >>> There's another problem above:
> >>> Rest Accept is for accepting the remote changes
> >>> Accept same source is for accepting the local...
> >>>
> >>> That can't be the same word, much confusing!
> >>>
> >> Right.  If that option is added, it must be called 'Reject same source'.
> >>
> >> Probably call it something else: 'Reject cosmetic changes', maybe.
> >>
> >> In any case, let's not make it worse after recently cleaning up the
> >> wording.
> >>
> >> -cbc
> >>
> >>
> >>
> >>
> >
> >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20160726/cfbe59fc/attachment.htm


More information about the Squeak-dev mailing list