From 44fd95ad0e1d2873fcd6408bd3f0f4c9b7232dcd Mon Sep 17 00:00:00 2001 From: Indrek Ardel Date: Tue, 7 May 2019 11:09:58 +0300 Subject: [PATCH] Stitch together multiple matches from single line --- lib/project/result-row-view.js | 129 ++++++++++++++++++++--------- lib/project/results-view.js | 4 +- spec/fixtures/project/combined.txt | 7 ++ spec/results-view-spec.js | 52 ++++++++++-- 4 files changed, 143 insertions(+), 49 deletions(-) create mode 100644 spec/fixtures/project/combined.txt diff --git a/lib/project/result-row-view.js b/lib/project/result-row-view.js index fbc0e9d7..e83bcd55 100644 --- a/lib/project/result-row-view.js +++ b/lib/project/result-row-view.js @@ -1,5 +1,5 @@ const getIconServices = require('../get-icon-services'); -const { Range } = require('atom'); +const { Point, Range } = require('atom'); const { LeadingContextRow, TrailingContextRow, @@ -111,57 +111,104 @@ class MatchRowView { } generatePreviewNode({matches, replacePattern, regex}) { - const subnodes = []; + const TYPE = { + MATCH: 1, + TEXT: 2, + ELLIPSIS: 3 + }; + + const segments = []; - let prevMatchEnd = matches[0].lineTextOffset; for (const match of matches) { - const range = Range.fromObject(match.range); - const prefixStart = Math.max(0, prevMatchEnd - match.lineTextOffset); - const matchStart = range.start.column - match.lineTextOffset; + if (segments.length) { + const previousRange = segments[segments.length - 1].range; + const currentRange = new Range( + new Point(0, match.lineTextOffset), + new Point(0, match.lineTextOffset + match.lineText.length) + ); + + if (previousRange.intersectsWith(currentRange)) { + const segment = segments.pop(); + // current range starts before previous range, therefore it should contain everything from previous range + if (previousRange.start.isGreaterThanOrEqual(currentRange.start)) { + // Delete everything up to previous match from the beginning of current range + match.lineText = match.lineText.substring(previousRange.start.column - currentRange.start.column); + match.lineTextOffset = previousRange.start.column; + } else { // current range starts midway through previous range + // Prepend non-overlapping part of previous range to current range + const preprefix = segment.content.substring(0, currentRange.start.column - previousRange.start.column); + match.lineText = preprefix + match.lineText; + match.lineTextOffset -= preprefix.length; + } + } else { // current range does not intersect and comes after previous range + segments.push({ type: TYPE.ELLIPSIS, range: new Range(previousRange.end, currentRange.start) }); + } + } - // TODO - Handle case where (prevMatchEnd < match.lineTextOffset) - // The solution probably needs Workspace.scan to be reworked to account - // for multiple matches lines first + const prefix = match.lineText.substring(0, match.range.start.column - match.lineTextOffset); + const suffix = match.lineText.substring(match.range.end.column - match.lineTextOffset); - const prefix = match.lineText.slice(prefixStart, matchStart); + if (prefix) { + segments.push({ + type: TYPE.TEXT, + content: prefix, + range: new Range( + new Point(0, match.lineTextOffset), + new Point(0, match.range.start.column) + ) + }); + } - let replacementText = '' - if (replacePattern && regex) { - replacementText = match.matchText.replace(regex, replacePattern); - } else if (replacePattern) { - replacementText = replacePattern; + let replacementText + if (replacePattern) { + replacementText = regex ? match.matchText.replace(regex, replacePattern) : replacePattern; } - subnodes.push( - $.span({}, prefix), - $.span( - { - className: - `match ${replacementText ? 'highlight-error' : 'highlight-info'}` - }, - match.matchText + segments.push({ + type: TYPE.MATCH, + content: match.matchText, + range: new Range( + new Point(0, match.range.start.column), + new Point(0, match.range.end.column) ), - $.span( - { - className: 'replacement highlight-success', - style: showIf(replacementText) - }, - replacementText - ) - ); - prevMatchEnd = range.end.column; + replacement: replacementText + }); + + if (suffix) { + segments.push({ + type: TYPE.TEXT, + content: suffix, + range: new Range( + new Point(0, match.range.end.column), + new Point(0, match.lineTextOffset + match.lineText.length) + ) + }); + } } - const lastMatch = matches[matches.length - 1]; - const suffix = lastMatch.lineText.slice( - prevMatchEnd - lastMatch.lineTextOffset - ); + const subnodes = []; - return $.span( - {className: 'preview'}, - ...subnodes, - $.span({}, suffix) - ); + for (const segment of segments) { + if (segment.type === TYPE.TEXT) { + subnodes.push($.span({}, segment.content)); + } + + if (segment.type === TYPE.MATCH) { + subnodes.push( + $.span({ className: `match ${segment.replacement ? 'highlight-error' : 'highlight-info'}` }, segment.content) + ); + + if (segment.replacement) { + subnodes.push($.span({ className: 'replacement highlight-success' }, segment.replacement)); + } + } + + if (segment.type === TYPE.ELLIPSIS) { + subnodes.push($.span({}, '…')); + } + } + + return $.span({ className: 'preview' }, ...subnodes); } render() { diff --git a/lib/project/results-view.js b/lib/project/results-view.js index 0ad1278a..ac0bebb1 100644 --- a/lib/project/results-view.js +++ b/lib/project/results-view.js @@ -1,4 +1,4 @@ -const { Range, CompositeDisposable, Disposable } = require('atom'); +const { Point, Range, CompositeDisposable, Disposable } = require('atom'); const ResultRowView = require('./result-row-view'); const { LeadingContextRow, @@ -48,7 +48,7 @@ class ResultsView { this.fakeGroup = new ResultRowGroup({ filePath: 'fake-file-path', matches: [{ - range: [[0, 1], [0, 2]], + range: new Range(new Point(0, 1), new Point(0, 2)), leadingContextLines: ['test-line-before'], trailingContextLines: ['test-line-after'], lineTextOffset: 1, diff --git a/spec/fixtures/project/combined.txt b/spec/fixtures/project/combined.txt new file mode 100644 index 00000000..44c1a2e9 --- /dev/null +++ b/spec/fixtures/project/combined.txt @@ -0,0 +1,7 @@ +1234 pxdding0 pxdding1 pxdding2 pxdding3 pxdding4 pxdding5 pxdding6 pxdding7 pxdding8 pxdding9 pxdding10 pxdding11 pxdding12 pxdding13 pxdding14 pxdding15 pxdding16 1234 pxdding17 pxdding18 pxdding19 pxdding20 pxdding21 pxdding22 pxdding23 pxdding24 pxdding25 pxdding26 pxdding27 pxdding28 pxdding29 pxdding30 pxdding31 pxdding32 pxdding33 pxdding34 1234 + +1234 pxdding0 pxdding1 pxdding2 1234 pxdding3 pxdding4 1234 pxdding5 1234 pxdding6 pxdding7 pxdding8 1234 + +1234 pxdding0 pxdding1 pxdding2 pxdding3 pxdding4 pxdding5 pxdding6 pxdding7 pxdding8 pxdding9 pxdding10 pxdding11 pxdding12 pxdding13 pxdding14 1234 pxdding15 pxdding16 pxdding17 pxdding18 pxdding19 pxdding20 pxdding21 pxdding22 pxdding23 pxdding24 pxdding25 pxdding26 pxdding27 1234 + +1234 pxdding0 pxdding1 pxdding2 pxdding3 pxdding4 pxdding5 pxdding6 pxdding7 pxdding8 pxdding9 pxdding10 pxdding11 pxdding12 pxdding13 pxdding14 pxdding15 pxdding16 1234 short text 1234 pxdding17 pxdding18 pxdding19 pxdding20 pxdding21 pxdding22 pxdding23 pxdding24 pxdding25 pxdding26 pxdding27 pxdding28 pxdding29 1234 diff --git a/spec/results-view-spec.js b/spec/results-view-spec.js index 2a731980..6c111b27 100644 --- a/spec/results-view-spec.js +++ b/spec/results-view-spec.js @@ -69,7 +69,48 @@ describe('ResultsView', () => { expect(resultsView.refs.listView.element.querySelectorAll('.preview').length).toBe(1); expect(resultsView.refs.listView.element.querySelector('.preview').textContent).toBe('test test test test test test test test test test test a b c d e f g h i j k l abcdefghijklmnopqrstuvwxyz'); expect(resultsView.refs.listView.element.querySelector('.match').textContent).toBe('ghijkl'); - }) + }); + }); + + describe("when the result has lines that contain multiple matches", () => { + let previews; + + beforeEach(async () => { + projectFindView.findEditor.setText('1234'); + atom.commands.dispatch(projectFindView.element, 'core:confirm'); + await searchPromise; + + resultsView = getResultsView(); + await resultsView.heightInvalidationPromise; + + previews = resultsView.refs.listView.element.querySelectorAll('.preview'); + }); + + it('returns search results for combined test', () => { + expect(resultsView.refs.listView.element.querySelector('.path-name').textContent).toBe("combined.txt"); + expect(previews.length).toBe(4); + }); + + it('concatenates noncontiguous matches on same line with ellipsis', () => { + expect(previews[0].textContent).toBe('1234 pxdding0 pxdding1 pxdding2 pxdding3 pxdding4 pxdding5 pxdding6 pxdding7 pxdding8 pxdding9 pxdding10…pxdding12 pxdding13 pxdding14 pxdding15 pxdding16 1234 pxdding17 pxdding18 pxdding19 pxdding20 pxdding21…pxdding25 pxdding26 pxdding27 pxdding28 pxdding29 pxdding30 pxdding31 pxdding32 pxdding33 pxdding34 1234'); + expect(previews[0].querySelectorAll('.match')[0].textContent).toBe('1234'); + expect(previews[0].querySelectorAll('.match').length).toBe(3); + }); + + it('concatenates contiguous matches with touching matches seamlessly', () => { + expect(previews[1].textContent).toBe('1234 pxdding0 pxdding1 pxdding2 1234 pxdding3 pxdding4 1234 pxdding5 1234 pxdding6 pxdding7 pxdding8 1234'); + expect(previews[1].querySelectorAll('.match').length).toBe(5); + }); + + it('concatenates contigous matches with touching contexts seamlessly', () => { + expect(previews[2].textContent).toBe('1234 pxdding0 pxdding1 pxdding2 pxdding3 pxdding4 pxdding5 pxdding6 pxdding7 pxdding8 pxdding9 pxdding10 pxdding11 pxdding12 pxdding13 pxdding14 1234 pxdding15 pxdding16 pxdding17 pxdding18 pxdding19 pxdding20 pxdding21 pxdding22 pxdding23 pxdding24 pxdding25 pxdding26 pxdding27 1234'); + expect(previews[2].querySelectorAll('.match').length).toBe(3); + }); + + it('concatenates different types of (non)contigous matches on same line accordingly', () => { + expect(previews[3].textContent).toBe('1234 pxdding0 pxdding1 pxdding2 pxdding3 pxdding4 pxdding5 pxdding6 pxdding7 pxdding8 pxdding9 pxdding10…pxdding12 pxdding13 pxdding14 pxdding15 pxdding16 1234 short text 1234 pxdding17 pxdding18 pxdding19 pxdding20 pxdding21 pxdding22 pxdding23 pxdding24 pxdding25 pxdding26 pxdding27 pxdding28 pxdding29 1234'); + expect(previews[3].querySelectorAll('.match').length).toBe(4); + }); }); describe("when there are multiple project paths", () => { @@ -117,8 +158,7 @@ describe('ResultsView', () => { await resultsView.heightInvalidationPromise; expect(resultsView.refs.listView.element.querySelector('.match').textContent).toBe('ghijkl'); expect(resultsView.refs.listView.element.querySelector('.match')).toHaveClass('highlight-info'); - expect(resultsView.refs.listView.element.querySelector('.replacement').textContent).toBe(''); - expect(resultsView.refs.listView.element.querySelector('.replacement')).toBeHidden(); + expect(resultsView.refs.listView.element.querySelector('.replacement')).toBeNull(); projectFindView.replaceEditor.setText('cats'); advanceClock(modifiedDelay); @@ -135,7 +175,7 @@ describe('ResultsView', () => { expect(resultsView.refs.listView.element.querySelector('.match').textContent).toBe('ghijkl'); expect(resultsView.refs.listView.element.querySelector('.match')).toHaveClass('highlight-info'); - expect(resultsView.refs.listView.element.querySelector('.replacement')).toBeHidden(); + expect(resultsView.refs.listView.element.querySelector('.replacement')).toBeNull(); }); it('renders the captured text when the replace pattern uses captures', async () => { @@ -816,8 +856,8 @@ describe('ResultsView', () => { runs(() => { resultsView = getResultsView() const iconElements = resultsView.element.querySelectorAll(iconSelector) - expect(iconElements[0].className.trim()).toBe('icon foo bar') - expect(iconElements[1].className.trim()).toBe('icon baz qlux') + expect(iconElements[0].className.trim()).toBe('icon baz qlux') + expect(iconElements[1].className.trim()).toBe('icon foo bar') expect(resultsView.element.querySelector('.icon-file-text')).toBe(null) disposable.dispose()