Skip to content

Commit b58c500

Browse files
committed
chore: collapse Skulpt and Pyodide adapters into one
It's evident with testing so far that a separate adapter for each is not needed; they produce error messages / traces in the same format (currently).
1 parent beacce4 commit b58c500

10 files changed

Lines changed: 41 additions & 86 deletions

File tree

README.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,17 @@ It can be used in browser-based editors (like RPF's [Code Editor web component](
2121
import {
2222
loadCopydeckFor,
2323
registerAdapter,
24-
skulptAdapter,
25-
pyodideAdapter,
24+
cpythonAdapter,
2625
friendlyExplain
2726
} from "python-friendly-error-messages";
2827

2928
await loadCopydeckFor(navigator.language); // falls back to "en"
3029

31-
// register runtimes
32-
registerAdapter("skulpt", skulptAdapter);
33-
registerAdapter("pyodide", pyodideAdapter);
30+
// register runtimes - Skulpt and Pyodide both appear to emit CPython-style tracebacks,
31+
// so the same adapter handles both. The runtime name you register under is
32+
// added onto the resulting trace
33+
registerAdapter("skulpt", cpythonAdapter);
34+
registerAdapter("pyodide", cpythonAdapter);
3435

3536
// later, when you have an error string and some code:
3637
const result = friendlyExplain({

docs/index.html

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ <h1>Python Friendly Error Messages - Demo</h1>
5757
: './vendor/python-friendly-error-messages/index.browser.js';
5858
const libPath = cacheBust ? `${libPathBase}?cb=${encodeURIComponent(cacheBust)}` : libPathBase;
5959

60-
const { friendlyExplain, loadCopydeckFor, skulptAdapter, pyodideAdapter } = await import(libPath);
60+
const { friendlyExplain, loadCopydeckFor, cpythonAdapter } = await import(libPath);
6161

6262
async function initDemo() {
6363
const container = document.getElementById('demo-container');
@@ -97,8 +97,8 @@ <h1>Python Friendly Error Messages - Demo</h1>
9797
await loadCopydeckFor('en');
9898

9999
const adaptersByRuntime = {
100-
skulpt: skulptAdapter,
101-
pyodide: pyodideAdapter
100+
skulpt: cpythonAdapter,
101+
pyodide: cpythonAdapter
102102
};
103103

104104
const processedExamples = examples.map((example) => {
Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,18 @@ const lastLineTypeMessage = (raw: string) => {
77
return { type: m ? m[1] : null, message: m ? m[2] : tail, tail, lines };
88
};
99

10-
export const skulptAdapter: AdapterFn = (raw, code) => {
10+
/**
11+
* Parses a CPython-shaped traceback (`File "...", line N`).
12+
*
13+
* Skulpt and Pyodide both emit this format, so a single adapter covers both.
14+
* There is no current need for runtime-specific parsing . The `runtime` label on
15+
* the returned trace is left as "unknown"; the engine adds the concrete runtime
16+
* (the registration key it dispatched on) in `coerceTrace`. If the two runtimes
17+
* ever diverge (e.g CPython 3.11+ caret ranges or "Did you mean?" hints that
18+
* Pyodide surfaces but Skulpt does not), branch here on a passed-in runtime
19+
* hint to parse those features out of the message
20+
*/
21+
export const cpythonAdapter: AdapterFn = (raw, code) => {
1122
const { type, message, tail, lines } = lastLineTypeMessage(raw);
1223
let file: string | undefined, line: number | undefined, col: number | undefined;
1324

@@ -17,6 +28,8 @@ export const skulptAdapter: AdapterFn = (raw, code) => {
1728
file = mm[1];
1829
line = parseInt(mm[2], 10);
1930
}
31+
const cc = L.match(/column\s+(\d+)/i);
32+
if (cc) col = parseInt(cc[1], 10);
2033
}
2134
if (!line) {
2235
const loc = tail.match(/\b(?:on|at)\s+line\s+(\d+)\s+(?:of|in)\s+([^\s:]+)\b/i);
@@ -38,7 +51,7 @@ export const skulptAdapter: AdapterFn = (raw, code) => {
3851
// - name: quoted symbol from the message (helpful for NameError/KeyError/etc.)
3952
//
4053
// If none are present, the input is not parseable enough for this adapter,
41-
// so we return null and let the caller handle that failure explicitly.
54+
// so we return null and let the caller handle that failure explicitly
4255
const hasStructuredSignal = Boolean(type || line || name);
4356
if (!hasStructuredSignal) return null;
4457

@@ -51,7 +64,7 @@ export const skulptAdapter: AdapterFn = (raw, code) => {
5164
col,
5265
frames: [],
5366
name,
54-
runtime: "skulpt"
67+
runtime: "unknown"
5568
};
5669

5770
if (code && line) {

src/adapters/pyodide.ts

Lines changed: 0 additions & 59 deletions
This file was deleted.

src/engine.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { CopyDeck, ExplainOptions, ExplainResult, Section, Trace } from "./types";
1+
import type { CopyDeck, ExplainOptions, ExplainResult, Runtime, Section, Trace } from "./types";
22
import { escapeHtml, safeRegexTest, tmpl } from "./utils";
33

44
type InternalState = {
@@ -34,6 +34,9 @@ const coerceTrace = (input: string | Error | Trace, code?: string, runtime?: str
3434
if (!parsed) {
3535
throw new Error(`Could not parse error for runtime \"${runtime}\".`);
3636
}
37+
// The runtime-agnostic adapter leaves `runtime: "unknown"`; this adds the concrete
38+
// runtime we dispatched on so the trace carries the correct label
39+
parsed.runtime = runtime as Runtime;
3740
return parsed;
3841
};
3942

src/index.browser.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
export type { Trace, ExplainOptions, ExplainResult, CopyDeck, AdapterFn } from "./types.js";
22
export { loadCopydeck, registerAdapter, friendlyExplain } from "./engine.js";
3-
export { skulptAdapter } from "./adapters/skulpt.js";
4-
export { pyodideAdapter } from "./adapters/pyodide.js";
3+
export { cpythonAdapter } from "./adapters/cpython.js";
54
export { loadCopydeckFor } from "./loaders.browser.js";
65

src/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
export type { Trace, ExplainOptions, ExplainResult, CopyDeck, AdapterFn } from "./types.js";
22
export { loadCopydeck, registerAdapter, friendlyExplain } from "./engine.js";
3-
export { skulptAdapter } from "./adapters/skulpt.js";
4-
export { pyodideAdapter } from "./adapters/pyodide.js";
3+
export { cpythonAdapter } from "./adapters/cpython.js";
54
export { loadCopydeckFor } from "./loaders.js";

tests/copydeck-demo-coverage.test.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { describe, it, expect, beforeAll } from "vitest";
22
import { friendlyExplain, loadCopydeck, registerAdapter } from "../src/engine";
3-
import { skulptAdapter } from "../src/adapters/skulpt";
4-
import { pyodideAdapter } from "../src/adapters/pyodide";
3+
import { cpythonAdapter } from "../src/adapters/cpython";
54
import copydeck from "../copydecks/en/copydeck.json";
65
import { examples } from "../docs/demo-examples.js";
76

@@ -14,15 +13,15 @@ type DemoExample = {
1413
};
1514

1615
const adaptersByRuntime = {
17-
skulpt: skulptAdapter,
18-
pyodide: pyodideAdapter,
16+
skulpt: cpythonAdapter,
17+
pyodide: cpythonAdapter,
1918
};
2019

2120
describe("copydeck/demo coverage", () => {
2221
beforeAll(() => {
2322
loadCopydeck(copydeck as any);
24-
registerAdapter("skulpt", skulptAdapter);
25-
registerAdapter("pyodide", pyodideAdapter);
23+
registerAdapter("skulpt", cpythonAdapter);
24+
registerAdapter("pyodide", cpythonAdapter);
2625
});
2726

2827
it("maps each demo example to its expected variant", () => {

tests/engine.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, it, expect } from "vitest";
22
import { friendlyExplain, loadCopydeck, registerAdapter } from "../src/engine";
3-
import { skulptAdapter } from "../src/adapters/skulpt";
3+
import { cpythonAdapter } from "../src/adapters/cpython";
44

55
const copydeck = {
66
meta: { language: "en", version: 1 },
@@ -62,7 +62,7 @@ const copydeck = {
6262

6363
describe("engine", () => {
6464
loadCopydeck(copydeck as any);
65-
registerAdapter("skulpt", skulptAdapter);
65+
registerAdapter("skulpt", cpythonAdapter);
6666

6767
it("explains NameError with name and patch", () => {
6868
const code = `print("Hello")\nprint(kittens)\n`;

tests/loaders.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { describe, it, expect, vi, beforeEach } from "vitest";
22
import { loadCopydeckFor } from "../src/loaders";
33
import { friendlyExplain, loadCopydeck, registerAdapter } from "../src/engine";
4-
import { skulptAdapter } from "../src/adapters/skulpt";
4+
import { cpythonAdapter } from "../src/adapters/cpython";
55

66
const makeRes = (ok: boolean, data?: any) =>
77
new Response(ok ? JSON.stringify(data) : "not found", { status: ok ? 200 : 404 });
@@ -23,7 +23,7 @@ describe("loadCopydeckFor", () => {
2323
.mockResolvedValueOnce(makeRes(true, minimalDeck)); // /copydecks/en/copydeck.json
2424

2525
await loadCopydeckFor("en-GB");
26-
registerAdapter("skulpt", skulptAdapter);
26+
registerAdapter("skulpt", cpythonAdapter);
2727

2828
const res = friendlyExplain({
2929
error: { type: "TypeError", message: "bad", raw: "TypeError: bad", runtime: "unknown" },
@@ -43,7 +43,7 @@ describe("loadCopydeckFor", () => {
4343

4444
it("still supports manual loadCopydeck for tests without fetch", () => {
4545
loadCopydeck(minimalDeck as any);
46-
registerAdapter("skulpt", skulptAdapter);
46+
registerAdapter("skulpt", cpythonAdapter);
4747
const res = friendlyExplain({
4848
error: { type: "NameError", message: "name 'x' is not defined", raw: "NameError: name 'x' is not defined", runtime: "unknown" },
4949
code: ""

0 commit comments

Comments
 (0)