Skip to content

Commit ad76016

Browse files
authored
Fix & test Windows bracketed pasting issue (#117)
* Improve testing docstrings & add convenience methods * Add laggy patched preview test & test sample * Remove code of fix #81 * Increase grid size a bit * Also try to remove ESC+ENTER * Install patch in site-packages * Remove patch from test samples * Improve error handling for when patch fails * Only apply fix on Windows * Fix wrong filename in test * Make scene laggier * Also escape \r in python string * Decrease grid size and increase timeout for tests * Simplify retrieval of python binary in environment * Add docstring for windows patch * Add docstring to function that executes patch and waits for success * Use success string that is not in source code itself * Use `python` instead of `python3` * Use Node.js `exec` instead of spawning new VSCode terminal See this comment: #117 (comment) * Remove prints in Python patch * Test that windows paste patch is correctly applied * Fix logger import path * Run all tests 🙈 (remove `it.only()` call) * Restore accidentally deleted lines * Assert that Logger.error() not called & use sinon.match * Add a manual delay in test * Stub Logger.info() and Logger.trace() to log outputs * Don't stub/spy twice & only test windows patch * Fix non-working windows paste patch test * Rename function to `applyWindowsPastePatch` * Increase timeout for windows paste patch test * Try to use python3 instead of python * Also inject Node.js exec() method for tests * Remove Emoji in Python patch to avoid encoding issues * Detect python in venv correctly on Windows & use `.exe` * Also call `python.exe` instead of just `python` in tests * Fix typo in windows paste patch * Log output of `exec()` in pipeline * Log separate groups of exec output * Print patch command to console * Get rid of \r in patch file for exec() * Also escape backslashes * Base64-encode/decode to get rid of nasty escape errors * Get rid of console logs & run all tests * Use color in Mocha TTY * Do some final cleanup
1 parent 68874a7 commit ad76016

17 files changed

+512
-67
lines changed

.vscode/settings.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
"ipython",
9595
"Keybord",
9696
"kisak",
97+
"laggy",
9798
"libglut",
9899
"libpango",
99100
"Logfile",

package-lock.json

Lines changed: 83 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,12 +263,14 @@
263263
},
264264
"devDependencies": {
265265
"@stylistic/eslint-plugin": "^2.12.1",
266+
"@types/chai": "^5.0.1",
266267
"@types/node": "^22.10.2",
267268
"@types/sinon": "^17.0.3",
268269
"@types/vscode": "^1.96.0",
269270
"@typescript-eslint/parser": "^8.18.2",
270271
"@vscode/test-cli": "^0.0.10",
271272
"@vscode/test-electron": "^2.4.1",
273+
"chai": "^5.1.2",
272274
"eslint": "^9.17.0",
273275
"glob": "^11.0.0",
274276
"globals": "^15.14.0",

src/extension.ts

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import * as path from "path";
21
import * as vscode from "vscode";
32
import { window } from "vscode";
43
import { ManimShell, NoActiveShellError } from "./manimShell";
@@ -12,6 +11,8 @@ import { ExportSceneCodeLens } from "./export";
1211
import { tryToDetermineManimVersion, LAST_WARNING_NO_VERSION_KEY } from "./manimVersion";
1312
import { setupTestEnvironment } from "./utils/testing";
1413
import { EventEmitter } from "events";
14+
import { applyWindowsPastePatch } from "./patches/applyPatches";
15+
import { getBinaryPathInPythonEnv } from "./utils/venv";
1516

1617
export let manimNotebookContext: vscode.ExtensionContext;
1718
class WaitingForPythonExtensionCancelled extends Error {}
@@ -49,23 +50,33 @@ export async function activate(context: vscode.ExtensionContext) {
4950
await vscode.commands.executeCommand("workbench.action.openWalkthrough",
5051
`${context.extension.id}#manim-notebook-walkthrough`, false);
5152
});
52-
5353
context.subscriptions.push(openWalkthroughCommand);
5454

