<div dir="ltr"><div><div><div><div>Yes, this is really a dictatorial abuse of committer's right.<br>We have discussed on the mailing list, got minimal agreement on the goals.<br>It's fine but then someone has to do the job.</div>
</div>IMO this kind of stuff has to go into a sprint, otherwise it is never finished.<br><br>If we put a bunch of changes in the inbox with complex load order, I can tell you that this will hardly be reviewable.<br><div>Without dictatorship, this kind of change usually does not get integrated.<br>
</div>I
wouldn't publish on Pharo slice/bug tracker for example, because when I have a bit complex change it often does not get integrated and the bug
report stales (too hard to understand/review/code rots).<br>For example, see what happened to <a href="http://code.google.com/p/pharo/issues/detail?id=5406">http://code.google.com/p/pharo/issues/detail?id=5406</a><br><br>
And with the impulsion/support of tim that's different, we have at least four eyes.<br><br></div><div>Now, we have to cross fingers because, yes, it's more aggressive than usual.<br></div></div></div><div class="gmail_extra">
<br><br><div class="gmail_quote">2013/9/30 Chris Muller <span dir="ltr"><<a href="mailto:asqueaker@gmail.com" target="_blank">asqueaker@gmail.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Yea, I was getting caught up after a week absent the list, saw your<br>
other commits and figured they were related.<br>
<br>
These paragraph bits are really complicated, yet crucial. It feels a<br>
bit aggressive to go straight into trunk, as I agree with Tim's<br>
comment about peer review and peer testing.<br>
<br>
In any case, bravo to you two for braving this code and supporting the issues.<br>
<div class="HOEnZb"><div class="h5"><br>
On Mon, Sep 30, 2013 at 2:52 PM, Nicolas Cellier<br>
<<a href="mailto:nicolas.cellier.aka.nice@gmail.com">nicolas.cellier.aka.nice@gmail.com</a>> wrote:<br>
> Ah yes, but it's a multiple .mcz saga whose goal is to:<br>
> Remove the Multilingual specific classes for composing text layout.<br>
><br>
> And this episode is specifically about removal of presentation as said...<br>
> What is a presentation/presentationText/presentationLines ?<br>
> It is an artifact containing precomposed unicode produced by some<br>
> Multi*Scanner.<br>
> Why to remove it?<br>
> Because we can compose/display/scan decomposed unicode and recompose on the<br>
> fly without exhibiting the artifact.<br>
> But it yet remains to be implemented...<br>
> Anyway, the feature was broken and deconnected some years ago, so it seems<br>
> that we have time to re-think how to implement it.<br>
><br>
> Generally it's a good thing to have self sufficient comments, so you are<br>
> absolutely right.<br>
> But sometimes there are long discussions associated in the mailing list<br>
> which are hard to transcribe.<br>
> A reference to a bug report containing some discussion could be a good thing<br>
> in this case, ...<br>
> ... but we're getting lazy and have a lighter trunk process for some years.<br>
> Can we have the cake and eat it too?<br>
> I know this is not a good excuse, but you must also consider that after 10<br>
> commits on the same subject, the saturated commiter's mind is getting<br>
> facetious ;)<br>
> That's my case at least.<br>
><br>
><br>
><br>
> 2013/9/30 Chris Muller <<a href="mailto:asqueaker@gmail.com">asqueaker@gmail.com</a>><br>
>><br>
>> These version comments are redundant with the code-diff; we can see<br>
>> that 'presenatation' vars are being removed, so could you also please<br>
>> explain _why_ changes are being made in version comments?<br>
>><br>
>> Is this a cleanup? And may we expect no impact to any paragraph /<br>
>> text-editor behaviors?<br>
>><br>
>> On Tue, Sep 24, 2013 at 3:40 PM, <<a href="mailto:commits@source.squeak.org">commits@source.squeak.org</a>> wrote:<br>
>> > Nicolas Cellier uploaded a new version of Morphic to project The Trunk:<br>
>> > <a href="http://source.squeak.org/trunk/Morphic-nice.687.mcz" target="_blank">http://source.squeak.org/trunk/Morphic-nice.687.mcz</a><br>
>> ><br>
>> > ==================== Summary ====================<br>
>> ><br>
>> > Name: Morphic-nice.687<br>
>> > Author: nice<br>
>> > Time: 24 September 2013, 10:40:04.266 pm<br>
>> > UUID: a0a8b7f9-f0a3-4071-b95b-06124aa146a4<br>
>> > Ancestors: Morphic-nice.686<br>
>> ><br>
>> > Expunge presentation* from MultiNewParagraph & MultiTextComposer.<br>
>> > Also remove those methods same as super.<br>
>> > Note the small difference in displaying method: NewPragraph has offset<br>
>> > truncated.<br>
>> > Prepare MultiNewParagraph funerals by mutating its instances -><br>
>> > NewParagraph<br>
>> ><br>
>> > =============== Diff against Morphic-nice.686 ===============<br>
>> ><br>
>> > Item was changed:<br>
>> > NewParagraph subclass: #MultiNewParagraph<br>
>> > + instanceVariableNames: ''<br>
>> > - instanceVariableNames: 'presentationText presentationLines'<br>
>> > classVariableNames: ''<br>
>> > poolDictionaries: 'TextConstants'<br>
>> > category: 'Morphic-Multilingual'!<br>
>> ><br>
>> > Item was added:<br>
>> > + ----- Method: MultiNewParagraph class>>initialize (in category 'class<br>
>> > initialization') -----<br>
>> > + initialize<br>
>> > + "Prepare our own funeral"<br>
>> > +<br>
>> > + MultiNewParagraph allInstancesDo:[:mnp| mnp becomeForward: (mnp<br>
>> > as: NewParagraph)]!<br>
>> ><br>
>> > Item was removed:<br>
>> > - ----- Method:<br>
>> > MultiNewParagraph>>multiComposeLinesFrom:to:delta:into:priorLines:atY: (in<br>
>> > category 'composition') -----<br>
>> > - multiComposeLinesFrom: start to: stop delta: delta into: lineColl<br>
>> > priorLines: priorLines<br>
>> > - atY: startingY<br>
>> > - "While the section from start to stop has changed, composition<br>
>> > may ripple all the way to the end of the text. However in a rectangular<br>
>> > container, if we ever find a line beginning with the same character as<br>
>> > before (ie corresponding to delta in the old lines), then we can just copy<br>
>> > the old lines from there to the end of the container, with adjusted indices<br>
>> > and y-values"<br>
>> > -<br>
>> > - | newResult composer presentationInfo |<br>
>> > -<br>
>> > - composer := MultiTextComposer new.<br>
>> > - presentationLines := nil.<br>
>> > - presentationText := nil.<br>
>> > - newResult := composer<br>
>> > - multiComposeLinesFrom: start<br>
>> > - to: stop<br>
>> > - delta: delta<br>
>> > - into: lineColl<br>
>> > - priorLines: priorLines<br>
>> > - atY: startingY<br>
>> > - textStyle: textStyle<br>
>> > - text: text<br>
>> > - container: container<br>
>> > - wantsColumnBreaks: wantsColumnBreaks == true.<br>
>> > - lines := newResult first asArray.<br>
>> > - maxRightX := newResult second.<br>
>> > - presentationInfo := composer getPresentationInfo.<br>
>> > - presentationLines := presentationInfo first asArray.<br>
>> > - presentationText := presentationInfo second.<br>
>> > - "maxRightX printString displayAt: 0@0."<br>
>> > - ^maxRightX<br>
>> > - !<br>
>> ><br>
>> > Item was removed:<br>
>> > - ----- Method: MultiNewParagraph>>presentationLines (in category<br>
>> > 'accessing') -----<br>
>> > - presentationLines<br>
>> > -<br>
>> > - ^ presentationLines.<br>
>> > - !<br>
>> ><br>
>> > Item was removed:<br>
>> > - ----- Method: MultiNewParagraph>>presentationText (in category<br>
>> > 'accessing') -----<br>
>> > - presentationText<br>
>> > -<br>
>> > - ^ presentationText.<br>
>> > - !<br>
>> ><br>
>> > Item was changed:<br>
>> > TextComposer subclass: #MultiTextComposer<br>
>> > + instanceVariableNames: ''<br>
>> > - instanceVariableNames: 'presentation presentationLines'<br>
>> > classVariableNames: ''<br>
>> > poolDictionaries: 'TextConstants'<br>
>> > category: 'Morphic-Multilingual'!<br>
>> ><br>
>> > Item was removed:<br>
>> > - ----- Method: MultiTextComposer>>composeEachRectangleIn: (in category<br>
>> > 'as yet unclassified') -----<br>
>> > - composeEachRectangleIn: rectangles<br>
>> > -<br>
>> > - | myLine lastChar |<br>
>> > -<br>
>> > - 1 to: rectangles size do: [:i |<br>
>> > - currCharIndex <= theText size ifFalse: [^false].<br>
>> > - myLine := scanner<br>
>> > - composeFrom: currCharIndex<br>
>> > - inRectangle: (rectangles at: i)<br>
>> > - firstLine: isFirstLine<br>
>> > - leftSide: i=1<br>
>> > - rightSide: i=rectangles size.<br>
>> > - lines addLast: myLine.<br>
>> > - presentationLines addLast: scanner getPresentationLine.<br>
>> > - presentation ifNil: [presentation := scanner<br>
>> > getPresentation]<br>
>> > - ifNotNil: [presentation := presentation, scanner<br>
>> > getPresentation].<br>
>> > - actualHeight := actualHeight max: myLine lineHeight.<br>
>> > "includes font changes"<br>
>> > - currCharIndex := myLine last + 1.<br>
>> > - lastChar := theText at: myLine last.<br>
>> > - (CharacterSet crlf includes: lastChar) ifTrue: [^#cr].<br>
>> > - wantsColumnBreaks ifTrue: [<br>
>> > - lastChar = TextComposer characterForColumnBreak<br>
>> > ifTrue: [^#columnBreak].<br>
>> > - ].<br>
>> > - ].<br>
>> > - ^false!<br>
>> ><br>
>> > Item was removed:<br>
>> > - ----- Method: MultiTextComposer>>getPresentationInfo (in category 'as<br>
>> > yet unclassified') -----<br>
>> > - getPresentationInfo<br>
>> > -<br>
>> > - ^ Array with: presentationLines with: presentation.<br>
>> > - !<br>
>> ><br>
>> > Item was changed:<br>
>> > ----- Method:<br>
>> > MultiTextComposer>>multiComposeLinesFrom:to:delta:into:priorLines:atY:textStyle:text:container:wantsColumnBreaks:<br>
>> > (in category 'as yet unclassified') -----<br>
>> > multiComposeLinesFrom: argStart to: argStop delta: argDelta into:<br>
>> > argLinesCollection priorLines: argPriorLines atY: argStartY textStyle:<br>
>> > argTextStyle text: argText container: argContainer wantsColumnBreaks:<br>
>> > argWantsColumnBreaks<br>
>> ><br>
>> > wantsColumnBreaks := argWantsColumnBreaks.<br>
>> > lines := argLinesCollection.<br>
>> > - presentationLines := argLinesCollection copy.<br>
>> > theTextStyle := argTextStyle.<br>
>> > theText := argText.<br>
>> > theContainer := argContainer.<br>
>> > deltaCharIndex := argDelta.<br>
>> > currCharIndex := startCharIndex := argStart.<br>
>> > stopCharIndex := argStop.<br>
>> > prevLines := argPriorLines.<br>
>> > currentY := argStartY.<br>
>> > defaultLineHeight := theTextStyle lineGrid.<br>
>> > maxRightX := theContainer left.<br>
>> > possibleSlide := stopCharIndex < theText size and: [theContainer<br>
>> > isMemberOf: Rectangle].<br>
>> > nowSliding := false.<br>
>> > prevIndex := 1.<br>
>> > scanner := MultiCompositionScanner new text: theText textStyle:<br>
>> > theTextStyle.<br>
>> > scanner wantsColumnBreaks: wantsColumnBreaks.<br>
>> > isFirstLine := true.<br>
>> > self composeAllLines.<br>
>> > isFirstLine ifTrue: ["No space in container or empty text"<br>
>> > self<br>
>> > addNullLineWithIndex: startCharIndex<br>
>> > andRectangle: (theContainer topLeft extent:<br>
>> > 0@defaultLineHeight)<br>
>> > ] ifFalse: [<br>
>> > self fixupLastLineIfCR<br>
>> > ].<br>
>> > ^{lines asArray. maxRightX}<br>
>> ><br>
>> > !<br>
>> ><br>
>> > Item was removed:<br>
>> > - ----- Method: NewParagraph>>testNewComposeAll3 (in category<br>
>> > 'composition') -----<br>
>> > - testNewComposeAll3<br>
>> > - | newResult |<br>
>> > - newResult := TextComposer new<br>
>> > - multiComposeLinesFrom: firstCharacterIndex<br>
>> > - to: text size<br>
>> > - delta: 0<br>
>> > - into: OrderedCollection new<br>
>> > - priorLines: Array new<br>
>> > - atY: container top<br>
>> > - textStyle: textStyle<br>
>> > - text: text<br>
>> > - container: (0@0 extent: 31@60)<br>
>> > - wantsColumnBreaks: false.<br>
>> > - ^{newResult. {lines. maxRightX}}<br>
>> > - !<br>
>> ><br>
>> ><br>
>><br>
><br>
><br>
><br>
><br>
<br>
</div></div></blockquote></div><br></div>