Ah, to be complete I forgot to say farewell multiComposeSomething, since it
was an unused sender of fixupLastLineIfCR that I wanted to remove too...

If you want to see the bug before this change, then:
open a Workspace, type M space space space ...
.. until the cursor wraps to... the beginning of first line.
type one more space and it skips on the next line.

What happens it's damn simple:
- the composition scanner crossedX
- the space that crossedX is inserted in the first line (like a CR would)
- there is no more character in the text
- the TextComposer has finished to composeLines and ^nil
- it tries to fixupLastLineIfCR, unnfortunately last char behaved as a CR
but wasn't a CR

Composition finishes with a single line when there should be logically an
empty second line...

Chris: I hope the comments carry enough intention :)


2013/10/2 <commits at source.squeak.org>

> Nicolas Cellier uploaded a new version of Graphics to project The Trunk:
> http://source.squeak.org/trunk/Graphics-nice.243.mcz
>
> ==================== Summary ====================
>
> Name: Graphics-nice.243
> Author: nice
> Time: 2 October 2013, 2:47:50.925 am
> UUID: 220dcb9f-1fed-4f7d-b6de-170fe2b2a3ce
> Ancestors: Graphics-nice.242
>
> Fix a composition glitch when the last character of a text is a space that
> crosses the right margin boundary.
> In such case, a virtual empty line must be added to the composition in
> order to correctly materialize text selection and cursor position, and so
> as to continue typing on next line.
>
> The case when last character is a carriage return is in all point similar.
> Indeed, a space that crossedX is visually turned into a new line.
> TextComposer previously tried to reverse engineer scanner's work to
> recognize the CR case, which is a smell.
> This change unifies handling for the two cases by rather asking to the
> scanner doesTheLineBreaksAfterLastChar?
> Remove fixupLastLineIfCR which is tainted with half case only.
> Remove the workaround in CharacterBlockScanner that did not work around
> anything.
>
> Fix the breaking at non space for eastern asia:
> 1) registerBreakableIndex records that the line can wrap before the
> current character, and spaceIndex was pointing at this character that will
> potentially wrap on next line.
> 2) It is still possible to apply Justified alignment based on space
> adjustment if some spaces are used in the text, so correctly set the line
> spaceCount and paddingWidth.
>
> =============== Diff against Graphics-nice.242 ===============
>
> Item was changed:
>   ----- Method: CharacterBlockScanner>>crossedX (in category 'stop
> conditions') -----
>   crossedX
>         "Text display has wrapping. The scanner just found a character
> past the x
>         location of the cursor. We know that the cursor is pointing at a
> character
>         or before one."
>
>         self retrieveLastCharacterWidth.
>
> -       characterIndex == nil ifFalse: [
> -               "If the last character of the last line is a space,
> -               and it crosses the right margin, then locating
> -               the character block after it is impossible without this
> hack."
> -               characterIndex > text size ifTrue: [
> -                       lastIndex := characterIndex.
> -                       characterPoint := (nextLeftMargin ifNil:
> [leftMargin]) @ (destY + line lineHeight).
> -                       ^true]].
>         characterPoint x <= (destX + (lastCharacterWidth // 2))
>                 ifTrue: [characterPoint := destX @ destY.
>                                 ^true].
>         lastIndex >= line last
>                 ifTrue: [characterPoint := destX @ destY.
>                                 ^true].
>
>         "Pointing past middle of a character, return the next character."
>         lastIndex := lastIndex + 1.
>         characterPoint := destX + lastCharacterWidth + kern @ destY.
>         ^ true!
>
> Item was changed:
>   CharacterScanner subclass: #CompositionScanner
> +       instanceVariableNames: 'spaceX spaceIndex lineHeight baseline
> lineHeightAtSpace baselineAtSpace lastBreakIsNotASpace
> nextIndexAfterLineBreak'
> -       instanceVariableNames: 'spaceX spaceIndex lineHeight baseline
> lineHeightAtSpace baselineAtSpace lastBreakIsNotASpace'
>         classVariableNames: ''
>         poolDictionaries: ''
>         category: 'Graphics-Text'!
>
>   !CompositionScanner commentStamp: '<historical>' prior: 0!
>   CompositionScanners are used to measure text and determine where line
> breaks and space padding should occur.!
>
> Item was changed:
>   ----- Method:
> CompositionScanner>>composeFrom:inRectangle:firstLine:leftSide:rightSide:
> (in category 'scanning') -----
>   composeFrom: startIndex inRectangle: lineRectangle
>         firstLine: firstLine leftSide: leftSide rightSide: rightSide
>         "Answer an instance of TextLineInterval that represents the next
> line in the paragraph."
>         | runLength stopCondition |
>         "Set up margins"
>         leftMargin := lineRectangle left.
>         leftSide ifTrue: [leftMargin := leftMargin +
>                                                 (firstLine ifTrue:
> [textStyle firstIndent]
>                                                                 ifFalse:
> [textStyle restIndent])].
>         destX := spaceX := leftMargin.
>         rightMargin := lineRectangle right.
>         rightSide ifTrue: [rightMargin := rightMargin - textStyle
> rightIndent].
>         lastIndex := startIndex.        "scanning sets last index"
>         destY := lineRectangle top.
>         lineHeight := baseline := 0.  "Will be increased by setFont"
>         line := (TextLine start: lastIndex stop: 0 internalSpaces: 0
> paddingWidth: 0)
>                                 rectangle: lineRectangle.
>         self setStopConditions. "also sets font"
>         runLength := text runLengthFor: startIndex.
>         runStopIndex := (lastIndex := startIndex) + (runLength - 1).
> +       nextIndexAfterLineBreak := spaceCount := 0.
> -       spaceCount := 0.
>         lastBreakIsNotASpace := false.
>         self handleIndentation.
>         leftMargin := destX.
>         line leftMargin: leftMargin.
>
>         [stopCondition := self scanCharactersFrom: lastIndex to:
> runStopIndex
>                 in: text string rightX: rightMargin stopConditions:
> stopConditions
>                 kern: kern.
>         "See setStopConditions for stopping conditions for composing."
>         self perform: stopCondition] whileFalse.
>
>         ^ line
>                 lineHeight: lineHeight + textStyle leading
>                 baseline: baseline + textStyle leading!
>
> Item was changed:
>   ----- Method:
> CompositionScanner>>composeLine:fromCharacterIndex:inParagraph: (in
> category 'scanning') -----
>   composeLine: lineIndex fromCharacterIndex: startIndex inParagraph:
> aParagraph
>         "Answer an instance of TextLineInterval that represents the next
> line in the paragraph."
>         | runLength stopCondition |
>         destX := spaceX := leftMargin := aParagraph
> leftMarginForCompositionForLine: lineIndex.
>         destY := 0.
>         rightMargin := aParagraph rightMarginForComposition.
>         leftMargin >= rightMargin ifTrue: [self error: 'No room between
> margins to compose'].
>         lastIndex := startIndex.        "scanning sets last index"
>         lineHeight := textStyle lineGrid.  "may be increased by
> setFont:..."
>         baseline := textStyle baseline.
>         self setStopConditions. "also sets font"
>         self handleIndentation.
>         runLength := text runLengthFor: startIndex.
>         runStopIndex := (lastIndex := startIndex) + (runLength - 1).
>         line := TextLineInterval
>                 start: lastIndex
>                 stop: 0
>                 internalSpaces: 0
>                 paddingWidth: 0.
> +       nextIndexAfterLineBreak := spaceCount := 0.
> -       spaceCount := 0.
>         lastBreakIsNotASpace := false.
>
>         [stopCondition := self scanCharactersFrom: lastIndex to:
> runStopIndex
>                 in: text string rightX: rightMargin stopConditions:
> stopConditions
>                 kern: kern.
>         "See setStopConditions for stopping conditions for composing."
>         self perform: stopCondition] whileFalse.
>
>         ^line
>                 lineHeight: lineHeight + textStyle leading
>                 baseline: baseline + textStyle leading!
>
> Item was changed:
>   ----- Method: CompositionScanner>>cr (in category 'stop conditions')
> -----
>   cr
>         "Answer true. Set up values for the text line interval currently
> being
>         composed."
>
>         pendingKernX := 0.
>         (lastIndex < text size and: [(text at: lastIndex) = CR and: [(text
> at: lastIndex+1) = Character lf]]) ifTrue: [lastIndex := lastIndex + 1].
>         line stop: lastIndex.
> +       nextIndexAfterLineBreak := lastIndex + 1.
>         spaceX := destX.
>         lastBreakIsNotASpace := false.
>         line paddingWidth: rightMargin - spaceX.
>         ^true!
>
> Item was changed:
>   ----- Method: CompositionScanner>>crossedX (in category 'stop
> conditions') -----
>   crossedX
>         "There is a word that has fallen across the right edge of the
> composition
>         rectangle. This signals the need for wrapping which is done to the
> last
>         space that was encountered, as recorded by the space stop
> condition,
>         or any other breakable character if the language permits so."
>
>         pendingKernX := 0.
>
>         lastBreakIsNotASpace ifTrue:
> +               ["In some languages break is possible before a non space."
> +               nextIndexAfterLineBreak := spaceIndex.
> +               line stop: spaceIndex - 1.
> -               ["In some languages break is possible on non space."
> -               line stop: spaceIndex.
>                 lineHeight := lineHeightAtSpace.
>                 baseline := baselineAtSpace.
> +               line paddingWidth: rightMargin - spaceX.
> -               spaceCount := spaceCount - 1.
> -               spaceIndex := spaceIndex - 1.
> -               line paddingWidth: rightMargin.
>                 line internalSpaces: spaceCount.
>                 ^true].
>
>         spaceCount >= 1 ifTrue:
>                 ["The common case. First back off to the space at which we
> wrap."
>                 line stop: spaceIndex.
> +               nextIndexAfterLineBreak := spaceIndex + 1.
>                 lineHeight := lineHeightAtSpace.
>                 baseline := baselineAtSpace.
>                 spaceCount := spaceCount - 1.
>                 spaceIndex := spaceIndex - 1.
>
>                 "Check to see if any spaces preceding the one at which we
> wrap.
>                         Double space after punctuation, most likely."
>                 [(spaceCount > 1 and: [(text at: spaceIndex) = Space])]
>                         whileTrue:
>                                 [spaceCount := spaceCount - 1.
>                                 "Account for backing over a run which might
>                                         change width of space."
>                                 font := text fontAt: spaceIndex withStyle:
> textStyle.
>                                 spaceIndex := spaceIndex - 1.
>                                 spaceX := spaceX - (font widthOf: Space)].
>                 line paddingWidth: rightMargin - spaceX.
>                 line internalSpaces: spaceCount]
>         ifFalse:
>                 ["Neither internal nor trailing spaces -- almost never
> happens."
>                 lastIndex := lastIndex - 1.
>                 [destX <= rightMargin or: [ lastIndex = 0 ]]
>                         whileFalse:
>                                 [destX := destX - (font widthOf: (text at:
> lastIndex)).
>                                 lastIndex := lastIndex - 1].
> +               nextIndexAfterLineBreak := lastIndex + 1.
>                 spaceX := destX.
>                 line paddingWidth: rightMargin - destX.
>                 line stop: (lastIndex max: line first)].
>         ^true!
>
> Item was added:
> + ----- Method: CompositionScanner>>doesTheLineBreaksAfterLastChar (in
> category 'accessing') -----
> + doesTheLineBreaksAfterLastChar
> +       ^nextIndexAfterLineBreak > text size!
>
> Item was changed:
>   ----- Method:
> TextComposer>>composeLinesFrom:to:delta:into:priorLines:atY:textStyle:text:container:wantsColumnBreaks:
> (in category 'as yet unclassified') -----
>   composeLinesFrom: argStart to: argStop delta: argDelta into:
> argLinesCollection priorLines: argPriorLines atY: argStartY textStyle:
> argTextStyle text: argText container: argContainer wantsColumnBreaks:
> argWantsColumnBreaks
>
>         wantsColumnBreaks := argWantsColumnBreaks.
>         lines := argLinesCollection.
>         theTextStyle := argTextStyle.
>         theText := argText.
>         theContainer := argContainer.
>         deltaCharIndex := argDelta.
>         currCharIndex := startCharIndex := argStart.
>         stopCharIndex := argStop.
>         prevLines := argPriorLines.
>         currentY := argStartY.
>         maxRightX := theContainer left.
>         possibleSlide := stopCharIndex < theText size and: [theContainer
> isMemberOf: Rectangle].
>         nowSliding := false.
>         prevIndex := 1.
>         "choose an appropriate scanner - should go away soon, when they
> can be unified"
>         scanner := CompositionScanner new.
>         scanner text: theText textStyle: theTextStyle.
>         scanner wantsColumnBreaks: wantsColumnBreaks.
>         defaultLineHeight := scanner computeDefaultLineHeight.
>         isFirstLine := true.
>         self composeAllLines.
>         isFirstLine ifTrue: ["No space in container or empty text"
>                 self
>                         addNullLineWithIndex: startCharIndex
>                         andRectangle: (theContainer topLeft extent:
> 0 at defaultLineHeight)
>         ] ifFalse: [
> +               (lines last last = theText size and: [scanner
> doesTheLineBreaksAfterLastChar])
> +                       ifTrue: [self addNullLineForIndex: theText size +
> 1]
> -               self fixupLastLineIfCR
>         ].
>         ^{lines asArray. maxRightX}
> -
>   !
>
> Item was removed:
> - ----- Method: TextComposer>>fixupLastLineIfCR (in category 'as yet
> unclassified') -----
> - fixupLastLineIfCR
> - "This awful bit is to ensure that if we have scanned all the text and
> the last character is a CR that there is a null line at the end of lines.
> Sometimes this was not happening which caused anomalous selections when
> selecting all the text. This is implemented as a post-composition fixup
> because I couldn't figure out where to put it in the main logic."
> -
> -       (theText size > 0 and: [CharacterSet crlf includes: theText last])
> ifFalse: [^self].
> -       self addNullLineForIndex: theText size + 1.
> - !
>
> Item was removed:
> - ----- Method:
> TextComposer>>multiComposeLinesFrom:to:delta:into:priorLines:atY:textStyle:text:container:wantsColumnBreaks:
> (in category 'as yet unclassified') -----
> - multiComposeLinesFrom: argStart to: argStop delta: argDelta into:
> argLinesCollection priorLines: argPriorLines atY: argStartY textStyle:
> argTextStyle text: argText container: argContainer wantsColumnBreaks:
> argWantsColumnBreaks
> -
> - "temporarily add this here to support move to drop MultiTextComposer"
> - "now redundant and ready to remove later"
> -       wantsColumnBreaks := argWantsColumnBreaks.
> -       lines := argLinesCollection.
> -       theTextStyle := argTextStyle.
> -       theText := argText.
> -       theContainer := argContainer.
> -       deltaCharIndex := argDelta.
> -       currCharIndex := startCharIndex := argStart.
> -       stopCharIndex := argStop.
> -       prevLines := argPriorLines.
> -       currentY := argStartY.
> -       maxRightX := theContainer left.
> -       possibleSlide := stopCharIndex < theText size and: [theContainer
> isMemberOf: Rectangle].
> -       nowSliding := false.
> -       prevIndex := 1.
> -       scanner := CompositionScanner new text: theText textStyle:
> theTextStyle.
> -       scanner wantsColumnBreaks: wantsColumnBreaks.
> -       defaultLineHeight := scanner computeDefaultLineHeight.
> -       isFirstLine := true.
> -       self composeAllLines.
> -       isFirstLine ifTrue: ["No space in container or empty text"
> -               self
> -                       addNullLineWithIndex: startCharIndex
> -                       andRectangle: (theContainer topLeft extent:
> 0 at defaultLineHeight)
> -       ] ifFalse: [
> -               self fixupLastLineIfCR
> -       ].
> -       ^{lines asArray. maxRightX}
> -
> - !
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20131002/d3d43da3/attachment-0001.htm