55-
let manimglPath: string | undefined = undefined;
55+
let pythonEnvPath: string | undefined = undefined;
5656
try {
57-
manimglPath = await waitForPythonExtension();
57+
pythonEnvPath = await waitForPythonExtension();
5858
} catch (err) {
5959
if (err instanceof WaitingForPythonExtensionCancelled) {
6060
Logger.info("💠 Waiting for Python extension cancelled, therefore"
6161
+ " we cancel the extension activation");
6262
return;
6363
}
6464
}
65-
if (manimglPath) {
66-
manimglPath = pythonEnvToManimglPath(manimglPath);
65+
66+
if (process.platform === "win32") {
67+
// Note that we shouldn't call `python3` on Windows,
68+
// see https://github.com/Manim-Notebook/manim-notebook/pull/117#discussion_r1932764875
69+
const pythonPath = pythonEnvPath
70+
? getBinaryPathInPythonEnv(pythonEnvPath, "python.exe")
71+
: "python";
72+
// not necessary to await here, can run in background
73+
applyWindowsPastePatch(context, pythonPath);
6774
}
68-
await tryToDetermineManimVersion(manimglPath);
75+
76+
const manimglBinary = pythonEnvPath
77+
? getBinaryPathInPythonEnv(pythonEnvPath, "manimgl")
78+
: "manimgl";
79+
await tryToDetermineManimVersion(manimglBinary);
6980

7081
const previewManimCellCommand = vscode.commands.registerCommand(
7182
"manim-notebook.previewManimCell", (cellCode?: string, startLine?: number) => {
@@ -135,7 +146,7 @@ export async function activate(context: vscode.ExtensionContext) {
135146
const redetectManimVersionCommand = vscode.commands.registerCommand(
136147
"manim-notebook.redetectManimVersion", async () => {
137148
Logger.info("💠 Command requested: Redetect Manim Version");
138-
await tryToDetermineManimVersion();
149+
await tryToDetermineManimVersion("manimgl");
139150
});
140151

141152
registerWalkthroughCommands(context);
@@ -205,22 +216,6 @@ async function waitForPythonExtension(): Promise<string | undefined> {
205216
});
206217
}
207218

208-
/**
209-
* Transforms a path pointing to either an environment folder or a
210-
* Python executable into a path pointing to the ManimGL binary.
211-
*
212-
* @param path The path to the Python environment or Python executable.
213-
* @returns The path to the ManimGL binary.
214-
*/
215-
function pythonEnvToManimglPath(envPath: string): string {
216-
if (envPath.endsWith("python") || envPath.endsWith("python3")) {
217-
return envPath.replace(/python3?$/, "manimgl");
218-
} else {
219-
const binFolderName = process.platform === "win32" ? "Scripts" : "bin";
220-
return path.join(envPath, binFolderName, "manimgl");
221-
}
222-
}
223-
224219
/**
225220
* A global event emitter that can be used to listen for the extension being
226221
* activated. This is only used for testing purposes.

src/manimShell.ts

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,6 @@ const IPYTHON_CELL_START_REGEX = /^\s*In \[\d+\]:/gm;
1919
*/
2020
const LOG_INFO_MESSAGE_REGEX = /^\s*\[.*\] INFO/m;
2121

22-
/**
23-
* Regular expression to match IPython multiline input "...:"
24-
* Sometimes IPython does not execute code when entering a newline, but enters a
25-
* multiline input mode, where it expects another line of code. We detect that
26-
* this happens and send an extra newline to run the code
27-
* See: https://github.com/Manim-Notebook/manim-notebook/issues/18
28-
*/
29-
const IPYTHON_MULTILINE_START_REGEX = /^\s*\.{3}:\s+$/m;
30-
3122
/**
3223
* Regular expression to match a KeyboardInterrupt.
3324
*/
@@ -326,9 +317,6 @@ export class ManimShell {
326317
startLine?: number,
327318
handler?: CommandExecutionEventHandler,
328319
) {
329-
// append ESC + ENTER to avoid IPython starting a multi-line input (#18)
330-
command += "\x1b\x0d";
331-
332320
Logger.debug(`🚀 Exec command: ${command}, waitUntilFinished=${waitUntilFinished}`
333321
+ `, forceExecute=${forceExecute}, errorOnNoActiveShell=${errorOnNoActiveShell}`);
334322

@@ -775,14 +763,6 @@ export class ManimShell {
775763
}
776764
}
777765

778-
if (this.isExecutingCommand && data.match(IPYTHON_MULTILINE_START_REGEX)) {
779-
Logger.debug("💨 IPython multiline detected, sending extra newline");
780-
// do not use shell integration here, as it might send a CTRL-C
781-
// while the prompt is not finished yet
782-
// \x7F deletes the extra line ("...:") from IPython
783-
this.exec(this.activeShell, "\x7F", false);
784-
}
785-
786766
if (data.match(ERROR_REGEX)) {
787767
Logger.debug("🚨 Error in IPython cell detected");
788768
this.activeShell?.show();

src/manimVersion.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ export async function isAtLeastManimVersion(versionRequired: string): Promise<bo
6868
+ " ManimGL version could not be determined.",
6969
determineAgainOption, "I don't care");
7070
if (answer === determineAgainOption) {
71-
await tryToDetermineManimVersion();
71+
await tryToDetermineManimVersion("manimgl");
7272
}
7373
}
7474
}
@@ -134,10 +134,10 @@ async function fetchLatestManimVersion(): Promise<string | undefined> {
134134
/**
135135
* Tries to determine the Manim version with the `manimgl --version` command.
136136
*
137-
* @param manimglPath The path to the ManimGL executable, e.g. in a virtual
137+
* @param manimglBinary The path to the ManimGL executable, e.g. in a virtual
138138
* environment. If undefined, we assume that `manimgl` is in the PATH.
139139
*/
140-
export async function tryToDetermineManimVersion(manimglPath: string | undefined = undefined) {
140+
export async function tryToDetermineManimVersion(manimglBinary: string) {
141141
MANIM_VERSION = undefined;
142142
let couldDetermineManimVersion = false;
143143
isCanceledByUser = false;
@@ -163,8 +163,7 @@ export async function tryToDetermineManimVersion(manimglPath: string | undefined
163163
Logger.info("🔒 Locking Manim welcome string detection");
164164

165165
const timeoutPromise = constructTimeoutPromise(8000, progress, token);
166-
const cmd = manimglPath ? `${manimglPath} --version` : "manimgl --version";
167-
const versionPromise = lookForManimVersionString(terminal, cmd);
166+
const versionPromise = lookForManimVersionString(terminal, `${manimglBinary} --version`);
168167
Promise.race([timeoutPromise, versionPromise])
169168
.then(couldResolveVersion => resolve(couldResolveVersion))
170169
.catch((err) => {
@@ -213,7 +212,7 @@ async function showNegativeUserVersionFeedback() {
213212
const errMessage = "Your ManimGL version could not be determined.";
214213
const answer = await Window.showErrorMessage(errMessage, tryAgainAnswer);
215214
if (answer === tryAgainAnswer) {
216-
await tryToDetermineManimVersion();
215+
await tryToDetermineManimVersion("manimgl");
217216
}
218217
}
219218

0 commit comments

Comments
 (0)