Skip to content
Open
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
114 changes: 89 additions & 25 deletions apps/vscode/src/providers/cell/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ import { ExtensionHost } from "../../host";
import { hasHooks } from "../../host/hooks";
import { isKnitrDocument } from "../../host/executors";
import { commands } from "vscode";
import { virtualDocForCode, withVirtualDocUri } from "../../vdoc/vdoc";
import { embeddedLanguage } from "../../vdoc/languages";
import { Uri } from "vscode";
import { StatementRange } from "positron";

export function cellCommands(host: ExtensionHost, engine: MarkdownEngine): Command[] {
return [
Expand Down Expand Up @@ -259,7 +263,19 @@ class RunPreviousCellCommand extends RunCommand implements Command {
}
}

// More permissive type than `Position` so its easier to construct via a literal
type LineAndCharPos = { line: number, character: number }
// More permissive type than `Range` so its easier to construct via a literal
type LineAndCharRange = { start: LineAndCharPos, end: LineAndCharPos }

function extractRangeFromCode(code: string, range: LineAndCharRange): string {
const extractedRange = lines(code).slice(range.start.line, range.end.line + 1)
extractedRange[0] = extractedRange[0].slice(range.start.character)
extractedRange[extractedRange.length - 1] = extractedRange[extractedRange.length - 1].slice(0, range.end.character)
return extractedRange.join('\n')
}

// Run the code at the cursor
class RunCurrentCommand extends RunCommand implements Command {
constructor(
host: ExtensionHost,
Expand Down Expand Up @@ -336,37 +352,85 @@ class RunCurrentCommand extends RunCommand implements Command {
context: CodeViewActiveBlockContext
): Promise<void> {
// get selection and active block
let selection = context.selectedText;
const selection = context.selectedText;
const activeBlock = context.blocks.find(block => block.active);

// if the selection is empty and this isn't a knitr document then it resolves to run cell
if (selection.length <= 0 && !isKnitrDocument(editor.document, this.engine_)) {
if (activeBlock) {
const executor = await this.cellExecutorForLanguage(activeBlock.language, editor.document, this.engine_);
if (executor) {
await executeInteractive(executor, [activeBlock.code], editor.document);
await activateIfRequired(editor);
}
const exec = async (action: CodeViewSelectionAction, selection: string) => {
const executor = await this.cellExecutorForLanguage(context.activeLanguage, editor.document, this.engine_);
if (executor) {
await executeInteractive(executor, [selection], editor.document);
await editor.setBlockSelection(context, action);
}
}

// if in Positron
if (hasHooks()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (hasHooks()) {
if (isPositron) {

Instead of adding a new use of hasHooks(), can we use the better support in the positron npm package, like this:

import { tryAcquirePositronApi } from "@posit-dev/positron";

And then something like:

const isPositron = tryAcquirePositronApi();

We don't need to do a wholesale refactor of the extension, but let's not add new uses of the old, worse approach.

if (activeBlock && selection.length <= 0) {
const codeLines = lines(activeBlock.code)
const vdoc = virtualDocForCode(codeLines, embeddedLanguage(activeBlock.language)!);
if (vdoc) {
const parentUri = Uri.file(editor.document.fileName);
const injectedLines = (vdoc.language?.inject?.length ?? 0)

const positionIntoVdoc = (p: LineAndCharPos) =>
new Position(p.line + injectedLines, p.character)
const positionOutOfVdoc = (p: LineAndCharPos) =>
new Position(p.line - injectedLines, p.character)
const rangeOutOfVdoc = (r: Range): LineAndCharRange => ({
start: positionOutOfVdoc(r.start),
end: positionOutOfVdoc(r.end)
})
const getStatementRange = async (pos: LineAndCharPos) => {
const result = await withVirtualDocUri(vdoc, parentUri, "statementRange", async (uri) => {
return await commands.executeCommand<StatementRange>(
"vscode.executeStatementRangeProvider",
uri,
positionIntoVdoc(pos)
);
});
return rangeOutOfVdoc(result.range)
}

const range = await getStatementRange(context.selection.start)
const code = extractRangeFromCode(activeBlock.code, range)

// BEGIN ref: https://github.com/posit-dev/positron/blob/main/src/vs/workbench/contrib/positronConsole/browser/positronConsoleActions.ts#L428
// strategy from Positron using `StatementRangeProvider` to find range of next statement
// and move cursor based on that.
if (range.end.line + 1 <= codeLines.length) {
// get range of statement at line after current statement)
const nextRange = await getStatementRange(new Position(range.end.line + 1, 1))

if (nextRange.start.line > range.end.line) {
exec(nextRange.start, code)
// the next statement range may start before & end after the current statement if e.g. inside a function:
} else if (nextRange.end.line > range.end.line) {
exec(nextRange.end, code)
} else {
exec("nextline", code)
}
} else {
exec("nextline", code)
}
// END ref.
}
}
// if not in Positron
} else {
// if the selection is empty take the whole line, otherwise take the selected text exactly
let action: CodeViewSelectionAction | undefined;
if (selection.length <= 0) {
// if the selection is empty and this isn't a knitr document then it resolves to run cell
if (selection.length <= 0 && !isKnitrDocument(editor.document, this.engine_)) {
if (activeBlock) {
selection = lines(activeBlock.code)[context.selection.start.line];
action = "nextline";
const executor = await this.cellExecutorForLanguage(activeBlock.language, editor.document, this.engine_);
if (executor) {
await executeInteractive(executor, [activeBlock.code], editor.document);
await activateIfRequired(editor);
}
}
}

// run code
const executor = await this.cellExecutorForLanguage(context.activeLanguage, editor.document, this.engine_);
if (executor) {
await executeInteractive(executor, [selection], editor.document);

// advance cursor if necessary
if (action) {
editor.setBlockSelection(context, "nextline");
} else {
if (selection.length > 0) {
exec("nextline", selection)
} else if (activeBlock) { // if the selection is empty take the whole line as the selection
exec("nextline", lines(activeBlock.code)[context.selection.start.line])
}
}
}
Expand All @@ -382,7 +446,7 @@ class RunSelectionCommand extends RunCurrentCommand implements Command {

}


// Run Cell and Advance
class RunCurrentAdvanceCommand extends RunCommand implements Command {
constructor(host: ExtensionHost, engine: MarkdownEngine) {
super(host, engine);
Expand Down
5 changes: 5 additions & 0 deletions apps/vscode/src/providers/editor/codeview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ export function vscodeCodeViewServer(_engine: MarkdownEngine, document: TextDocu
async codeViewAssist(context: CodeViewCellContext) {
await commands.executeCommand("quarto.codeViewAssist", context, lspRequest);
},
// This execute command is used when the user clicks an execute button on a cell in the visual editor.
//
// Note: this is NOT used when the user uses a keyboard command to execute a cell,
// that goes through VSCode commands (commands are registered in package.json),
// the keyboard command code is in apps/vscode/src/providers/cell/commands.ts.
async codeViewExecute(execute: CodeViewExecute) {
switch (execute) {
case "cell":
Expand Down
22 changes: 11 additions & 11 deletions packages/editor-codemirror/src/behaviors/trackselection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { DispatchEvent, codeViewCellContext, kCodeViewNextLineTransaction } from
import { Behavior, BehaviorContext, State } from ".";

// track the selection in prosemirror
export function trackSelectionBehavior(context: BehaviorContext) : Behavior {
export function trackSelectionBehavior(context: BehaviorContext): Behavior {

let unsubscribe: VoidFunction;

Expand All @@ -50,32 +50,32 @@ export function trackSelectionBehavior(context: BehaviorContext) : Behavior {
unsubscribe = context.pmContext.events.subscribe(DispatchEvent, (tr: Transaction | undefined) => {
if (tr) {
// track selection changes that occur when we don't have focus
if (!cmView.hasFocus && tr.selectionSet && !tr.docChanged && (tr.selection instanceof TextSelection)) {
if (tr.selectionSet && !tr.docChanged && (tr.selection instanceof TextSelection)) {
const cmSelection = asCodeMirrorSelection(context.view, cmView, context.getPos);
context.withState(State.Updating, () => {
if (cmSelection) {
cmView.dispatch({ selection: cmSelection });
} else {
cmView.dispatch({ selection: EditorSelection.single(0)})
}
cmView.dispatch({ selection: EditorSelection.single(0) })
}
})
} else if (tr.getMeta(kCodeViewNextLineTransaction) === true) {
// NOTE: this is a special directive to advance to the next line. as distinct
// from the block above it is not a reporting of a change in the PM selection
// but rather an instruction to move the CM selection to the next line. as
// but rather an instruction to move the CM selection to the next line. as
// such we do not encose the code in State.Updating, because we want an update
// to the PM selection to occur
const cmSelection = asCodeMirrorSelection(context.view, cmView, context.getPos);
if (cmSelection) {
if (cursorLineDown(cmView)) {
cursorLineStart(cmView);
}
}
}
// for other selection changes
// for other selection changes
} else if (cmView.hasFocus && tr.selectionSet && (tr.selection instanceof TextSelection)) {
codeViewAssist();
}
}
}
});
},

Expand All @@ -91,7 +91,7 @@ export const asCodeMirrorSelection = (
cmView: EditorView,
getPos: (() => number) | boolean
) => {
if (typeof(getPos) === "function") {
if (typeof (getPos) === "function") {
const offset = getPos() + 1;
const node = pmView.state.doc.nodeAt(getPos());
if (node) {
Expand All @@ -104,8 +104,8 @@ export const asCodeMirrorSelection = (
} else if (selection.from <= cmRange.from && selection.to >= cmRange.to) {
return EditorSelection.single(0, cmView.state.doc.length);
}

}
}
return undefined;
}
}
2 changes: 1 addition & 1 deletion packages/editor-types/src/codeview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export interface CodeViewActiveBlockContext {
selectedText: string;
}

export type CodeViewSelectionAction = "nextline" | "nextblock" | "prevblock";
export type CodeViewSelectionAction = "nextline" | "nextblock" | "prevblock" | { line: number, character: number };
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if you want to add a field to that object that you can explicitly use as the tag for a tagged union.

If you needed to expand the type today, then you've accepted that migrations are necessary. But you've now made migration a little bit harder in the case where a new value also has line: number, character: number.

export type CodeViewSelectionAction = "nextline" | "nextblock" | "prevblock" | { line: number, character: number } 
| {some_other_field: string, line: number, character: number} // testing for this is not annoying.


export interface CodeViewCellContext {
filepath: string;
Expand Down
22 changes: 16 additions & 6 deletions packages/editor/src/api/codeview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,25 @@ export function codeViewSetBlockSelection(
context: CodeViewActiveBlockContext,
action: CodeViewSelectionAction
) {


const activeIndex = context.blocks.findIndex(block => block.active);

if (activeIndex !== -1) {
if (action === "nextline") {
if (typeof action === 'object') {
// convert action line and character in code block space to pos in prosemirror space
const block = context.blocks[activeIndex]
// asummes the meta line looks like this:
const metaLine = '{' + block.language + '}\n'
const code = lines(block.code)
if (action.line > code.length) throw 'trying to move cursor outside block!'
let pos = block.pos + metaLine.length
for (let i = 0; i < action.line; i++) {
pos += code[i].length + 1
}
pos += action.character

navigateToPos(view, pos, false)
}
else if (action === "nextline") {
const tr = view.state.tr;
tr.setMeta(kCodeViewNextLineTransaction, true);
view.dispatch(tr);
Expand All @@ -226,9 +239,6 @@ export function codeViewSetBlockSelection(
}
}
}



}


Expand Down