From 581790760b9d1853fa86959c8f45dba42392630c Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 6 Sep 2016 17:33:14 +0200 Subject: [PATCH] Clip to next boundary when seeking iterator to the middle of a text tag Previously, when calling `TokenizedBufferIterator.seek` with a position that lied within a text tag, we advanced the iterator by the extent of that tag without, however, consuming it. Hence, when calling `moveToSuccessor` afterward, we would consume that tag and advance the iterator again, thus effectively moving it twice and making its position inaccurate. An option could be to clip to the left of the textual tag without consuming it. However, this would be a little odd with respect to the current contract between (`DisplayLayer` and) `seek`, whose promise is to move the iterator to a position that is greater or equal than the one asked by the caller. Therefore, with this commit, we are changing the behavior of `seek` in this particular scenario to consume the tag in question and process all its siblings until a tag boundary is finally found. This ensures that the above contract is always respected, while still preserving the "seek to leftmost tag boundary" semantics (i.e. notice how in the changed test case, calling `seek` with `Point(0, 1)` is the same as calling it with `Point(0, 3)`). --- spec/tokenized-buffer-iterator-spec.js | 38 ++++++++++++++++++-------- src/tokenized-buffer-iterator.coffee | 7 ++--- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/spec/tokenized-buffer-iterator-spec.js b/spec/tokenized-buffer-iterator-spec.js index df0676640..77e049b89 100644 --- a/spec/tokenized-buffer-iterator-spec.js +++ b/spec/tokenized-buffer-iterator-spec.js @@ -8,10 +8,14 @@ describe('TokenizedBufferIterator', () => { it('seeks to the leftmost tag boundary at the given position, returning the containing tags', function () { const tokenizedBuffer = { tokenizedLineForRow (row) { - return { - tags: [-1, -2, -3, -4, -5, 3, -3, -4, -6], - text: 'foo', - openScopes: [] + if (row === 0) { + return { + tags: [-1, -2, -3, -4, -5, 3, -3, -4, -6, 4], + text: 'foo bar', + openScopes: [] + } + } else { + return null } }, @@ -29,6 +33,7 @@ describe('TokenizedBufferIterator', () => { const iterator = new TokenizedBufferIterator(tokenizedBuffer) expect(iterator.seek(Point(0, 0))).toEqual([]) + expect(iterator.getPosition()).toEqual(Point(0, 0)) expect(iterator.getCloseTags()).toEqual([]) expect(iterator.getOpenTags()).toEqual(['foo']) @@ -37,19 +42,28 @@ describe('TokenizedBufferIterator', () => { expect(iterator.getOpenTags()).toEqual(['bar']) expect(iterator.seek(Point(0, 1))).toEqual(['baz']) + expect(iterator.getPosition()).toEqual(Point(0, 3)) expect(iterator.getCloseTags()).toEqual([]) + expect(iterator.getOpenTags()).toEqual(['bar']) + + iterator.moveToSuccessor() + expect(iterator.getPosition()).toEqual(Point(0, 3)) + expect(iterator.getCloseTags()).toEqual(['bar', 'baz']) + expect(iterator.getOpenTags()).toEqual([]) + + expect(iterator.seek(Point(0, 3))).toEqual(['baz']) + expect(iterator.getPosition()).toEqual(Point(0, 3)) + expect(iterator.getCloseTags()).toEqual([]) + expect(iterator.getOpenTags()).toEqual(['bar']) + + iterator.moveToSuccessor() + expect(iterator.getPosition()).toEqual(Point(0, 3)) + expect(iterator.getCloseTags()).toEqual(['bar', 'baz']) expect(iterator.getOpenTags()).toEqual([]) iterator.moveToSuccessor() + expect(iterator.getPosition()).toEqual(Point(1, 0)) expect(iterator.getCloseTags()).toEqual([]) - expect(iterator.getOpenTags()).toEqual(['bar']) - - expect(iterator.seek(Point(0, 3))).toEqual(['baz']) - expect(iterator.getCloseTags()).toEqual([]) - expect(iterator.getOpenTags()).toEqual(['bar']) - - iterator.moveToSuccessor() - expect(iterator.getCloseTags()).toEqual(['bar', 'baz']) expect(iterator.getOpenTags()).toEqual([]) }) }) diff --git a/src/tokenized-buffer-iterator.coffee b/src/tokenized-buffer-iterator.coffee index ffefc9609..23b72d5a9 100644 --- a/src/tokenized-buffer-iterator.coffee +++ b/src/tokenized-buffer-iterator.coffee @@ -21,21 +21,18 @@ class TokenizedBufferIterator for tag, index in @currentTags if tag >= 0 - if currentColumn is position.column + if currentColumn >= position.column @tagIndex = index break else currentColumn += tag @containingTags.pop() while @closeTags.shift() @containingTags.push(openTag) while openTag = @openTags.shift() - if currentColumn > position.column - @tagIndex = index - break else scopeName = @tokenizedBuffer.grammar.scopeForId(tag) if tag % 2 is 0 # close tag if @openTags.length > 0 - if currentColumn is position.column + if currentColumn >= position.column @tagIndex = index break else