Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/actions/EditNewLine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,18 @@ class EditNewLine implements Action {
this.correctForParagraph(targets);
if (this.isAbove) {
await this.graph.actions.setSelectionBefore.run([targets]);
await commands.executeCommand("editor.action.insertLineBefore");
await commands.executeCommand(
targets[0].selectionContext.isNotebookCell
? "jupyter.insertCellAbove"
: "editor.action.insertLineBefore"
);
} else {
await this.graph.actions.setSelectionAfter.run([targets]);
await commands.executeCommand("editor.action.insertLineAfter");
await commands.executeCommand(
targets[0].selectionContext.isNotebookCell
? "jupyter.insertCellBelow"
: "editor.action.insertLineAfter"
);
}

return {
Expand Down
26 changes: 26 additions & 0 deletions src/checkCommandValidity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { ActionType, PartialTarget, SelectionType } from "./typings/Types";
import { getPrimitiveTargets } from "./util/targetUtils";

export function checkCommandValidity(
actionName: ActionType,
partialTargets: PartialTarget[],
extraArgs: any[]
) {
if (
usesSelectionType("notebookCell", partialTargets) &&
!["editNewLineAbove", "editNewLineBelow"].includes(actionName)
) {
throw new Error(
"The notebookCell scope type is currently only supported with the actions editNewLineAbove and editNewLineBelow"
);
}
}

function usesSelectionType(
selectionType: SelectionType,
partialTargets: PartialTarget[]
) {
return getPrimitiveTargets(partialTargets).some(
(partialTarget) => partialTarget.selectionType === selectionType
);
}
3 changes: 3 additions & 0 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { TestCase } from "./testUtil/TestCase";
import { ThatMark } from "./core/ThatMark";
import { TestCaseRecorder } from "./testUtil/TestCaseRecorder";
import { getParseTreeApi } from "./util/getExtensionApi";
import { checkCommandValidity } from "./checkCommandValidity";

export async function activate(context: vscode.ExtensionContext) {
const fontMeasurements = new FontMeasurements(context);
Expand Down Expand Up @@ -113,6 +114,8 @@ export async function activate(context: vscode.ExtensionContext) {

const action = graph.actions[actionName];

checkCommandValidity(actionName, partialTargets, extraArgs);

const targets = inferFullTargets(
partialTargets,
action.targetPreferences
Expand Down
17 changes: 17 additions & 0 deletions src/processTargets/processSelectionType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export default function (
switch (target.selectionType) {
case "token":
return processToken(target, selection, selectionContext);
case "notebookCell":
return processNotebookCell(target, selection, selectionContext);
case "document":
return processDocument(target, selection, selectionContext);
case "line":
Expand All @@ -32,6 +34,21 @@ export default function (
}
}

function processNotebookCell(
target: PrimitiveTarget,
selection: SelectionWithEditor,
selectionContext: SelectionContext
): TypedSelection {
const { selectionType, insideOutsideType, position } = target;
return {
selection,
selectionType,
position,
insideOutsideType,
selectionContext: { ...selectionContext, isNotebookCell: true },
};
}

function processToken(
target: PrimitiveTarget,
selection: SelectionWithEditor,
Expand Down
10 changes: 7 additions & 3 deletions src/test/runTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
downloadAndUnzipVSCode,
} from "vscode-test";

const extensionDependencies = ["pokey.parse-tree", "ms-toolsai.jupyter"];

async function main() {
try {
// The folder containing the Extension Manifest package.json
Expand All @@ -22,9 +24,11 @@ async function main() {
resolveCliPathFromVSCodeExecutablePath(vscodeExecutablePath);

// Install extension dependencies
cp.spawnSync(cliPath, ["--install-extension", "pokey.parse-tree"], {
encoding: "utf-8",
stdio: "inherit",
extensionDependencies.forEach((dependency) => {
cp.spawnSync(cliPath, ["--install-extension", dependency], {
encoding: "utf-8",
stdio: "inherit",
});
});

// Run the integration test
Expand Down
27 changes: 27 additions & 0 deletions src/test/suite/fixtures/recorded/selectionTypes/drinkCell.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Note that this is just checking that no errors are thrown
spokenForm: drink cell
languageId: python
command:
actionName: editNewLineAbove
partialTargets:
- {type: primitive, selectionType: notebookCell}
extraArgs: []
marks: {}
initialState:
documentContents: |-
# %%
print("hello")
selections:
- anchor: {line: 1, character: 12}
active: {line: 1, character: 12}
finalState:
documentContents: |-
# %%
print("hello")
Comment on lines +19 to +20
Copy link
Copy Markdown
Member Author

@pokey pokey Aug 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops. looks like (shockingly) the jupyter.insertCellAbove command returns a promise that resolves before the action is actually complete, so test recorder shows nothing having changed. awesome. so do we keep the test to just make sure nothing errors, delete the test, or insert some random sleep? cc/ @AndreasArvidsson @brxck

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wtf they dont ubderstand how a promise work do they?
I dont really care since we cant actually test it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yikes. If it were up to me I would leave the test as is with a note. I don't think it's worth it to accommodate with a sleep, we're never going to be 100% tests anyhow.

selections:
- anchor: {line: 1, character: 0}
active: {line: 1, character: 0}
thatMark:
- anchor: {line: 1, character: 0}
active: {line: 1, character: 0}
fullTargets: [{type: primitive, mark: {type: cursor}, selectionType: notebookCell, position: contents, insideOutsideType: inside, modifier: {type: identity}}]
38 changes: 38 additions & 0 deletions src/test/suite/fixtures/recorded/selectionTypes/drinkCellEach.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Note that this is just checking that no errors are thrown
spokenForm: drink cell each
languageId: python
command:
actionName: editNewLineAbove
partialTargets:
- type: primitive
selectionType: notebookCell
mark: {type: decoratedSymbol, symbolColor: default, character: e}
extraArgs: []
marks:
default.e:
start: {line: 1, character: 7}
end: {line: 1, character: 12}
initialState:
documentContents: |-
# %%
print("hello")

# %%
print("hello")
selections:
- anchor: {line: 4, character: 12}
active: {line: 4, character: 12}
finalState:
documentContents: |-
# %%
print("hello")

# %%
print("hello")
selections:
- anchor: {line: 1, character: 0}
active: {line: 1, character: 0}
thatMark:
- anchor: {line: 1, character: 0}
active: {line: 1, character: 0}
fullTargets: [{type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: e}, selectionType: notebookCell, position: contents, insideOutsideType: inside, modifier: {type: identity}}]
27 changes: 27 additions & 0 deletions src/test/suite/fixtures/recorded/selectionTypes/pourCell.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Note that this is just checking that no errors are thrown
spokenForm: pour cell
languageId: python
command:
actionName: editNewLineBelow
partialTargets:
- {type: primitive, selectionType: notebookCell}
extraArgs: []
marks: {}
initialState:
documentContents: |-
# %%
print("hello")
selections:
- anchor: {line: 1, character: 12}
active: {line: 1, character: 12}
finalState:
documentContents: |-
# %%
print("hello")
selections:
- anchor: {line: 3, character: 0}
active: {line: 3, character: 0}
thatMark:
- anchor: {line: 3, character: 0}
active: {line: 3, character: 0}
fullTargets: [{type: primitive, mark: {type: cursor}, selectionType: notebookCell, position: contents, insideOutsideType: inside, modifier: {type: identity}}]
38 changes: 38 additions & 0 deletions src/test/suite/fixtures/recorded/selectionTypes/pourCellEach.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Note that this is just checking that no errors are thrown
spokenForm: pour cell each
languageId: python
command:
actionName: editNewLineBelow
partialTargets:
- type: primitive
selectionType: notebookCell
mark: {type: decoratedSymbol, symbolColor: default, character: e}
extraArgs: []
marks:
default.e:
start: {line: 1, character: 7}
end: {line: 1, character: 12}
initialState:
documentContents: |-
# %%
print("hello")

# %%
print("hello")
selections:
- anchor: {line: 4, character: 12}
active: {line: 4, character: 12}
finalState:
documentContents: |-
# %%
print("hello")

# %%
print("hello")
selections:
- anchor: {line: 4, character: 0}
active: {line: 4, character: 0}
thatMark:
- anchor: {line: 4, character: 0}
active: {line: 4, character: 0}
fullTargets: [{type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: e}, selectionType: notebookCell, position: contents, insideOutsideType: inside, modifier: {type: identity}}]
11 changes: 5 additions & 6 deletions src/typings/Types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export type Mark =
| CursorMarkToken
| That
| Source
// | LastCursorPosition Not implemented yet
// | LastCursorPosition Not implemented yet
| DecoratedSymbol
| LineNumber;
export type Delimiter =
Expand Down Expand Up @@ -137,11 +137,8 @@ export type Modifier =
| TailModifier;

export type SelectionType =
// | "character" Not implemented
| "token"
| "line"
| "paragraph"
| "document";
// | "character" Not implemented
"token" | "line" | "notebookCell" | "paragraph" | "document";
export type Position = "before" | "after" | "contents";
export type InsideOutsideType = "inside" | "outside" | null;

Expand Down Expand Up @@ -228,6 +225,8 @@ export interface SelectionContext {
* The range of the delimiter after the selection
*/
trailingDelimiterRange?: vscode.Range | null;

isNotebookCell?: boolean;
}

export interface TypedSelection {
Expand Down
30 changes: 29 additions & 1 deletion src/util/targetUtils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { TextEditor, Selection, Position } from "vscode";
import { groupBy } from "./itertools";
import { TypedSelection } from "../typings/Types";
import {
PartialPrimitiveTarget,
PartialTarget,
TypedSelection,
} from "../typings/Types";

export function ensureSingleEditor(targets: TypedSelection[]) {
if (targets.length === 0) {
Expand Down Expand Up @@ -86,3 +90,27 @@ function createTypeSelection(
position: "contents",
};
}

/**
* Given a list of targets, recursively descends all targets and returns every
* contained primitive target.
*
* @param targets The targets to extract from
* @returns A list of primitive targets
*/
export function getPrimitiveTargets(targets: PartialTarget[]) {
return targets.flatMap(getPrimitiveTargetsHelper);
}

function getPrimitiveTargetsHelper(
target: PartialTarget
): PartialPrimitiveTarget[] {
switch (target.type) {
case "primitive":
return [target];
case "list":
return target.elements.flatMap(getPrimitiveTargetsHelper);
case "range":
return [target.start, target.end];
}
}