refactor: consolidate donut/pie chart generators (#5)

* refactor: consolidate chart generators

* refactor: convert tests to single generate file, remove missed render chart test

* docs: update file descriptions

* chore: remove donut/pie exports

* chore: bump v1.1.1

Author: masonlet

README.md

@@ -19,9 +19,7 @@ Core library for github-top-languages projects — chart generation, SVG output,
 | `charts/geometry.js`    | SVG arc path math and segment helpers     |
 | `charts/legend.js`      | Legend element generation                 |
 | `charts/layout.js`      | Shared layout calculations                |
-| `charts/generate.js`    | Chart type dispatcher                     |
-| `charts/donut.js`       | Donut chart segment and legend generation |
-| `charts/pie.js`         | Pie chart segment and legend generation   |
+| `charts/generate.js`    | Chart geometry dispatch, segment and legend generation |
 | `render/svg.js`         | SVG document rendering                    |
 | `render/error.js`       | Error SVG rendering                       |
 

package.json

@@ -2,7 +2,7 @@
   "name": "@gh-top-languages/lib",
   "description": "Library for github-top-languages — chart generation, SVG output, and parameter parsing",
   "author": "Mason L'Etoile",
-  "version": "1.1.0",
+  "version": "1.1.1",
   "license": "MIT",
   "repository": {
     "type": "git",
@@ -29,8 +29,6 @@
   "files": ["dist", "src"],
   "exports": {
     "./charts/types.js":       { "types": "./dist/charts/types.d.ts",       "import": "./dist/charts/types.js"       },
-    "./charts/donut.js":       { "types": "./dist/charts/donut.d.ts",       "import": "./dist/charts/donut.js"       },
-    "./charts/pie.js":         { "types": "./dist/charts/pie.d.ts",         "import": "./dist/charts/pie.js"         },
     "./charts/geometry.js":    { "types": "./dist/charts/geometry.d.ts",    "import": "./dist/charts/geometry.js"    },
     "./charts/legend.js":      { "types": "./dist/charts/legend.d.ts",      "import": "./dist/charts/legend.js"      },
     "./charts/layout.js":      { "types": "./dist/charts/layout.d.ts",      "import": "./dist/charts/layout.js"      },

src/charts/donut.ts

@@ -1,24 +1,45 @@
-import type { ChartResult, Language, Theme, ChartType, GapType } from "./types.js";
-import { generateDonutChart } from "./donut.js";
-import { generatePieChart   } from "./pie.js";
+import { DONUT_GEOMETRY, PIE_GEOMETRY } from "../constants/geometry.js";
+import type { ChartResult, Language, Theme, ChartType, GapType, Geometry } from "./types.js";
+import { computeLayout                } from "./layout.js";
+import { createDonutSegments          } from "./geometry.js";
+import { createLegend                 } from "./legend.js";
 
-const CHART_GENERATORS: Record<ChartType, (
-  data:    Language[],
-  theme:   Theme,
-  gapType: GapType,
-  stroke:  boolean
-) => ChartResult> = {
-  donut: generateDonutChart,
-  pie:   generatePieChart,
-}
+const GEOMETRY: Record<ChartType, Geometry> = { donut: DONUT_GEOMETRY, pie: PIE_GEOMETRY };
 
 export function generateChartData(
-  data:      Language[],
-  theme:     Theme,
+  data: Language[],
+  theme: Theme,
   chartType: ChartType,
-  gapType:   GapType,
-  stroke:    boolean
+  gapType: GapType,
+  stroke: boolean
 ): ChartResult {
-  const generator = CHART_GENERATORS[chartType] || CHART_GENERATORS.donut;
-  return generator(data, theme, gapType, stroke);
+  const geometry = GEOMETRY[chartType] ?? GEOMETRY.donut;
+
+  const {
+    chartX,
+    legendStartX,
+    columnWidth,
+    contentWidth,
+    contentHeight
+  } = computeLayout(data, geometry, gapType);
+
+  const segments = createDonutSegments(
+    data,
+    chartX,
+    geometry,
+    [...theme.colours],
+    stroke,
+    gapType,
+    theme.gap
+  );
+  const legend = createLegend(
+    data,
+    theme,
+    legendStartX,
+    stroke,
+    columnWidth,
+    gapType
+  );
+
+  return { segments, legend, contentWidth, contentHeight };
 }

src/charts/generate.ts

@@ -0,0 +1,85 @@
+import { describe, it, expect, vi, afterEach } from "vitest";
+import type { ChartType, Theme               } from "../../src/charts/types.js";
+import { generateChartData        } from "../../src/charts/generate.js";
+import { createDonutSegments      } from "../../src/charts/geometry.js";
+import { createLegend             } from "../../src/charts/legend.js";
+
+vi.mock("../../src/charts/geometry.js", () => ({
+  createDonutSegments: vi.fn(() => `<path d="mockSegment"/>`)
+}));
+
+vi.mock("../../src/charts/legend.js", () => ({
+  createLegend: vi.fn(() => "<rect/><text>mockLegend</text>")
+}));
+
+const mockCreateDonutSegments = vi.mocked(createDonutSegments);
+const mockCreateLegend = vi.mocked(createLegend);
+
+const theme: Theme = { colours: ["#f00", "#0f0"], text: "#333", bg: "#fff", gap: "#000" };
+
+describe("generateChartData", () => {
+  afterEach(() => {
+    vi.clearAllMocks();
+  });
+
+  ["donut" as ChartType, "pie" as ChartType].forEach((type) => {
+    describe(`${type} context`, () => {
+      it("returns segments, legend, and content dimensions", () => {
+        const langs  = [{ lang: "JS", pct: 100 }];
+        const result = generateChartData(langs, theme, type, "gap", false);
+        expect(result).toHaveProperty("segments");
+        expect(result).toHaveProperty("legend");
+        expect(result).toHaveProperty("contentWidth");
+        expect(result).toHaveProperty("contentHeight");
+        expect(mockCreateDonutSegments).toHaveBeenCalled();
+        expect(mockCreateLegend).toHaveBeenCalled();
+      });
+
+      it("computes chartX and legendStartX as numbers", () => {
+        const langs = [{ lang: "Python", pct: 100 }];
+        generateChartData(langs, theme, type, "gap", false);
+        const segmentCall = mockCreateDonutSegments.mock.calls.at(-1);
+        const legendCall = mockCreateLegend.mock.calls.at(-1) ?? [];
+        expect(typeof segmentCall![1]).toBe("number");
+        expect(typeof legendCall[2]).toBe("number");
+        expect(typeof legendCall[4]).toBe("number");
+      });
+
+      it("passes theme to both segments and legend", () => {
+        const langs = [{ lang: "HTML", pct: 100 }];
+        generateChartData(langs, theme, type, "gap", false);
+        const segmentsCall = mockCreateDonutSegments.mock.calls.at(-1)!;
+        const legendCall = mockCreateLegend.mock.calls.at(-1) ?? [];
+        expect(segmentsCall[3]).toEqual(theme.colours);
+        expect(legendCall[1]).toBe(theme);
+      });
+    });
+  });
+
+  describe("donut", () => {
+    it("falls back to donut geometry for unrecognized chartType", () => {
+      const langs = [{ lang: "JS", pct: 100 }];
+      generateChartData(langs, theme, "bigbadwolf" as ChartType, "gap", false);
+      const call = mockCreateDonutSegments.mock.calls.at(-1)!;
+      expect(call[2].INNER_RADIUS).not.toBe(0);
+    });
+
+    it("passes non-zero INNER_RADIUS for donut", () => {
+      const langs = [{ lang: "HTML", pct: 100 }];
+      generateChartData(langs, theme, "donut", "gap", false);
+      const call = mockCreateDonutSegments.mock.calls.at(-1)!;
+      const geometry = call[2];
+      expect(geometry.INNER_RADIUS).not.toBe(0);
+    });
+  });
+
+  describe("pie", () => {
+    it("passes INNER_RADIUS: 0 for filled pie", () => {
+      const langs = [{ lang: "Python", pct: 100 }];
+      generateChartData(langs, theme, "pie", "gap", false);
+      const call = mockCreateDonutSegments.mock.calls.at(-1)!;
+      const geometry = call[2];
+      expect(geometry.INNER_RADIUS).toBe(0);
+    });
+  });
+});