Skip to content

Commit d9d07c4

Browse files
YoyokrazyYoyokrazy
authored
Support wrapping for notebook multicursor (microsoft#232773)
* fix wrapping in textModel find + path multicursor * unnecessary do while * use isBefore * patch test
1 parent 0dd03f7 commit d9d07c4

File tree

3 files changed

+86
-39
lines changed

3 files changed

+86
-39
lines changed

‎src/vs/workbench/contrib/notebook/browser/contrib/multicursor/notebookMulticursor.ts

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ export class NotebookMultiCursorController extends Disposable implements INotebo
8888
static readonly id: string = 'notebook.multiCursorController';
8989

9090
private word: string = '';
91+
private startPosition: {
92+
cellIndex: number;
93+
position: Position;
94+
} | undefined;
9195
private trackedMatches: TrackedMatch[] = [];
9296

9397
private readonly _onDidChangeAnchorCell = this._register(new Emitter<void>());
@@ -439,6 +443,8 @@ export class NotebookMultiCursorController extends Disposable implements INotebo
439443
this.cursorsDisposables.clear();
440444
this.cursorsControllers.clear();
441445
this.trackedMatches = [];
446+
this.startPosition = undefined;
447+
this.word = '';
442448
}
443449

444450
public async findAndTrackNextSelection(cell: ICellViewModel): Promise<void> {
@@ -455,6 +461,16 @@ export class NotebookMultiCursorController extends Disposable implements INotebo
455461
}
456462
this.word = word.word;
457463

464+
const index = this.notebookEditor.getCellIndex(cell);
465+
if (index === undefined) {
466+
return;
467+
}
468+
469+
this.startPosition = {
470+
cellIndex: index,
471+
position: new Position(inputSelection.startLineNumber, word.startColumn),
472+
};
473+
458474
const newSelection = new Selection(
459475
inputSelection.startLineNumber,
460476
word.startColumn,
@@ -501,31 +517,37 @@ export class NotebookMultiCursorController extends Disposable implements INotebo
501517
} else if (this.state === NotebookMultiCursorState.Selecting) { // use the word we stored from idle state transition to find next match, track it
502518
const notebookTextModel = this.notebookEditor.textModel;
503519
if (!notebookTextModel) {
504-
return;
520+
return; // should not happen
505521
}
506522

507523
const index = this.notebookEditor.getCellIndex(cell);
508524
if (index === undefined) {
509-
return;
525+
return; // should not happen
526+
}
527+
528+
if (!this.startPosition) {
529+
return; // should not happen
510530
}
511531

512532
const findResult = notebookTextModel.findNextMatch(
513533
this.word,
514534
{ cellIndex: index, position: cell.getSelections()[cell.getSelections().length - 1].getEndPosition() },
515535
false,
516536
true,
517-
USUAL_WORD_SEPARATORS //! might want to get these from the editor config
537+
USUAL_WORD_SEPARATORS,
538+
this.startPosition,
518539
);
519540
if (!findResult) {
520-
return; //todo: some sort of message to the user alerting them that there are no more matches? editor does not do this
541+
this.state = NotebookMultiCursorState.Editing;
542+
return;
521543
}
522544

523545
const resultCellViewModel = this.notebookEditor.getCellByHandle(findResult.cell.handle);
524546
if (!resultCellViewModel) {
525547
return;
526548
}
527549

528-
let newMatch: TrackedMatch;
550+
let newMatch: TrackedMatch | undefined;
529551
if (findResult.cell.handle === cell.handle) { // match is in the same cell, find tracked entry, update and set selections in viewmodel and cursorController
530552
newMatch = this.trackedMatches.find(match => match.cellViewModel.handle === findResult.cell.handle)!;
531553
newMatch.wordSelections.push(Selection.fromRange(findResult.match.range, SelectionDirection.LTR));
@@ -537,7 +559,6 @@ export class NotebookMultiCursorController extends Disposable implements INotebo
537559
return;
538560
}
539561
controller.setSelections(new ViewModelEventsCollector(), undefined, newMatch.wordSelections, CursorChangeReason.Explicit);
540-
541562
} else if (findResult.cell.handle !== cell.handle) { // result is in a different cell, move focus there and apply selection, then update anchor
542563
await this.notebookEditor.revealRangeInViewAsync(resultCellViewModel, findResult.match.range);
543564
this.notebookEditor.focusNotebookCell(resultCellViewModel, 'editor');
@@ -566,17 +587,29 @@ export class NotebookMultiCursorController extends Disposable implements INotebo
566587
cursorSmoothCaretAnimation: rawEditorOptions.cursorSmoothCaretAnimation!
567588
};
568589

