From da46389070f745535d9f837fc110ccb333c77e85 Mon Sep 17 00:00:00 2001 From: Ryuichi Inagaki Date: Sun, 22 Jul 2018 19:39:08 +1000 Subject: [PATCH] ref #18 Fixed the issue that selecting the exact text doesn't unhighlight patterns --- lib/commands/toggle-highlight.js | 2 +- lib/commands/update-highlight.js | 2 +- lib/text-editor.js | 7 +++ lib/text-location-registry.js | 14 ++++-- .../features/highlight-selected-text.test.js | 29 ++++++++++-- test/acceptance/helpers/fake-editor.js | 23 +++++++-- test/unit/text-location-registry.test.js | 47 ++++++++++++------- 7 files changed, 93 insertions(+), 31 deletions(-) diff --git a/lib/commands/toggle-highlight.js b/lib/commands/toggle-highlight.js index 93ae4cd..dc6dbcd 100644 --- a/lib/commands/toggle-highlight.js +++ b/lib/commands/toggle-highlight.js @@ -12,7 +12,7 @@ class ToggleHighlightCommand { const textEditor = this._textEditorFactory.create(editor); const decorationId = this._textLocationRegistry.queryDecorationId({ editorId: textEditor.id, - offset: textEditor.cursorOffset + flatRange: textEditor.flatRange }); if (decorationId) { this._removeDecoration(decorationId); diff --git a/lib/commands/update-highlight.js b/lib/commands/update-highlight.js index 7ea5719..2c4100e 100644 --- a/lib/commands/update-highlight.js +++ b/lib/commands/update-highlight.js @@ -13,7 +13,7 @@ class UpdateHighlightCommand { const textEditor = this._textEditorFactory.create(editor); const decorationId = this._textLocationRegistry.queryDecorationId({ editorId: textEditor.id, - offset: textEditor.cursorOffset + flatRange: textEditor.flatRange }); if (!decorationId) return; diff --git a/lib/text-editor.js b/lib/text-editor.js index d52314c..f33bfc2 100644 --- a/lib/text-editor.js +++ b/lib/text-editor.js @@ -26,6 +26,13 @@ class TextEditor { return this._editor.document.offsetAt(this._editor.selection.active); } + get flatRange() { + return { + start: this._editor.document.offsetAt(this._editor.selection.start), + end: this._editor.document.offsetAt(this._editor.selection.end) + }; + } + setDecorations(decorationType, ranges) { const vsRanges = ranges.map(range => new this._VsRange( diff --git a/lib/text-location-registry.js b/lib/text-location-registry.js index 04f22a1..415445e 100644 --- a/lib/text-location-registry.js +++ b/lib/text-location-registry.js @@ -17,17 +17,23 @@ class TextLocationRegistry { }); } - queryDecorationId({editorId, offset}) { + queryDecorationId({editorId, flatRange}) { const decorationMap = this._recordMap.get(editorId); if (!decorationMap) return null; const [decorationId] = Array.from(decorationMap.entries()) - .find(([_decorationId, ranges]) => - ranges.some(range => range.start <= offset && offset <= range.end) - ) || []; + .find(([_decorationId, ranges]) => ranges.some(this._isPointingRange(flatRange))) || []; return decorationId || null; } + _isPointingRange(range2) { + return range1 => { + if (range2.start < range1.start || range2.end > range1.end) return false; + return range2.start === range2.end || + (range1.start === range2.start && range1.end === range2.end); + }; + } + } module.exports = TextLocationRegistry; diff --git a/test/acceptance/features/highlight-selected-text.test.js b/test/acceptance/features/highlight-selected-text.test.js index 70712c6..e05c6a2 100644 --- a/test/acceptance/features/highlight-selected-text.test.js +++ b/test/acceptance/features/highlight-selected-text.test.js @@ -7,13 +7,15 @@ suite('Highlight command', () => { let editor1; let editor2; + let editor3; let command; let fakeVscode; beforeEach(() => { editor1 = createFakeEditor({wholeText: 'A TEXT B TEXT C', selectedText: 'TEXT'}); editor2 = createFakeEditor({wholeText: 'a TEXT'}); - fakeVscode = createFakeVsCode({editors: [editor1, editor2]}); + editor3 = createFakeEditor({wholeText: 'a TEXT', selectedText: 'TEX'}); + fakeVscode = createFakeVsCode({editors: [editor1, editor2, editor3]}); AppIntegrator.create(fakeVscode, console).integrate(EXECUTION_CONTEXT); @@ -38,11 +40,30 @@ suite('Highlight command', () => { ]]); }); - test.skip('unhighlight selected text if the exact text is already selected', async () => { + test('add another highlight to the substring of already selected text', async () => { + await command(editor1); + await command(editor3); + + expect(editor1.setDecorations.args[1]).to.eql([ + 'DECORATION_TYPE_2', + [ + new fakeVscode.Range('POSITION:2', 'POSITION:5'), + new fakeVscode.Range('POSITION:9', 'POSITION:12') + ] + ]); + expect(editor3.setDecorations.args[1]).to.eql([ + 'DECORATION_TYPE_2', + [ + new fakeVscode.Range('POSITION:2', 'POSITION:5') + ] + ]); + }); + + test('unhighlight selected text if the exact text is already selected', async () => { await command(editor1); await command(editor1); - expect(editor1.setDecorations.args[1]).to.eql([['DECORATION_TYPE_1', []]]); - expect(editor2.setDecorations.args[1]).to.eql([['DECORATION_TYPE_1', []]]); + expect(editor1.setDecorations.args[1]).to.eql(['DECORATION_TYPE_1', []]); + expect(editor2.setDecorations.args[1]).to.eql(['DECORATION_TYPE_1', []]); }); }); diff --git a/test/acceptance/helpers/fake-editor.js b/test/acceptance/helpers/fake-editor.js index aa17d8e..fbe5d6c 100644 --- a/test/acceptance/helpers/fake-editor.js +++ b/test/acceptance/helpers/fake-editor.js @@ -1,13 +1,30 @@ exports.createFakeEditor = ({selectedText, wholeText} = {}) => { - const selection = selectedText ? 'SELECTION' : null; return { document: { getText: selection => selection ? selectedText : wholeText, - positionAt: position => `POSITION:${position}`, + positionAt: position, + offsetAt: offset, uri: 'EDITOR_ID' }, - selection, + selection: createSelection(selectedText, wholeText), setDecorations: sinon.spy() }; }; + +function createSelection(selectedText, wholeText) { + if (!selectedText) return null; + return { + start: position(wholeText.indexOf(selectedText)), + end: position(wholeText.indexOf(selectedText) + selectedText.length) + }; +} + +function position(offset) { + return `POSITION:${offset}`; +} + +function offset(position) { + const POS_PREFIX = 'POSITION:'; + return position.startsWith(POS_PREFIX) && parseInt(position.replace(POS_PREFIX, ''), 10); +} diff --git a/test/unit/text-location-registry.test.js b/test/unit/text-location-registry.test.js index 15a3834..6602e99 100644 --- a/test/unit/text-location-registry.test.js +++ b/test/unit/text-location-registry.test.js @@ -3,41 +3,52 @@ const TextLocationRegistry = require('../../lib/text-location-registry'); suite('TextLocationRegistry', () => { - test('it queries decoration id at given position', () => { - const registry = createRegistry(); - const decorationId = registry.queryDecorationId({editorId: 'EDITOR_ID', offset: 1}); + let registry; + + beforeEach(() => { + registry = new TextLocationRegistry(); + registry.register({ + editorId: 'EDITOR_ID', + decorationId: 'DECORATION_ID', + ranges: [range(0, 3)] + }); + }); + + test('treat the exact same range decoration the same decoration', () => { + const decorationId = registry.queryDecorationId({editorId: 'EDITOR_ID', flatRange: range(0, 3)}); + expect(decorationId).to.eql('DECORATION_ID'); + }); + + test('treat the included single point the same decoration', () => { + const decorationId = registry.queryDecorationId({editorId: 'EDITOR_ID', flatRange: range(1, 1)}); expect(decorationId).to.eql('DECORATION_ID'); }); + test('does not treat sub-ranged decoration the same decoration', () => { + const decorationId = registry.queryDecorationId({editorId: 'EDITOR_ID', flatRange: range(1, 2)}); + expect(decorationId).to.be.null; + }); + test('it returns null if no decoration ids are registered yet', () => { const registry = new TextLocationRegistry(); - const decorationId = registry.queryDecorationId({editorId: 'EDITOR_ID', offset: 5}); + const decorationId = registry.queryDecorationId({editorId: 'EDITOR_ID', flatRange: range(1, 1)}); expect(decorationId).to.be.null; }); test('it returns null if decoration is not found', () => { - const registry = createRegistry(); - const decorationId = registry.queryDecorationId({editorId: 'EDITOR_ID', offset: 5}); + const decorationId = registry.queryDecorationId({editorId: 'EDITOR_ID', flatRange: range(5, 5)}); expect(decorationId).to.be.null; }); test('it deregisters a decoration id and their positions', () => { - const registry = createRegistry(); - const decorationId = registry.queryDecorationId({editorId: 'EDITOR_ID', offset: 1}); + const decorationId = registry.queryDecorationId({editorId: 'EDITOR_ID', flatRange: range(1, 1)}); expect(decorationId).to.eql('DECORATION_ID'); registry.deregister(decorationId); - const decorationId2 = registry.queryDecorationId({editorId: 'EDITOR_ID', offset: 1}); + const decorationId2 = registry.queryDecorationId({editorId: 'EDITOR_ID', flatRange: range(1, 1)}); expect(decorationId2).to.be.null; }); - function createRegistry() { - const registry = new TextLocationRegistry(); - registry.register({ - editorId: 'EDITOR_ID', - decorationId: 'DECORATION_ID', - ranges: [{start: 0, end: 3}] - }); - return registry; + function range(start, end) { + return {start, end}; } - });