From 3551780cdf953a0f275858663761bc9973015321 Mon Sep 17 00:00:00 2001 From: Nikolaus Wittenstein Date: Tue, 27 Jan 2015 10:34:06 -0500 Subject: [PATCH 1/4] Marker::setHead/TailScreenPosition no longer clip unnecessarily DisplayBuffer::bufferPositionForScreenPosition calls clipScreenPosition first thing, meaning that Marker::setHead/TailScreenPosition don't need to call clipScreenPosition before calling bufferPositionForScreenPosition. --- src/marker.coffee | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/marker.coffee b/src/marker.coffee index 22460c1f8..962ebb7e7 100644 --- a/src/marker.coffee +++ b/src/marker.coffee @@ -286,7 +286,6 @@ class Marker # * `screenPosition` The new {Point} to use # * `properties` (optional) {Object} properties to associate with the marker. setHeadScreenPosition: (screenPosition, properties) -> - screenPosition = @displayBuffer.clipScreenPosition(screenPosition, properties) @setHeadBufferPosition(@displayBuffer.bufferPositionForScreenPosition(screenPosition, properties)) # Extended: Retrieves the buffer position of the marker's tail. @@ -313,7 +312,6 @@ class Marker # * `screenPosition` The new {Point} to use # * `properties` (optional) {Object} properties to associate with the marker. setTailScreenPosition: (screenPosition, options) -> - screenPosition = @displayBuffer.clipScreenPosition(screenPosition, options) @setTailBufferPosition(@displayBuffer.bufferPositionForScreenPosition(screenPosition, options)) # Extended: Returns a {Boolean} indicating whether the marker has a tail. From 372fb49c88599ccf4e28bb821fa8dd9e2b7f94af Mon Sep 17 00:00:00 2001 From: Nikolaus Wittenstein Date: Tue, 27 Jan 2015 14:42:00 -0500 Subject: [PATCH 2/4] TokenizedLine::screenColumnForBufferColumn calculates more accurately screenColumnForBufferColumn used to break only if the current column was strictly greater than the target column. This commit changes it so it breaks when greater or equal, which is how bufferColumnForScreenColumn works. This also adds some unit tests for screenColumnForBufferColumn's interactions with hard tab characters. --- spec/display-buffer-spec.coffee | 8 ++++++++ src/tokenized-line.coffee | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/spec/display-buffer-spec.coffee b/spec/display-buffer-spec.coffee index 2893fd5a4..7635f5074 100644 --- a/spec/display-buffer-spec.coffee +++ b/spec/display-buffer-spec.coffee @@ -719,6 +719,14 @@ describe "DisplayBuffer", -> expect(displayBuffer.screenPositionForBufferPosition([100000, 0])).toEqual [12, 2] expect(displayBuffer.screenPositionForBufferPosition([100000, 100000])).toEqual [12, 2] + it "clips to the (left or right) edge of an atomic token without simply rounding up", -> + tabLength = 4 + displayBuffer.setTabLength(tabLength) + + buffer.insert([0, 0], '\t') + expect(displayBuffer.screenPositionForBufferPosition([0, 0])).toEqual [0, 0] + expect(displayBuffer.screenPositionForBufferPosition([0, 1])).toEqual [0, tabLength] + describe "position translation in the presence of hard tabs", -> it "correctly translates positions on either side of a tab", -> buffer.setText('\t') diff --git a/src/tokenized-line.coffee b/src/tokenized-line.coffee index 11badc859..a6209e210 100644 --- a/src/tokenized-line.coffee +++ b/src/tokenized-line.coffee @@ -67,7 +67,7 @@ class TokenizedLine screenColumn = 0 currentBufferColumn = 0 for token in @tokens - break if currentBufferColumn > bufferColumn + break if currentBufferColumn + token.bufferDelta > bufferColumn screenColumn += token.screenDelta currentBufferColumn += token.bufferDelta @clipScreenColumn(screenColumn + (bufferColumn - currentBufferColumn)) From 5a3f2035a1f7a7820eb01dc569a5d0ec5fcbcb8f Mon Sep 17 00:00:00 2001 From: Nikolaus Wittenstein Date: Tue, 27 Jan 2015 15:14:15 -0500 Subject: [PATCH 3/4] Replace skipAtomicTokens with clip When clipping a screen position, callers used to have to pick between clipping to the left edge or the right edge when the position was in the middle of an atomic token. This change allows them to choose the closest edge, and makes this the default. This makes selecting hard tabs (or any other atomic tokens) work in a similar manner as in other text editors; that is, when clicking near the middle of a tab, the insertion point will move to the closest edge rather than the left edge. --- spec/display-buffer-spec.coffee | 26 +++++++++++++++++++------- src/cursor.coffee | 4 ++-- src/selection.coffee | 2 +- src/tokenized-line.coffee | 21 ++++++++++++++++++--- 4 files changed, 40 insertions(+), 13 deletions(-) diff --git a/spec/display-buffer-spec.coffee b/spec/display-buffer-spec.coffee index 7635f5074..4d46f3da7 100644 --- a/spec/display-buffer-spec.coffee +++ b/spec/display-buffer-spec.coffee @@ -638,8 +638,11 @@ describe "DisplayBuffer", -> expect(displayBuffer.outermostFoldsInBufferRowRange(3, 18)).toEqual [fold1, fold3, fold5] expect(displayBuffer.outermostFoldsInBufferRowRange(5, 16)).toEqual [fold3] - describe "::clipScreenPosition(screenPosition, wrapBeyondNewlines: false, wrapAtSoftNewlines: false, skipAtomicTokens: false)", -> + describe "::clipScreenPosition(screenPosition, wrapBeyondNewlines: false, wrapAtSoftNewlines: false, clip: 'closest')", -> beforeEach -> + tabLength = 4 + + displayBuffer.setTabLength(tabLength) displayBuffer.setSoftWrapped(true) displayBuffer.setEditorWidthInChars(50) @@ -698,19 +701,28 @@ describe "DisplayBuffer", -> expect(displayBuffer.clipScreenPosition([3, 58], wrapAtSoftNewlines: true)).toEqual [4, 4] expect(displayBuffer.clipScreenPosition([3, 1000], wrapAtSoftNewlines: true)).toEqual [4, 4] - describe "when skipAtomicTokens is false (the default)", -> - it "clips screen positions in the middle of atomic tab characters to the beginning of the character", -> + describe "when clip is 'closest' (the default)", -> + it "clips screen positions in the middle of atomic tab characters to the closest edge of the character", -> buffer.insert([0, 0], '\t') expect(displayBuffer.clipScreenPosition([0, 0])).toEqual [0, 0] expect(displayBuffer.clipScreenPosition([0, 1])).toEqual [0, 0] + expect(displayBuffer.clipScreenPosition([0, 2])).toEqual [0, 0] + expect(displayBuffer.clipScreenPosition([0, tabLength-1])).toEqual [0, tabLength] expect(displayBuffer.clipScreenPosition([0, tabLength])).toEqual [0, tabLength] - describe "when skipAtomicTokens is true", -> + describe "when clip is 'backward'", -> + it "clips screen positions in the middle of atomic tab characters to the beginning of the character", -> + buffer.insert([0, 0], '\t') + expect(displayBuffer.clipScreenPosition([0, 0], clip: 'backward')).toEqual [0, 0] + expect(displayBuffer.clipScreenPosition([0, tabLength-1], clip: 'backward')).toEqual [0, 0] + expect(displayBuffer.clipScreenPosition([0, tabLength], clip: 'backward')).toEqual [0, tabLength] + + describe "when clip is 'forward'", -> it "clips screen positions in the middle of atomic tab characters to the end of the character", -> buffer.insert([0, 0], '\t') - expect(displayBuffer.clipScreenPosition([0, 0], skipAtomicTokens: true)).toEqual [0, 0] - expect(displayBuffer.clipScreenPosition([0, 1], skipAtomicTokens: true)).toEqual [0, tabLength] - expect(displayBuffer.clipScreenPosition([0, tabLength], skipAtomicTokens: true)).toEqual [0, tabLength] + expect(displayBuffer.clipScreenPosition([0, 0], clip: 'forward')).toEqual [0, 0] + expect(displayBuffer.clipScreenPosition([0, 1], clip: 'forward')).toEqual [0, tabLength] + expect(displayBuffer.clipScreenPosition([0, tabLength], clip: 'forward')).toEqual [0, tabLength] describe "::screenPositionForBufferPosition(bufferPosition, options)", -> it "clips the specified buffer position", -> diff --git a/src/cursor.coffee b/src/cursor.coffee index de311358b..c7db4b04a 100644 --- a/src/cursor.coffee +++ b/src/cursor.coffee @@ -305,7 +305,7 @@ class Cursor extends Model columnCount-- # subtract 1 for the row move column = column - columnCount - @setScreenPosition({row, column}) + @setScreenPosition({row, column}, clip: 'backward') # Public: Moves the cursor right one screen column. # @@ -332,7 +332,7 @@ class Cursor extends Model columnsRemainingInLine = rowLength column = column + columnCount - @setScreenPosition({row, column}, skipAtomicTokens: true, wrapBeyondNewlines: true, wrapAtSoftNewlines: true) + @setScreenPosition({row, column}, clip: 'forward', wrapBeyondNewlines: true, wrapAtSoftNewlines: true) # Public: Moves the cursor to the top of the buffer. moveToTop: -> diff --git a/src/selection.coffee b/src/selection.coffee index 0bee6f7de..ea4b54034 100644 --- a/src/selection.coffee +++ b/src/selection.coffee @@ -393,7 +393,7 @@ class Selection extends Model if options.select @setBufferRange(newBufferRange, reversed: wasReversed) else - @cursor.setBufferPosition(newBufferRange.end, skipAtomicTokens: true) if wasReversed + @cursor.setBufferPosition(newBufferRange.end, clip: 'forward') if wasReversed if autoIndentFirstLine @editor.setIndentationForBufferRow(oldBufferRange.start.row, desiredIndentLevel) diff --git a/src/tokenized-line.coffee b/src/tokenized-line.coffee index a6209e210..074e7a785 100644 --- a/src/tokenized-line.coffee +++ b/src/tokenized-line.coffee @@ -41,10 +41,20 @@ class TokenizedLine copy: -> new TokenizedLine({@tokens, @lineEnding, @ruleStack, @startBufferColumn, @fold}) + # This clips a given screen column to a valid column that's within the line + # and not in the middle of any atomic tokens. + # + # column - A {Number} representing the column to clip + # options - A hash with the key clip. Valid values for this key: + # 'closest' (default): clip to the closest edge of an atomic token. + # 'forward': clip to the forward edge. + # 'backward': clip to the backward edge. + # + # Returns a {Number} representing the clipped column. clipScreenColumn: (column, options={}) -> return 0 if @tokens.length == 0 - { skipAtomicTokens } = options + { clip } = options column = Math.min(column, @getMaxScreenColumn()) tokenStartColumn = 0 @@ -55,10 +65,15 @@ class TokenizedLine if @isColumnInsideSoftWrapIndentation(tokenStartColumn) @softWrapIndentationDelta else if token.isAtomic and tokenStartColumn < column - if skipAtomicTokens + if clip == 'forward' tokenStartColumn + token.screenDelta - else + else if clip == 'backward' tokenStartColumn + else #'closest' + if column > tokenStartColumn + (token.screenDelta / 2) + tokenStartColumn + token.screenDelta + else + tokenStartColumn else column From b28ee92896a770504732f3d081e4053e4f93105e Mon Sep 17 00:00:00 2001 From: Nikolaus Wittenstein Date: Mon, 23 Mar 2015 09:42:37 -0400 Subject: [PATCH 4/4] Add tests for DisplayBuffer::screenPositionForBufferPosition around soft tabs This makes sure that a buffer position in the middle of a soft tab will correctly clip to the closest edge by default. --- spec/display-buffer-spec.coffee | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/display-buffer-spec.coffee b/spec/display-buffer-spec.coffee index 4d46f3da7..93460f173 100644 --- a/spec/display-buffer-spec.coffee +++ b/spec/display-buffer-spec.coffee @@ -739,6 +739,17 @@ describe "DisplayBuffer", -> expect(displayBuffer.screenPositionForBufferPosition([0, 0])).toEqual [0, 0] expect(displayBuffer.screenPositionForBufferPosition([0, 1])).toEqual [0, tabLength] + it "clips to the edge closest to the given position when it's inside a soft tab", -> + tabLength = 4 + displayBuffer.setTabLength(tabLength) + + buffer.insert([0, 0], ' ') + expect(displayBuffer.screenPositionForBufferPosition([0, 0])).toEqual [0, 0] + expect(displayBuffer.screenPositionForBufferPosition([0, 1])).toEqual [0, 0] + expect(displayBuffer.screenPositionForBufferPosition([0, 2])).toEqual [0, 0] + expect(displayBuffer.screenPositionForBufferPosition([0, 3])).toEqual [0, 4] + expect(displayBuffer.screenPositionForBufferPosition([0, 4])).toEqual [0, 4] + describe "position translation in the presence of hard tabs", -> it "correctly translates positions on either side of a tab", -> buffer.setText('\t')