569-
newMatch = {
570-
cellViewModel: resultCellViewModel,
571-
initialSelection: initialSelection,
572-
wordSelections: [newSelection],
573-
editorConfig: editorConfig,
574-
cursorConfig: cursorConfig,
575-
decorationIds: [],
576-
undoRedoHistory: this.undoRedoService.getElements(resultCellViewModel.uri)
577-
} satisfies TrackedMatch;
578-
this.trackedMatches.push(newMatch);
579-
590+
newMatch = this.trackedMatches.find(match => match.cellViewModel.handle === findResult.cell.handle);
591+
if (newMatch) {
592+
newMatch.wordSelections.push(Selection.fromRange(findResult.match.range, SelectionDirection.LTR));
593+
this.clearDecorations(newMatch); // clear old decorations for this cell, since we are active here again
594+
resultCellViewModel.setSelections(newMatch.wordSelections);
595+
const controller = this.cursorsControllers.get(newMatch.cellViewModel.uri);
596+
if (!controller) {
597+
// should not happen
598+
return;
599+
}
600+
controller.setSelections(new ViewModelEventsCollector(), undefined, newMatch.wordSelections, CursorChangeReason.Explicit);
601+
} else {
602+
newMatch = {
603+
cellViewModel: resultCellViewModel,
604+
initialSelection: initialSelection,
605+
wordSelections: [newSelection],
606+
editorConfig: editorConfig,
607+
cursorConfig: cursorConfig,
608+
decorationIds: [],
609+
undoRedoHistory: this.undoRedoService.getElements(resultCellViewModel.uri)
610+
} satisfies TrackedMatch;
611+
this.trackedMatches.push(newMatch);
612+
}
580613
this._onDidChangeAnchorCell.fire();
581614

