ref #18 Fixed the issue that selecting the exact text doesn't unhighl…

…ight patterns
ryu1kn · Jul 22, 2018 · da46389 · da46389
1 parent 6bc9bf3
commit da46389
Show file tree

Hide file tree

Showing 7 changed files with 93 additions and 31 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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};
     }
-
 });