582615
// we set the decorations manually for the cell we have just departed, since it blurs
@@ -696,7 +729,7 @@ export class NotebookMultiCursorController extends Disposable implements INotebo
696729
*
697730
* @param match -- match object containing the viewmodel + selections
698731
*/
699-
private initializeMultiSelectDecorations(match: TrackedMatch | undefined, isCurrentWord?: boolean) {
732+
private initializeMultiSelectDecorations(match: TrackedMatch | undefined) {
700733
if (!match) {
701734
return;
702735
}

‎src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { FindMatch, ITextModel } from '../../../../../editor/common/model.js';
2121
import { TextModel } from '../../../../../editor/common/model/textModel.js';
2222
import { isDefined } from '../../../../../base/common/types.js';
2323
import { ILanguageDetectionService } from '../../../../services/languageDetection/common/languageDetectionWorkerService.js';
24-
import { IPosition } from '../../../../../editor/common/core/position.js';
24+
import { Position } from '../../../../../editor/common/core/position.js';
2525
import { Range } from '../../../../../editor/common/core/range.js';
2626
import { SearchParams } from '../../../../../editor/common/model/textModelSearch.js';
2727
import { IModelContentChangedEvent } from '../../../../../editor/common/textModelEvents.js';
@@ -1194,8 +1194,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
11941194
}
11951195

11961196
//#region Find
1197-
// TODO: enable wrapping through cell indices -- could do this with a wrapped flag
1198-
findNextMatch(searchString: string, searchStart: { cellIndex: number; position: IPosition }, isRegex: boolean, matchCase: boolean, wordSeparators: string | null): { cell: NotebookCellTextModel; match: FindMatch } | null {
1197+
findNextMatch(searchString: string, searchStart: { cellIndex: number; position: Position }, isRegex: boolean, matchCase: boolean, wordSeparators: string | null, searchEnd?: { cellIndex: number; position: Position }): { cell: NotebookCellTextModel; match: FindMatch } | null {
11991198
// check if search cell index is valid
12001199
this._assertIndex(searchStart.cellIndex);
12011200
const searchParams = new SearchParams(searchString, isRegex, matchCase, wordSeparators);
@@ -1208,23 +1207,37 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
12081207
let cellIndex = searchStart.cellIndex;
12091208
let searchStartPosition = searchStart.position;
12101209

1211-
while (cellIndex < this._cells.length) {
1210+
let searchEndCell = this._cells.length;
1211+
1212+
while (cellIndex < searchEndCell) {
12121213
const cell = this._cells[cellIndex];
1214+
1215+
// if we have wrapped back to the point of the initial search cell, we search from beginning to the provided searchEnd position
1216+
const wrapFlag = searchEnd && cellIndex === searchEnd.cellIndex && searchStartPosition.isBefore(searchEnd.position);
12131217
const searchRange = new Range(
12141218
searchStartPosition.lineNumber,
12151219
searchStartPosition.column,
1216-
cell.textBuffer.getLineCount(),
1217-
cell.textBuffer.getLineMaxColumn(cell.textBuffer.getLineCount())
1220+
(wrapFlag) ? searchEnd.position.lineNumber : cell.textBuffer.getLineCount(),
1221+
(wrapFlag) ? searchEnd.position.column : cell.textBuffer.getLineMaxColumn(cell.textBuffer.getLineCount())
12181222
);
12191223

12201224
const result = cell.textBuffer.findMatchesLineByLine(searchRange, searchData, false, 1);
12211225
if (result.length > 0) {
12221226
return { cell, match: result[0] };
1227+
} else if (wrapFlag) { // this means there are no more valid matches in the notebook
1228+
break;
12231229
}
12241230

12251231
// Move to the next cell
12261232
cellIndex++;
1227-
searchStartPosition = { lineNumber: 1, column: 1 }; // Reset position to start of the next cell
1233+
1234+
// wrap if a searchEnd is provided and we are past the end of the notebook
1235+
if (searchEnd && cellIndex >= this._cells.length) {
1236+
cellIndex = 0;
1237+
searchEndCell = searchEnd.cellIndex + 1;
1238+
}
1239+
1240+
searchStartPosition = new Position(1, 1); // Reset position to start of the next cell
12281241
}
12291242

12301243
return null;

‎src/vs/workbench/contrib/notebook/test/browser/notebookTextModel.test.ts

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { VSBuffer } from '../../../../../base/common/buffer.js';
88
import { DisposableStore } from '../../../../../base/common/lifecycle.js';
99
import { Mimes } from '../../../../../base/common/mime.js';
1010
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
11+
import { Position } from '../../../../../editor/common/core/position.js';
1112
import { ILanguageService } from '../../../../../editor/common/languages/language.js';
1213
import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js';
1314
import { IUndoRedoService } from '../../../../../platform/undoRedo/common/undoRedo.js';
@@ -1454,31 +1455,31 @@ suite('NotebookTextModel', () => {
14541455
const notebookModel = viewModel.notebookDocument;
14551456

14561457
// Test case 1: Find 'var' starting from the first cell
1457-
let findMatch = notebookModel.findNextMatch('var', { cellIndex: 0, position: { lineNumber: 1, column: 1 } }, false, false, null);
1458+
let findMatch = notebookModel.findNextMatch('var', { cellIndex: 0, position: new Position(1, 1) }, false, false, null);
14581459
assert.ok(findMatch);
14591460
assert.strictEqual(findMatch!.match.range.startLineNumber, 1);
14601461
assert.strictEqual(findMatch!.match.range.startColumn, 1);
14611462

14621463
// Test case 2: Find 'b' starting from the second cell
1463-
findMatch = notebookModel.findNextMatch('b', { cellIndex: 1, position: { lineNumber: 1, column: 1 } }, false, false, null);
1464+
findMatch = notebookModel.findNextMatch('b', { cellIndex: 1, position: new Position(1, 1) }, false, false, null);
14641465
assert.ok(findMatch);
14651466
assert.strictEqual(findMatch!.match.range.startLineNumber, 1);
14661467
assert.strictEqual(findMatch!.match.range.startColumn, 5);
14671468

14681469
// Test case 3: Find 'c' starting from the third cell
1469-
findMatch = notebookModel.findNextMatch('c', { cellIndex: 2, position: { lineNumber: 1, column: 1 } }, false, false, null);
1470+
findMatch = notebookModel.findNextMatch('c', { cellIndex: 2, position: new Position(1, 1) }, false, false, null);
14701471
assert.ok(findMatch);
14711472
assert.strictEqual(findMatch!.match.range.startLineNumber, 1);
14721473
assert.strictEqual(findMatch!.match.range.startColumn, 5);
14731474

14741475
// Test case 4: Find 'd' starting from the fourth cell
1475-
findMatch = notebookModel.findNextMatch('d', { cellIndex: 3, position: { lineNumber: 1, column: 1 } }, false, false, null);
1476+
findMatch = notebookModel.findNextMatch('d', { cellIndex: 3, position: new Position(1, 1) }, false, false, null);
14761477
assert.ok(findMatch);
14771478
assert.strictEqual(findMatch!.match.range.startLineNumber, 1);
14781479
assert.strictEqual(findMatch!.match.range.startColumn, 5);
14791480

14801481
// Test case 5: No match found
1481-
findMatch = notebookModel.findNextMatch('e', { cellIndex: 0, position: { lineNumber: 1, column: 1 } }, false, false, null);
1482+
findMatch = notebookModel.findNextMatch('e', { cellIndex: 0, position: new Position(1, 1) }, false, false, null);
14821483
assert.strictEqual(findMatch, null);
14831484
}
14841485
);
@@ -1496,52 +1497,52 @@ suite('NotebookTextModel', () => {
14961497
const notebookModel = viewModel.notebookDocument;
14971498

14981499
// Test case 1: Find 'var' starting from the first cell
1499-
let findMatch = notebookModel.findNextMatch('var', { cellIndex: 0, position: { lineNumber: 1, column: 1 } }, false, false, null);
1500+
let findMatch = notebookModel.findNextMatch('var', { cellIndex: 0, position: new Position(1, 1) }, false, false, null);
15001501
assert.ok(findMatch);
15011502
assert.strictEqual(findMatch!.match.range.startLineNumber, 1);
15021503
assert.strictEqual(findMatch!.match.range.startColumn, 1);
15031504

15041505
// Test case 2: Find 'b' starting from the second cell
1505-
findMatch = notebookModel.findNextMatch('b', { cellIndex: 1, position: { lineNumber: 1, column: 1 } }, false, false, null);
1506+
findMatch = notebookModel.findNextMatch('b', { cellIndex: 1, position: new Position(1, 1) }, false, false, null);
15061507
assert.ok(findMatch);
15071508
assert.strictEqual(findMatch!.match.range.startLineNumber, 1);
15081509
assert.strictEqual(findMatch!.match.range.startColumn, 5);
15091510

15101511
// Test case 3: Find 'c' starting from the third cell
1511-
findMatch = notebookModel.findNextMatch('c', { cellIndex: 2, position: { lineNumber: 1, column: 1 } }, false, false, null);
1512+
findMatch = notebookModel.findNextMatch('c', { cellIndex: 2, position: new Position(1, 1) }, false, false, null);
15121513
assert.ok(findMatch);
15131514
assert.strictEqual(findMatch!.match.range.startLineNumber, 1);
15141515
assert.strictEqual(findMatch!.match.range.startColumn, 5);
15151516

15161517
// Test case 4: Find 'd' starting from the fourth cell
1517-
findMatch = notebookModel.findNextMatch('d', { cellIndex: 3, position: { lineNumber: 1, column: 1 } }, false, false, null);
1518+
findMatch = notebookModel.findNextMatch('d', { cellIndex: 3, position: new Position(1, 1) }, false, false, null);
15181519
assert.ok(findMatch);
15191520
assert.strictEqual(findMatch!.match.range.startLineNumber, 1);
15201521
assert.strictEqual(findMatch!.match.range.startColumn, 5);
15211522

15221523
// Test case 5: No match found
1523-
findMatch = notebookModel.findNextMatch('e', { cellIndex: 0, position: { lineNumber: 1, column: 1 } }, false, false, null);
1524+
findMatch = notebookModel.findNextMatch('e', { cellIndex: 0, position: new Position(1, 1) }, false, false, null);
15241525
assert.strictEqual(findMatch, null);
15251526

15261527
// Test case 6: Same keywords in the same cell
1527-
findMatch = notebookModel.findNextMatch('var', { cellIndex: 0, position: { lineNumber: 1, column: 1 } }, false, false, null);
1528+
findMatch = notebookModel.findNextMatch('var', { cellIndex: 0, position: new Position(1, 1) }, false, false, null);
15281529
assert.ok(findMatch);
15291530
assert.strictEqual(findMatch!.match.range.startLineNumber, 1);
15301531
assert.strictEqual(findMatch!.match.range.startColumn, 1);
15311532

1532-
findMatch = notebookModel.findNextMatch('var', { cellIndex: 0, position: { lineNumber: 1, column: 5 } }, false, false, null);
1533+
findMatch = notebookModel.findNextMatch('var', { cellIndex: 0, position: new Position(1, 5) }, false, false, null);
15331534
assert.ok(findMatch);
15341535
assert.strictEqual(findMatch!.match.range.startLineNumber, 1);
15351536
assert.strictEqual(findMatch!.match.range.startColumn, 12);
15361537

15371538
// Test case 7: Search from the middle of a cell with keyword before and after
1538-
findMatch = notebookModel.findNextMatch('a', { cellIndex: 0, position: { lineNumber: 1, column: 10 } }, false, false, null);
1539+
findMatch = notebookModel.findNextMatch('a', { cellIndex: 0, position: new Position(1, 10) }, false, false, null);
15391540
assert.ok(findMatch);
15401541
assert.strictEqual(findMatch!.match.range.startLineNumber, 1);
15411542
assert.strictEqual(findMatch!.match.range.startColumn, 13);
15421543

15431544
// Test case 8: Search from a cell and next match is in another cell below
1544-
findMatch = notebookModel.findNextMatch('var', { cellIndex: 0, position: { lineNumber: 1, column: 20 } }, false, false, null);
1545+
findMatch = notebookModel.findNextMatch('var', { cellIndex: 0, position: new Position(1, 20) }, false, false, null);
15451546
assert.ok(findMatch);
15461547
assert.strictEqual(findMatch!.match.range.startLineNumber, 1);
15471548
assert.strictEqual(findMatch!.match.range.startColumn, 1);

0 commit comments

Comments
 (0)