diff --git a/app/src/main/java/app/drydock/DrydockApplication.java b/app/src/main/java/app/drydock/DrydockApplication.java index beb285e..218698b 100644 --- a/app/src/main/java/app/drydock/DrydockApplication.java +++ b/app/src/main/java/app/drydock/DrydockApplication.java @@ -6,7 +6,10 @@ import app.drydock.app.RepositoryManager; import app.drydock.app.SessionManager; import app.drydock.activity.SessionActivityWatcher; +import app.drydock.config.UserConfig; import app.drydock.domain.Repository; +import app.drydock.domain.UiTheme; +import app.drydock.domain.WorkspaceUiState; import app.drydock.git.ChangedLineService; import app.drydock.git.DiffService; import app.drydock.git.GhCliService; @@ -22,6 +25,8 @@ import app.drydock.ui.MainWorkspace; import app.drydock.ui.RemoteRepositoryModal; import app.drydock.ui.RepositorySidebar; +import app.drydock.ui.SettingsModal; +import app.drydock.ui.SizeSetting; import app.drydock.ui.model.WorkspaceViewModel; import app.drydock.ui.review.ReviewView; import javafx.application.Application; @@ -199,6 +204,7 @@ private void startOnFxThread(Stage primaryStage) { appShell = new AppShell(primaryStage, WINDOW_TITLE, sidebar, mainWorkspace, repositoryManager.state().ui().sidebarWidth(), repositoryManager.state().ui().theme(), + repositoryManager.state().ui().uiFontSize(), theme -> { repositoryManager.updateTheme(theme); // Terminals follow the app theme: re-theme every live @@ -208,6 +214,79 @@ private void startOnFxThread(Stage primaryStage) { DEFAULT_SCENE_WIDTH, DEFAULT_SCENE_HEIGHT); mainWorkspace.setThemeProvider(() -> appShell.themeManager().theme()); + mainWorkspace.setTerminalFontSizeProvider( + () -> repositoryManager.state().ui().terminalFontSize()); + // Warms the ghostty config cache for the pair the FIRST opened + // session will need, off the FX thread: MainWorkspace. + // createOpenSessionTab reads TerminalThemes.configFileFor + // synchronously, which is only cheap once this (theme, size) pair + // is cached (see that method's Javadoc). Reuses applyTerminalTheme + // rather than a bespoke warm call -- there are no open tabs yet, so + // its "apply to every open terminal" step is a no-op, and every + // later theme toggle or terminal-size change re-warms the same way + // through that same method. + mainWorkspace.applyTerminalTheme(appShell.themeManager().theme()); + + appShell.setOnShowSettings(() -> { + SettingsModal settingsModal = new SettingsModal(new SettingsModal.Settings() { + @Override + public UiTheme theme() { + return appShell.themeManager().theme(); + } + + @Override + public void setTheme(UiTheme theme) { + // Persists and re-themes terminals via AppShell's own + // onThemeChanged callback; no duplicate write here. + appShell.themeManager().setTheme(theme); + } + + @Override + public SizeSetting interfaceSize() { + // Applying is live only and persisting is a plain write of + // the chosen value: ThemeManager.setUiFontSize regenerates + // off the FX thread on a cache miss, so its applied size + // can still be one FX event behind when the persist runs + // (see SizeSetting) -- which is why nothing here reads it + // back. A size that fails to generate now degrades to the + // unscaled stylesheet instead of failing the next launch, + // so persisting the raw choice is safe. + return new SizeSetting( + () -> appShell.themeManager().uiFontSize(), + size -> appShell.themeManager().setUiFontSize(size), + repositoryManager::updateUiFontSize); + } + + @Override + public SizeSetting terminalSize() { + // The live step bypasses repositoryManager.state() on + // purpose: mainWorkspace.applyTerminalTheme reads the size + // back through terminalFontSizeProvider, which would still + // see the OLD persisted value mid-drag. + return new SizeSetting( + () -> repositoryManager.state().ui().terminalFontSize(), + mainWorkspace::previewTerminalFontSize, + repositoryManager::updateTerminalFontSize); + } + + @Override + public CompletableFuture> loadWorktreesDirectory() { + return UserConfig.loadAsync().thenApply(UserConfig::worktreesDirectory); + } + + @Override + public CompletableFuture saveWorktreesDirectory(Optional directory) { + return UserConfig.saveAsync(new UserConfig(directory)); + } + }, appShell.modalLayer()::close); + // onClosed, not just the Done/× onClose above: Esc and a + // backdrop click hide the modal without moving focus off the + // worktrees field, so its focus-lost commit never fires -- + // flushPendingEdit is the one seam every close path runs + // through (see ModalLayer.close's onClosed callback). + appShell.modalLayer().show(settingsModal, settingsModal::flushPendingEdit); + }); + mainWorkspace.setModalLayer(appShell.modalLayer()); mainWorkspace.setOnToggleSidebar(appShell::toggleSidebar); // The native ghostty view paints over in-scene modals; hide it while @@ -443,6 +522,48 @@ private void startOnFxThread(Stage primaryStage) { reviewOpener.start(); } + // Diagnostic hook for the Settings modal's visual and behavioural + // pass. The modal is the one part of the settings feature no unit + // test can reach (there is no JavaFX toolkit harness here), and its + // riskiest paths -- "does Esc lose a typed directory", "is the + // radio legible in both themes" -- are only observable in a running + // app, so they get a driver rather than an unverified claim. + // + // Value is a comma-separated ":[:]" script, + // same shape as diag.explorerScript. Verbs: + // open show the Settings modal (as the gear button does) + // theme:DARK set the theme absolutely + // uisize:15 drag the interface-size slider to 15 + // tsize:18 drag the terminal-size slider to 18 + // dirtext:/p type into the worktrees-directory field + // esc close via the SAME path the Esc key filter uses + // shot:/tmp/a.png snapshot the scene + // dump print the persisted sizes and the on-disk config + // Inert unless -Dapp.drydock.diag.settingsScript is set. + String settingsScript = System.getProperty("app.drydock.diag.settingsScript"); + if (settingsScript != null) { + Thread driver = new Thread(() -> { + long start = System.nanoTime(); + for (String step : settingsScript.split(",")) { + String[] parts = step.split(":", 3); + long atMillis = (long) (Double.parseDouble(parts[0].strip()) * 1000); + long elapsed = (System.nanoTime() - start) / 1_000_000; + if (elapsed < atMillis) { + try { + Thread.sleep(atMillis - elapsed); + } catch (InterruptedException e) { + return; + } + } + String verb = parts[1].strip(); + String arg = parts.length > 2 ? parts[2] : ""; + Platform.runLater(() -> diagSettingsStep(primaryStage, verb, arg)); + } + }); + driver.setDaemon(true); + driver.start(); + } + // Diagnostic hook: sends 10 synthetic scroll-up events through the // selected tab's scroll path (verifies the Java -> // ghostty_surface_mouse_scroll pipeline without real NSEvents). @@ -544,7 +665,14 @@ private void installGlobalShortcuts(RepositorySidebar sidebar) { return; } if (cmd && event.isShiftDown() && event.getCode() == KeyCode.L) { - appShell.toggleTheme(); + // Gated like ⌘, below: the settings modal's theme radio reads + // the theme once at construction and has no listener on the + // manager, so a toggle underneath it would desync the radio + // from reality with no way for the user to click it back + // (SettingsModal's class Javadoc). + if (!appShell.modalLayer().isShowingModal()) { + appShell.toggleTheme(); + } event.consume(); } else if (cmd && event.getCode() == KeyCode.OPEN_BRACKET) { mainWorkspace.selectPreviousSessionTab(); @@ -584,6 +712,15 @@ private void installGlobalShortcuts(RepositorySidebar sidebar) { .filter(s -> s.id().equals(id)).findFirst()) .ifPresent(mainWorkspace::promptRenameSession); event.consume(); + } else if (cmd && event.getCode() == KeyCode.COMMA) { + // Inert while another modal is up: ⌘, must never replace an + // in-progress Start-session or New-worktree modal underneath + // the user. (The gear button is unreachable then anyway -- + // the backdrop covers the title bar.) + if (!appShell.modalLayer().isShowingModal()) { + appShell.showSettings(); + } + event.consume(); } else if (!inTextInput && !cmd && event.isShiftDown() && event.getCode() == KeyCode.SLASH) { appShell.showShortcutsOverlay(); @@ -659,6 +796,7 @@ public void stop() { closeQuietly("sidebar-width persistence", () -> repositoryManager.updateSidebarWidth(sidebarWidth)); } } + closeQuietly("UserConfig saves", UserConfig::flushPendingSaves); if (gitHubService != null) { closeQuietly("GitHubService", gitHubService::close); } @@ -758,6 +896,107 @@ private static void closeQuietly(String what, Runnable close) { } } + /** + * Diagnostic-only: one step of {@code app.drydock.diag.settingsScript}. + * Drives the Settings modal through the same entry points the UI uses -- + * {@code showSettings} is what the gear button calls, and {@code esc} + * goes through {@code ModalLayer.close()} exactly as the scene's Esc + * filter does -- so what this exercises is the real path, not a + * test-only shortcut past it. + */ + private void diagSettingsStep(Stage stage, String verb, String arg) { + try { + switch (verb) { + case "open" -> { + appShell.showSettings(); + System.out.println("[diag] settings opened"); + } + case "theme" -> { + appShell.themeManager().setTheme(UiTheme.valueOf(arg.strip())); + System.out.println("[diag] theme set to " + arg.strip()); + } + case "uisize" -> { + double size = Double.parseDouble(arg.strip()); + appShell.themeManager().setUiFontSize(size); + repositoryManager.updateUiFontSize(size); + System.out.println("[diag] interface size set to " + size); + } + case "tsize" -> { + double size = Double.parseDouble(arg.strip()); + repositoryManager.updateTerminalFontSize(size); + mainWorkspace.applyTerminalTheme(appShell.themeManager().theme()); + System.out.println("[diag] terminal size set to " + size); + } + case "dirtext" -> { + TextInputControl field = diagSettingsDirectoryField(); + if (field == null) { + System.out.println("[diag] dirtext: no directory field found (is the modal open?)"); + } else { + field.setText(arg); + System.out.println("[diag] dirtext typed: " + arg); + } + } + case "esc" -> { + appShell.modalLayer().close(); + System.out.println("[diag] modal closed via the Esc path"); + } + case "type" -> { + // Types into the focused terminal and presses Return. + // Used to prove the terminal font size took effect + // WITHOUT pixels: the ghostty surface is a native view + // stacked above the JavaFX scene, so a scene snapshot + // cannot see it, but `stty size` reports the cell grid, + // which must change when the cell size does. + arg.codePoints().forEach(cp -> { + String ch = new String(Character.toChars(cp)); + mainWorkspace.diagPressKey(0, ch, ch); + }); + mainWorkspace.diagPressKey(36, "\r", "\r"); + System.out.println("[diag] typed: " + arg); + } + case "view" -> { + // The terminal sub-tab runs a plain login shell, so a + // font-size check does not depend on `claude` being + // installed or authenticated. + if ("terminal".equals(arg.strip())) { + mainWorkspace.showTerminalSubTab(); + } else { + mainWorkspace.showClaudeSubTab(); + } + System.out.println("[diag] view -> " + arg.strip()); + } + case "shot" -> diagSnapshot(stage, Path.of(arg)); + case "dump" -> { + WorkspaceUiState ui = repositoryManager.state().ui(); + System.out.println("[diag] persisted uiFontSize=" + ui.uiFontSize() + + " terminalFontSize=" + ui.terminalFontSize() + + " theme=" + ui.theme()); + Path configFile = UserConfig.defaultConfigFile(); + System.out.println("[diag] config " + configFile + " -> " + + (Files.exists(configFile) + ? Files.readString(configFile).replace('\n', ' ') + : "(absent)")); + } + default -> System.out.println("[diag] unknown settings verb " + verb); + } + } catch (IOException | RuntimeException e) { + System.out.println("[diag] settings step '" + verb + "' failed: " + e); + } + } + + /** + * The Settings modal's worktrees-directory field, or {@code null} when the + * modal is not showing. Located by type: it is the modal's only text input. + */ + private TextInputControl diagSettingsDirectoryField() { + for (javafx.scene.Node node : appShell.modalLayer().lookupAll(".text-field")) { + if (node instanceof TextInputControl field) { + return field; + } + } + return null; + } + /** * Diagnostic-only: writes a PNG of the window's scene graph to {@code target}. * diff --git a/app/src/main/java/app/drydock/app/RepositoryManager.java b/app/src/main/java/app/drydock/app/RepositoryManager.java index 3294060..494f7a2 100644 --- a/app/src/main/java/app/drydock/app/RepositoryManager.java +++ b/app/src/main/java/app/drydock/app/RepositoryManager.java @@ -190,6 +190,20 @@ public void updateTheme(UiTheme theme) { stateStore.update(state -> state.withUi(state.ui().withTheme(theme))); } + /** + * Persists the interface font size immediately, like the theme: a + * discrete user action in the settings modal, cheap to save, and losing + * it to a crash would be a visible regression on the next launch. + */ + public void updateUiFontSize(double uiFontSize) { + stateStore.update(state -> state.withUi(state.ui().withUiFontSize(uiFontSize))); + } + + /** Persists the terminal font size immediately, for the same reason as {@link #updateUiFontSize}. */ + public void updateTerminalFontSize(double terminalFontSize) { + stateStore.update(state -> state.withUi(state.ui().withTerminalFontSize(terminalFontSize))); + } + private static String defaultDisplayName(Path root) { Path fileName = root.getFileName(); return fileName == null ? root.toString() : fileName.toString(); diff --git a/app/src/main/java/app/drydock/config/UserConfig.java b/app/src/main/java/app/drydock/config/UserConfig.java index 598bd2b..bc18a1a 100644 --- a/app/src/main/java/app/drydock/config/UserConfig.java +++ b/app/src/main/java/app/drydock/config/UserConfig.java @@ -5,6 +5,7 @@ import app.drydock.state.json.JsonValue; import app.drydock.state.json.JsonValue.JsonObject; import app.drydock.state.json.JsonValue.JsonString; +import app.drydock.state.json.JsonWriter; import java.io.IOException; import java.lang.System.Logger; @@ -13,8 +14,15 @@ import java.nio.file.Files; import java.nio.file.InvalidPathException; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicReference; /** * User-editable settings read from {@code ~/.drydock/config.json}, separate @@ -51,10 +59,31 @@ public static UserConfig load() { * "file loading" must never block the JavaFX application thread) -- * {@link #load()} does a synchronous stat + read, so callers on the FX * thread (e.g. the create-worktree modal) must use this instead. + * + *

Routed through {@link #SAVE_EXECUTOR} rather than an independent + * virtual thread so a load is FIFO-ordered against every {@link + * #saveAsync} queued before it: an unordered read could otherwise land + * before a just-committed save's write and hand back the stale value + * (e.g. close-then-reopen the settings modal right after editing the + * worktrees directory).

*/ public static CompletableFuture loadAsync() { CompletableFuture future = new CompletableFuture<>(); - Thread.ofVirtual().start(() -> future.complete(load())); + SAVE_EXECUTOR.execute(() -> { + try { + future.complete(load()); + } catch (Throwable t) { + // load() already catches everything it knows how to handle + // and falls back to empty(); anything reaching here is + // unexpected. Completing exceptionally (instead of letting + // it silently kill the thread) matters because the caller + // -- the settings modal -- disables its controls until this + // future completes one way or the other; an uncompleted + // future would leave them stuck at "Loading…" forever. + future.completeExceptionally(t); + throw t; + } + }); return future; } @@ -79,4 +108,173 @@ static UserConfig load(Path configFile) { return empty(); } } + + /** + * Writes {@code config} to {@code configFile}: temp file in the same + * directory, then an atomic move, so a crash mid-write can never leave a + * truncated config where a valid one was. + * + *

Members this build does not know about are read back from the + * existing file and preserved -- the file is hand-editable, so silently + * dropping a key a newer build (or the user) put there would be data + * loss.

+ * + *

Unlike {@link #load()}, a failure here throws: a save is a + * user-visible action, and one that silently did nothing is worse than + * one that reports why.

+ * + *

{@code configFile} is resolved to an absolute path first so it + * always has a parent directory to create the temp file in -- a bare + * relative filename with no parent segment would otherwise force the + * temp file into the JVM's default temp directory, which can be a + * different filesystem and break {@link StandardCopyOption#ATOMIC_MOVE}. + * A path that resolves to a filesystem root (no parent even after that) + * is rejected outright rather than silently falling back.

+ */ + static void save(UserConfig config, Path configFile) throws IOException { + Path resolvedConfigFile = configFile.toAbsolutePath().normalize(); + Path parent = resolvedConfigFile.getParent(); + if (parent == null) { + throw new IOException("Cannot save config to a path with no parent directory: " + resolvedConfigFile); + } + + JsonObject root = readExistingRootOrEmpty(resolvedConfigFile); + root.members().remove("worktreesDirectory"); + config.worktreesDirectory().ifPresent(dir -> root.put("worktreesDirectory", new JsonString(dir.toString()))); + + Files.createDirectories(parent); + Path temp = Files.createTempFile(parent, "config", ".json.tmp"); + try { + Files.writeString(temp, JsonWriter.write(root), StandardCharsets.UTF_8); + Files.move(temp, resolvedConfigFile, StandardCopyOption.REPLACE_EXISTING, + StandardCopyOption.ATOMIC_MOVE); + } finally { + Files.deleteIfExists(temp); + } + } + + /** Best-effort read of {@code configFile}'s existing top-level object, for {@link #save} to preserve unknown members. */ + private static JsonObject readExistingRootOrEmpty(Path configFile) { + if (Files.exists(configFile)) { + try { + if (JsonParser.parse(Files.readString(configFile, StandardCharsets.UTF_8)) + instanceof JsonObject existing) { + return existing; + } + } catch (IOException | JsonParseException e) { + LOG.log(Level.WARNING, "Existing config " + configFile + + " is unreadable or malformed; replacing it", e); + } + } + return JsonObject.empty(); + } + + /** + * Runs saves for {@link #saveAsync} one at a time. A plain "one virtual + * thread per call" approach (as an earlier version of this class did) + * lets concurrent saves race: the settings modal commits on both + * field-blur and Browse, which can fire back-to-back, and with + * unordered threads the stale call's {@code Files.move} can land after + * the fresh one's, silently reverting the file. A single-thread executor + * makes writes happen in submission order, same as {@code + * AnnotationStore.saveExecutor}. + */ + private static final ExecutorService SAVE_EXECUTOR = + Executors.newSingleThreadExecutor(runnable -> Thread.ofVirtual().unstarted(runnable)); + + /** + * Newest-wins pending save, mirroring {@code AnnotationStore.pendingSnapshot}: + * {@link #saveAsync} always writes the *whole* config, so a call that + * arrives while an earlier one is still queued (not yet picked up by + * {@link #SAVE_EXECUTOR}'s single thread) can simply replace it instead + * of enqueuing a second, soon-to-be-overwritten write. Only one task is + * ever queued at a time; {@link #runPendingSave} is what drains it. + */ + private static final AtomicReference PENDING_SAVE = new AtomicReference<>(); + + private record PendingSave(UserConfig config, CompletableFuture future) { + } + + /** + * As {@link #save}, off the caller's thread -- the settings modal calls + * this from the FX thread, where a synchronous write is forbidden. The + * returned future completes exceptionally on failure so the caller can + * surface it; it is never swallowed. + * + *

Concurrent calls coalesce onto {@link #PENDING_SAVE} rather than + * racing independent threads (see {@link #SAVE_EXECUTOR}'s Javadoc). A + * call that gets superseded before it is written still has its returned + * future completed -- with the outcome of the save that superseded it -- + * so a caller awaiting it never hangs.

+ * + *

Tests: {@link #PENDING_SAVE} and {@link #SAVE_EXECUTOR} are + * static, i.e. shared by the whole test JVM. A test that calls this must + * call {@link #flushPendingSaves()} (in a {@code finally}, before its + * {@code @TempDir} is removed) so a write cannot leak into, or race, an + * unrelated test.

+ */ + public static CompletableFuture saveAsync(UserConfig config) { + CompletableFuture future = new CompletableFuture<>(); + PendingSave superseded = PENDING_SAVE.getAndSet(new PendingSave(config, future)); + if (superseded == null) { + SAVE_EXECUTOR.execute(UserConfig::runPendingSave); + } else { + future.whenComplete((ignoredValue, error) -> { + if (error != null) { + superseded.future().completeExceptionally(error); + } else { + superseded.future().complete(null); + } + }); + } + return future; + } + + private static void runPendingSave() { + PendingSave pending = PENDING_SAVE.getAndSet(null); + if (pending == null) { + // Nothing left to do: a coalesced call already claimed this slot. + return; + } + try { + save(pending.config(), defaultConfigFile()); + pending.future().complete(null); + } catch (IOException | RuntimeException e) { + pending.future().completeExceptionally(e); + } catch (Throwable t) { + // Anything beyond IOException/RuntimeException is unexpected, + // but this runs on SAVE_EXECUTOR's single background thread: an + // uncaught throwable here would kill that thread silently and + // every future saveAsync call would queue forever with its + // future never completing (SAVE_EXECUTOR.execute would still + // "succeed" -- ThreadPoolExecutor just replaces the dead worker + // -- but PENDING_SAVE's coalescing means a caller waiting on + // this specific future would hang). Complete it exceptionally + // first so no caller of saveAsync/flushPendingSaves is left + // waiting forever, then rethrow so the failure is still visible. + pending.future().completeExceptionally(t); + throw t; + } + } + + /** + * Awaits every {@link #saveAsync} queued so far (AGENTS.md: a service + * writing files from a background thread exposes a flush, so shutdown + * and tests do not race a pending write). Submitting a no-op to {@link + * #SAVE_EXECUTOR} and awaiting it works because the executor is single + * threaded and FIFO: the no-op cannot run until every save task queued + * ahead of it has finished, so awaiting the no-op transitively awaits + * all of them -- same trick as {@code AnnotationStore.flushPendingSaves}. + * Bounded, because a wedged disk must not hang shutdown -- the atomic + * move means the worst case is a stale file, never a corrupt one. + */ + public static void flushPendingSaves() { + try { + SAVE_EXECUTOR.submit(() -> { }).get(10, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } catch (ExecutionException | TimeoutException e) { + // saveAsync's own future already reported the failure to its caller. + } + } } diff --git a/app/src/main/java/app/drydock/domain/WorkspaceUiState.java b/app/src/main/java/app/drydock/domain/WorkspaceUiState.java index a195af6..062e67c 100644 --- a/app/src/main/java/app/drydock/domain/WorkspaceUiState.java +++ b/app/src/main/java/app/drydock/domain/WorkspaceUiState.java @@ -7,7 +7,8 @@ /** * The subset of workspace UI state relevant to Milestone 4 (plan section * 10.3): which repository is selected, the sidebar width, and which - * repository nodes are expanded. + * repository nodes are expanded. Also includes cosmetic preferences for + * interface and terminal font sizes. * *

Plan section 10.3 also lists workspace-state fields that belong to * later milestones (selected session, open tabs, right-pane width, last @@ -21,11 +22,22 @@ public record WorkspaceUiState( Optional selectedRepositoryId, double sidebarWidth, Set expandedRepositoryIds, - UiTheme theme + UiTheme theme, + double uiFontSize, + double terminalFontSize ) { /** Design default 288px (handoff README section 2), clamped 220-520 at the SplitPane. */ public static final double DEFAULT_SIDEBAR_WIDTH = 288.0; + /** + * The app.css {@code .root} base size, and therefore the divisor every + * interface-scale factor is computed against (see {@code UiFontScale}). + */ + public static final double DEFAULT_UI_FONT_SIZE = 13.0; + + /** Matches the interface default so terminals start visually consistent with the UI. */ + public static final double DEFAULT_TERMINAL_FONT_SIZE = 13.0; + public WorkspaceUiState { Objects.requireNonNull(selectedRepositoryId, "selectedRepositoryId"); expandedRepositoryIds = Set.copyOf(Objects.requireNonNull(expandedRepositoryIds, "expandedRepositoryIds")); @@ -33,22 +45,37 @@ public record WorkspaceUiState( } public static WorkspaceUiState empty() { - return new WorkspaceUiState(Optional.empty(), DEFAULT_SIDEBAR_WIDTH, Set.of(), UiTheme.DARK); + return new WorkspaceUiState(Optional.empty(), DEFAULT_SIDEBAR_WIDTH, Set.of(), UiTheme.DARK, + DEFAULT_UI_FONT_SIZE, DEFAULT_TERMINAL_FONT_SIZE); } public WorkspaceUiState withSelectedRepositoryId(Optional newSelectedRepositoryId) { - return new WorkspaceUiState(newSelectedRepositoryId, sidebarWidth, expandedRepositoryIds, theme); + return new WorkspaceUiState(newSelectedRepositoryId, sidebarWidth, expandedRepositoryIds, theme, + uiFontSize, terminalFontSize); } public WorkspaceUiState withSidebarWidth(double newSidebarWidth) { - return new WorkspaceUiState(selectedRepositoryId, newSidebarWidth, expandedRepositoryIds, theme); + return new WorkspaceUiState(selectedRepositoryId, newSidebarWidth, expandedRepositoryIds, theme, + uiFontSize, terminalFontSize); } public WorkspaceUiState withExpandedRepositoryIds(Set newExpandedRepositoryIds) { - return new WorkspaceUiState(selectedRepositoryId, sidebarWidth, newExpandedRepositoryIds, theme); + return new WorkspaceUiState(selectedRepositoryId, sidebarWidth, newExpandedRepositoryIds, theme, + uiFontSize, terminalFontSize); } public WorkspaceUiState withTheme(UiTheme newTheme) { - return new WorkspaceUiState(selectedRepositoryId, sidebarWidth, expandedRepositoryIds, newTheme); + return new WorkspaceUiState(selectedRepositoryId, sidebarWidth, expandedRepositoryIds, newTheme, + uiFontSize, terminalFontSize); + } + + public WorkspaceUiState withUiFontSize(double newUiFontSize) { + return new WorkspaceUiState(selectedRepositoryId, sidebarWidth, expandedRepositoryIds, theme, + newUiFontSize, terminalFontSize); + } + + public WorkspaceUiState withTerminalFontSize(double newTerminalFontSize) { + return new WorkspaceUiState(selectedRepositoryId, sidebarWidth, expandedRepositoryIds, theme, + uiFontSize, newTerminalFontSize); } } diff --git a/app/src/main/java/app/drydock/state/ApplicationStateCodec.java b/app/src/main/java/app/drydock/state/ApplicationStateCodec.java index bc6a323..bf66918 100644 --- a/app/src/main/java/app/drydock/state/ApplicationStateCodec.java +++ b/app/src/main/java/app/drydock/state/ApplicationStateCodec.java @@ -69,7 +69,10 @@ * "ui": { * "selectedRepositoryId": "" | null, * "sidebarWidth": 260.0, - * "expandedRepositoryIds": ["", ...] + * "expandedRepositoryIds": ["", ...], + * "theme": "DARK" | "LIGHT", + * "uiFontSize": 13.0, + * "terminalFontSize": 13.0 * } * } * } @@ -204,6 +207,8 @@ private static JsonValue uiToJson(WorkspaceUiState ui) { } obj.put("expandedRepositoryIds", new JsonArray(expanded)); obj.put("theme", new JsonString(ui.theme().name())); + obj.put("uiFontSize", JsonNumber.of(ui.uiFontSize())); + obj.put("terminalFontSize", JsonNumber.of(ui.terminalFontSize())); return obj; } @@ -375,7 +380,18 @@ private static WorkspaceUiState uiFromJson(JsonObject obj) { ? UiTheme.fromPersisted(s.value()) : UiTheme.DARK; - return new WorkspaceUiState(selected, sidebarWidth, expanded, theme); + // Cosmetic, so lenient like sidebarWidth: absent or non-numeric + // falls back to the design default. Deliberately NOT clamped here + // -- ThemeManager and TerminalThemes own their ranges at the point + // of application, so a hand-edited value is never silently rewritten. + double uiFontSize = obj.get("uiFontSize") instanceof JsonNumber n + ? n.asDouble() + : WorkspaceUiState.DEFAULT_UI_FONT_SIZE; + double terminalFontSize = obj.get("terminalFontSize") instanceof JsonNumber n + ? n.asDouble() + : WorkspaceUiState.DEFAULT_TERMINAL_FONT_SIZE; + + return new WorkspaceUiState(selected, sidebarWidth, expanded, theme, uiFontSize, terminalFontSize); } private static int readSchemaVersion(JsonObject root) { diff --git a/app/src/main/java/app/drydock/ui/AppShell.java b/app/src/main/java/app/drydock/ui/AppShell.java index 309f214..9623446 100644 --- a/app/src/main/java/app/drydock/ui/AppShell.java +++ b/app/src/main/java/app/drydock/ui/AppShell.java @@ -42,13 +42,15 @@ public final class AppShell { private boolean sidebarCollapsed; private double savedSidebarWidth = -1; + private Runnable onShowSettings = () -> { }; + private final BorderPane shell = new BorderPane(); /** Slim strip shown in the sidebar's place while collapsed: an expand button + the ⌘0 hint. */ private final Region collapsedRail = buildCollapsedRail(); public AppShell(Stage stage, String title, Region sidebar, Region mainPane, - double initialSidebarWidth, UiTheme initialTheme, Consumer onThemeChanged, - double sceneWidth, double sceneHeight) { + double initialSidebarWidth, UiTheme initialTheme, double initialUiFontSize, + Consumer onThemeChanged, double sceneWidth, double sceneHeight) { this.sidebar = sidebar; sidebar.setMinWidth(SIDEBAR_MIN); @@ -58,6 +60,7 @@ public AppShell(Stage stage, String title, Region sidebar, Region mainPane, titleBar = new TitleBar(stage, title, () -> showShortcutsOverlay(), + this::showSettings, () -> toggleTheme(), () -> toggleSidebar()); @@ -67,7 +70,7 @@ public AppShell(Stage stage, String title, Region sidebar, Region mainPane, StackPane root = new StackPane(shell, modalLayer); scene = new Scene(root, sceneWidth, sceneHeight); - themeManager = new ThemeManager(scene, initialTheme, theme -> { + themeManager = new ThemeManager(scene, initialTheme, initialUiFontSize, theme -> { titleBar.showThemeGlyphFor(theme); onThemeChanged.accept(theme); }); @@ -147,4 +150,13 @@ public void toggleTheme() { public void showShortcutsOverlay() { modalLayer.show(ShortcutsOverlay.create(modalLayer::close)); } + + /** Wires the gear button and ⌘, to the settings modal (supplied by the application). */ + public void setOnShowSettings(Runnable onShowSettings) { + this.onShowSettings = onShowSettings == null ? () -> { } : onShowSettings; + } + + public void showSettings() { + onShowSettings.run(); + } } diff --git a/app/src/main/java/app/drydock/ui/MainWorkspace.java b/app/src/main/java/app/drydock/ui/MainWorkspace.java index 4c564e7..113d10a 100644 --- a/app/src/main/java/app/drydock/ui/MainWorkspace.java +++ b/app/src/main/java/app/drydock/ui/MainWorkspace.java @@ -15,6 +15,7 @@ import app.drydock.domain.SessionStatus; import app.drydock.domain.SshRemote; import app.drydock.domain.UiTheme; +import app.drydock.domain.WorkspaceUiState; import app.drydock.git.ChangedLineService; import app.drydock.git.DiffService; import app.drydock.git.GhCliService; @@ -75,6 +76,7 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.function.DoubleSupplier; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -197,6 +199,18 @@ public final class MainWorkspace extends BorderPane implements WorkspaceNavigato /** Current UI theme, for terminal config selection; wired by DrydockApplication once the shell exists. */ private Supplier themeProvider = () -> UiTheme.DARK; + /** Where new and re-themed terminals read the persisted font size from. */ + private DoubleSupplier terminalFontSizeProvider = () -> WorkspaceUiState.DEFAULT_TERMINAL_FONT_SIZE; + + /** + * The most recently requested (theme, fontSize) pair passed to {@link + * #applyTerminalConfig}, read back inside its callback to drop a + * superseded result -- see that method's Javadoc. FX-thread-only, like + * the rest of this class. + */ + private UiTheme pendingTerminalTheme; + private double pendingTerminalFontSize = Double.NaN; + /** * True while a modal is showing. The ghostty terminal is a NATIVE view * stacked above the whole JavaFX scene, so it would paint over any @@ -443,12 +457,70 @@ public void setThemeProvider(Supplier provider) { this.themeProvider = provider == null ? () -> UiTheme.DARK : provider; } - /** Re-themes every open terminal to {@code theme} (called on the FX thread by the theme toggle). */ + /** Wires where terminals read the configured font size from (settings modal). */ + public void setTerminalFontSizeProvider(DoubleSupplier provider) { + this.terminalFontSizeProvider = + provider == null ? () -> WorkspaceUiState.DEFAULT_TERMINAL_FONT_SIZE : provider; + } + + /** + * Re-applies the terminal config to every open terminal (called on the FX + * thread by the theme toggle). ghostty re-reads the whole config file, so + * theme and font size travel together over this one path -- which is + * exactly why the size lives in the config rather than in the per-surface + * struct, where {@code ghostty_surface_update_config} would discard it. + * The terminal font size for this call comes from {@link + * #terminalFontSizeProvider}, whereas live preview uses {@link + * #previewTerminalFontSize} to bypass persisted state during a slider drag. + */ public void applyTerminalTheme(UiTheme theme) { - Path configFile = TerminalThemes.configFileFor(theme); - for (OpenSessionTab open : openTabs.values()) { - open.applyTerminalTheme(configFile); - } + applyTerminalConfig(theme, terminalFontSizeProvider.getAsDouble()); + } + + /** + * Live preview of a terminal font size as the settings modal's slider + * moves: applied immediately to every open surface, bypassing + * {@link #terminalFontSizeProvider} (which reads persisted state) since + * the caller persists separately -- see {@link SizeSetting}. Reuses + * {@link #applyTerminalConfig}, so a rapid sequence of previews cannot + * block the FX thread either. + */ + public void previewTerminalFontSize(double fontSize) { + applyTerminalConfig(themeProvider.get(), fontSize); + } + + /** + * Extracts (or looks up) the config for {@code (theme, fontSize)} and + * applies it to every open terminal. Extraction happens off the FX + * thread on a cache miss (see {@link TerminalThemes#configFileForAsync}), + * which matters here because both callers above can fire on every tick + * of a slider drag; {@code openTabs} is read again inside the callback + * rather than captured up front, so a tab opened or closed while + * extraction is in flight is still handled correctly. + * + *

{@code (theme, fontSize)} is recorded as the pending request before + * the lookup starts, and re-checked when the result lands, exactly as + * {@link ThemeManager#setUiFontSize} does for the interface stylesheet: + * {@code configFileForAsync}'s cache misses each spawn their own virtual + * thread, and the monitor inside {@link TerminalThemes#configFileFor} is + * released before {@code Platform.runLater} is even called, so nothing + * orders the callbacks -- a drag from size 10 to 18 can have the size-14 + * result land after the size-15 one. Dropping any callback that is no + * longer the latest request keeps the terminals in sync with the + * slider. A theme toggle always becomes the latest request (it is + * always the most recent call), so it always applies.

+ */ + private void applyTerminalConfig(UiTheme theme, double fontSize) { + pendingTerminalTheme = theme; + pendingTerminalFontSize = fontSize; + TerminalThemes.configFileForAsync(theme, fontSize, configFile -> { + if (theme != pendingTerminalTheme || fontSize != pendingTerminalFontSize) { + return; + } + for (OpenSessionTab open : openTabs.values()) { + open.applyTerminalTheme(configFile); + } + }); } public boolean hasOpenSessions() { @@ -1273,11 +1345,21 @@ private OpenSessionTab createOpenSessionTab(ManagedSessionId sessionId, String d // The wakeup coalescer already delivers on the FX thread with at most // one pending runnable; a second Platform.runLater here would defeat // that coalescing. + // + // configFileFor is called synchronously here (current theme, current + // size) rather than through configFileForAsync: this call site is + // warmed-by-construction -- see that method's Javadoc -- because + // DrydockApplication warms this exact pair at startup and every + // theme/size change re-warms it, so a filesystem touch here is the + // rare exception, not the rule. Restructuring TerminalRuntime + // creation to await the async variant would ripple into + // OpenSessionTab/SessionManager for a hit that already almost never + // happens. TerminalRuntime app = TerminalFactory.createRuntime(() -> { if (holder[0] != null) { holder[0].tickAndDraw(); } - }, Optional.of(TerminalThemes.configFileFor(themeProvider.get()))); + }, Optional.of(TerminalThemes.configFileFor(themeProvider.get(), terminalFontSizeProvider.getAsDouble()))); TerminalHostView host; try { host = TerminalFactory.createHostForCurrentWindow(); @@ -1301,8 +1383,10 @@ private OpenSessionTab createOpenSessionTab(ManagedSessionId sessionId, String d openTab.setShellWorkingDirectory(System.getProperty("user.home")); }); openTab.setShellTerminalProvider(onWakeup -> { + // Synchronous and warmed-by-construction for the same reason as + // the Claude runtime above. TerminalRuntime shellRuntime = TerminalFactory.createRuntime(onWakeup, - Optional.of(TerminalThemes.configFileFor(themeProvider.get()))); + Optional.of(TerminalThemes.configFileFor(themeProvider.get(), terminalFontSizeProvider.getAsDouble()))); TerminalHostView shellHost; try { shellHost = TerminalFactory.createHostForCurrentWindow(); diff --git a/app/src/main/java/app/drydock/ui/SettingsModal.java b/app/src/main/java/app/drydock/ui/SettingsModal.java new file mode 100644 index 0000000..08fdd22 --- /dev/null +++ b/app/src/main/java/app/drydock/ui/SettingsModal.java @@ -0,0 +1,315 @@ +package app.drydock.ui; + +import app.drydock.domain.UiTheme; +import javafx.application.Platform; +import javafx.geometry.Pos; +import javafx.scene.control.Button; +import javafx.scene.control.Label; +import javafx.scene.control.RadioButton; +import javafx.scene.control.Slider; +import javafx.scene.control.TextField; +import javafx.scene.control.ToggleGroup; +import javafx.scene.layout.HBox; +import javafx.scene.layout.Priority; +import javafx.scene.layout.Region; +import javafx.scene.layout.VBox; +import javafx.stage.DirectoryChooser; + +import java.io.File; +import java.nio.file.Path; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; + +/** + * The Settings modal, reached from the title-bar gear or ⌘,. Four settings, + * applied as they change (the macOS preferences convention -- there is no + * OK/Cancel; Done, Esc, and × all just close). + * + *

Holds no manager or store: everything it can change arrives as a + * callback in {@link Settings}, so persistence stays with the single state + * writer and this class stays a view.

+ * + *

The theme radio reads {@link Settings#theme()} once, at construction, + * and never again -- so nothing else may change the theme while this modal + * is open, or the radio would silently drift from reality with no way for + * the user to click it back (a same-value selection fires no {@code + * ToggleGroup} event). {@code DrydockApplication}'s ⌘⇧L branch is gated on + * {@code modalLayer().isShowingModal()} for exactly this reason.

+ */ +public final class SettingsModal extends VBox { + + /** Everything the modal reads and writes, supplied by the application wiring. */ + public interface Settings { + UiTheme theme(); + + void setTheme(UiTheme theme); + + /** The interface font size: read, applied live, and persisted (see {@link SizeSetting}). */ + SizeSetting interfaceSize(); + + /** The terminal font size, with the same three parts as {@link #interfaceSize()}. */ + SizeSetting terminalSize(); + + CompletableFuture> loadWorktreesDirectory(); + + CompletableFuture saveWorktreesDirectory(Optional directory); + } + + private static final double MODAL_WIDTH = 520; + + /** + * Flushes a pending worktrees-directory edit, wired by {@link + * #worktreesRow} and invoked by {@code ModalLayer}'s {@code onClosed} + * callback (see {@link #flushPendingEdit}) so a typed value is + * committed no matter which of Done/×/Esc/backdrop-click closes the + * modal -- only Done and × happen to move focus off the field first. + * A no-op until {@link #worktreesRow} runs. + */ + private Runnable pendingWorktreesFlush = () -> { }; + + public SettingsModal(Settings settings, Runnable onClose) { + getStyleClass().add("modal"); + setMaxWidth(MODAL_WIDTH); + setMaxHeight(Region.USE_PREF_SIZE); + setSpacing(12); + + Label title = new Label("Settings"); + title.getStyleClass().add("modal-title"); + Button close = new Button("×"); + close.getStyleClass().add("icon-button"); + close.setOnAction(e -> onClose.run()); + Region headerSpacer = new Region(); + HBox.setHgrow(headerSpacer, Priority.ALWAYS); + HBox header = new HBox(8, title, headerSpacer, close); + header.setAlignment(Pos.CENTER_LEFT); + + Button done = new Button("Done"); + done.getStyleClass().add("worktree-create-button"); + done.setDefaultButton(true); + done.setOnAction(e -> onClose.run()); + Region footerSpacer = new Region(); + HBox.setHgrow(footerSpacer, Priority.ALWAYS); + HBox footer = new HBox(8, footerSpacer, done); + + getChildren().addAll(header, + sectionTitle("Appearance"), + themeRow(settings), + // The interface slider keeps its 0.5px resolution -- UiFontScale genuinely + // honours fractional sizes. The terminal slider is integer-only (see sizeRow): + // TerminalThemes rounds to int, so a fractional readout there would lie. + sizeRow("Interface size", UiFontScale.MIN_FONT_SIZE, UiFontScale.MAX_FONT_SIZE, + true, settings.interfaceSize()), + sizeRow("Terminal size", TerminalThemes.MIN_FONT_SIZE, TerminalThemes.MAX_FONT_SIZE, + false, settings.terminalSize()), + sectionTitle("Worktrees"), + worktreesRow(settings), + footer); + } + + /** + * Commits a pending worktrees-directory edit, if any. Meant to be + * passed as {@code ModalLayer}'s {@code onClosed} callback: Esc and a + * backdrop click hide the modal without ever moving focus off the text + * field, so the field's own focus-lost commit never fires and a typed + * path would otherwise be silently discarded. Idempotent -- see + * {@link #worktreesRow}'s {@code commit} -- so calling this after Done + * or × (which already committed via focus loss) is harmless. + */ + public void flushPendingEdit() { + pendingWorktreesFlush.run(); + } + + private static Label sectionTitle(String text) { + Label label = new Label(text); + label.getStyleClass().add("settings-section-title"); + return label; + } + + private static Region themeRow(Settings settings) { + ToggleGroup group = new ToggleGroup(); + RadioButton dark = new RadioButton("Dark"); + RadioButton light = new RadioButton("Light"); + dark.getStyleClass().add("settings-radio"); + light.getStyleClass().add("settings-radio"); + dark.setToggleGroup(group); + light.setToggleGroup(group); + (settings.theme() == UiTheme.LIGHT ? light : dark).setSelected(true); + + // Applied on selection, not on Done: the whole modal is apply-on-change. + // `selected == light` is the real test; the `dark` fallback below is + // unreachable via the UI (the group always has one toggle selected, + // and DrydockApplication's cmd+shift+L branch is gated on the modal + // being closed -- see the class Javadoc), but naming it explicitly + // beats letting a null toggle silently coerce to DARK. + group.selectedToggleProperty().addListener((obs, old, selected) -> + settings.setTheme(selected == light ? UiTheme.LIGHT + : selected == dark ? UiTheme.DARK + : settings.theme())); + + return labelled("Theme", new HBox(12, dark, light)); + } + + /** A settings row: a fixed-width caption on the left, the control on the right. */ + private static Region labelled(String caption, Region control) { + Label label = new Label(caption); + label.getStyleClass().add("settings-row-label"); + label.setMinWidth(120); + HBox row = new HBox(12, label, control); + row.setAlignment(Pos.CENTER_LEFT); + HBox.setHgrow(control, Priority.ALWAYS); + return row; + } + + /** + * A font-size slider. Every tick applies the size live, so the effect is + * visible immediately, while persisting is left to {@link SizeSetting} -- + * which needs to know whether the tick is part of a drag, since a state + * write per pixel would be pointless disk traffic. + * {@link Slider#valueChangingProperty()} is the only signal for that: it + * is true for the whole span of a mouse drag, and this row is where the + * two events (a tick, and a drag's release) are mapped onto that type's + * two entry points. + * + * @param halfStepResolution whether the slider snaps to 0.5 as well as + * whole values (the interface slider does, + * because {@link UiFontScale} honours the + * fraction; the terminal slider does not, + * because {@link TerminalThemes} rounds to + * int, so a fractional readout there would + * lie about what the terminal actually renders) + */ + private static Region sizeRow(String caption, double min, double max, + boolean halfStepResolution, SizeSetting setting) { + Slider slider = new Slider(min, max, Math.clamp(setting.current(), min, max)); + slider.getStyleClass().add("settings-slider"); + slider.setMajorTickUnit(1); + slider.setMinorTickCount(halfStepResolution ? 1 : 0); + slider.setSnapToTicks(true); + slider.setBlockIncrement(1); + + Label value = new Label(format(slider.getValue())); + value.getStyleClass().add("settings-value"); + value.setMinWidth(52); + + slider.valueProperty().addListener((obs, old, now) -> { + value.setText(format(now.doubleValue())); + setting.changed(now.doubleValue(), slider.isValueChanging()); + }); + slider.valueChangingProperty().addListener((obs, was, changing) -> { + if (!changing) { + setting.dragEnded(slider.getValue()); + } + }); + + HBox control = new HBox(10, slider, value); + control.setAlignment(Pos.CENTER_LEFT); + HBox.setHgrow(slider, Priority.ALWAYS); + return labelled(caption, control); + } + + /** "13 px" / "13.5 px" -- no trailing zero on whole sizes. */ + static String format(double size) { + return (size == Math.rint(size) + ? String.valueOf((int) size) + : String.valueOf(Math.round(size * 10) / 10.0)) + " px"; + } + + private Region worktreesRow(Settings settings) { + // Reuses the New-worktree modal's classes rather than inventing new + // ones: this is the same kind of control doing the same job (a path + // input and its secondary action), and unstyled stock controls fall + // back to modena's light defaults, which render as a white field and + // a grey 3D button inside the dark modal. + TextField field = new TextField(); + field.getStyleClass().add("worktree-field"); + field.setPromptText("Loading…"); + field.setDisable(true); + Button browse = new Button("Browse…"); + browse.getStyleClass().add("worktree-cancel-button"); + browse.setDisable(true); + + Label hint = new Label("New worktrees are created here."); + hint.getStyleClass().add("settings-hint"); + + // The text last committed (or loaded), so `commit` below can tell a + // real edit from a no-op close and never fire a redundant save -- + // and so a `commit` re-entered mid-save (see `committing`) has + // something stable to compare against. Starts at "" to match the + // field's initial (disabled, empty) text, so a close raced against + // the load below commits nothing rather than saving an empty + // directory over whatever is actually on disk. + String[] lastCommitted = {""}; + // Re-entrancy guard: `commit` disables the field while it is the + // focus owner, which JavaFX resolves by moving focus off it -- + // synchronously re-entering `commit` via the focusedProperty + // listener below, before the outer call has even reached + // saveWorktreesDirectory. Without this, one keystroke's Enter can + // fire two overlapping saves. + boolean[] committing = {false}; + + // Load off the FX thread (UserConfig reads the file); the controls + // stay disabled with a "Loading…" prompt until it lands, so the row + // never shows a stale or empty value as if it were the real one. + settings.loadWorktreesDirectory().whenComplete((directory, failure) -> Platform.runLater(() -> { + field.setDisable(false); + browse.setDisable(false); + field.setPromptText(System.getProperty("user.home") + "/dev/wt"); + if (failure == null) { + String text = directory.map(Path::toString).orElse(""); + field.setText(text); + lastCommitted[0] = text; + } else { + UiErrors.show("Could not read the settings file", failure); + } + })); + + Runnable commit = () -> { + String text = field.getText() == null ? "" : field.getText().strip(); + if (committing[0] || text.equals(lastCommitted[0])) { + return; + } + Optional directory = text.isEmpty() ? Optional.empty() : Optional.of(Path.of(text)); + committing[0] = true; + field.setDisable(true); + browse.setDisable(true); + settings.saveWorktreesDirectory(directory).whenComplete((ignored, failure) -> + Platform.runLater(() -> { + // saveWorktreesDirectory is a one-line delegation to + // UserConfig.saveAsync: success and failure are its + // only two completions, so both are handled here + // together, unconditionally re-enabling the row. + committing[0] = false; + field.setDisable(false); + browse.setDisable(false); + if (failure != null) { + UiErrors.show("Could not save the worktrees directory", failure); + } else { + lastCommitted[0] = text; + } + })); + }; + pendingWorktreesFlush = commit; + + field.setOnAction(e -> commit.run()); + field.focusedProperty().addListener((obs, had, has) -> { + if (!has) { + commit.run(); + } + }); + + browse.setOnAction(e -> { + DirectoryChooser chooser = new DirectoryChooser(); + chooser.setTitle("Choose the worktrees directory"); + File chosen = chooser.showDialog(getScene() == null ? null : getScene().getWindow()); + if (chosen != null) { + field.setText(chosen.getAbsolutePath()); + commit.run(); + } + }); + + HBox control = new HBox(8, field, browse); + control.setAlignment(Pos.CENTER_LEFT); + HBox.setHgrow(field, Priority.ALWAYS); + return new VBox(4, labelled("Directory", control), hint); + } +} diff --git a/app/src/main/java/app/drydock/ui/ShortcutsOverlay.java b/app/src/main/java/app/drydock/ui/ShortcutsOverlay.java index be89363..eb5192a 100644 --- a/app/src/main/java/app/drydock/ui/ShortcutsOverlay.java +++ b/app/src/main/java/app/drydock/ui/ShortcutsOverlay.java @@ -30,6 +30,7 @@ final class ShortcutsOverlay { {"Toggle sidebar", "⌘0"}, {"Filter repositories", "⌘F"}, {"Toggle theme", "⌘⇧L"}, + {"Settings", "⌘,"}, {"Cancel / close", "Esc"}, }; diff --git a/app/src/main/java/app/drydock/ui/SizeSetting.java b/app/src/main/java/app/drydock/ui/SizeSetting.java new file mode 100644 index 0000000..8f07f3c --- /dev/null +++ b/app/src/main/java/app/drydock/ui/SizeSetting.java @@ -0,0 +1,63 @@ +package app.drydock.ui; + +import java.util.function.DoubleConsumer; +import java.util.function.DoubleSupplier; + +/** + * One font-size setting the {@link SettingsModal} drives: where the slider's + * opening value comes from, how a size is applied live, and how a size is + * persisted. Owning the apply/persist sequencing here -- rather than in the + * modal's slider listener and the application's anonymous {@link + * SettingsModal.Settings} -- is what makes it testable without a JavaFX + * toolkit (see {@code SizeSettingTest}). + * + *

{@link #persist} is handed the value the user chose, never {@link + * #currentSize}. The distinction is the whole point of this type: applying a + * size can complete asynchronously (a first visit to a size regenerates the + * stylesheet or the terminal config off-thread), while a commit can land in + * the very same FX event as the apply that triggered it -- an arrow key or a + * track click never sets {@code Slider.valueChanging}, so there is no drag + * span to defer the commit past. A commit that read the applied state back + * would then store the previous size, silently, until the next launch. Making + * the persisted value an argument removes the ordering assumption entirely: + * what the slider shows and what gets stored are the same number by + * construction.

+ * + * @param currentSize the size the slider should show when the modal opens. + * For interface size, this reads the theme manager's + * applied size; for terminal size, it reads persisted state. + * Read once when the modal builds the row; not consulted + * afterwards + * @param applyLive applies a size to the live UI, possibly completing + * asynchronously; called on every slider tick, so it must + * not block the FX thread + * @param persist writes a size to persistent state; called once per + * discrete change or per finished drag, never per tick + */ +public record SizeSetting(DoubleSupplier currentSize, DoubleConsumer applyLive, DoubleConsumer persist) { + + /** The size to show when the modal opens. */ + public double current() { + return currentSize.getAsDouble(); + } + + /** + * One {@code Slider.valueProperty} tick. While a drag is in progress the + * size is only applied -- persisting per tick would be pointless disk + * traffic, and {@link #dragEnded} covers the release. A discrete change + * (arrow key, or a click landing straight on a value) never reports a + * drag, so it applies and persists in one go, which is correct: there is + * no burst to debounce. + */ + public void changed(double size, boolean dragging) { + applyLive.accept(size); + if (!dragging) { + persist.accept(size); + } + } + + /** The end of a drag: persist where the slider came to rest. */ + public void dragEnded(double size) { + persist.accept(size); + } +} diff --git a/app/src/main/java/app/drydock/ui/TerminalThemes.java b/app/src/main/java/app/drydock/ui/TerminalThemes.java index 29d3ac1..326ae61 100644 --- a/app/src/main/java/app/drydock/ui/TerminalThemes.java +++ b/app/src/main/java/app/drydock/ui/TerminalThemes.java @@ -1,44 +1,127 @@ package app.drydock.ui; import app.drydock.domain.UiTheme; +import javafx.application.Platform; import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; +import java.lang.System.Logger; +import java.lang.System.Logger.Level; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.StandardCopyOption; -import java.util.EnumMap; +import java.util.HashMap; import java.util.Map; +import java.util.Optional; +import java.util.function.Consumer; /** - * Provides the on-disk ghostty config file matching each {@link UiTheme}. - * The configs live as classpath resources ({@code terminal-dark.conf} / - * {@code terminal-light.conf}, kept next to the theme CSS whose tokens - * they mirror) but {@code ghostty_config_load_file} needs a real path, so - * they are extracted once per process into a private temp directory. + * Provides the on-disk ghostty config file matching each {@link UiTheme} and + * terminal font size. The theme bodies live as classpath resources ({@code + * terminal-dark.conf} / {@code terminal-light.conf}, kept next to the theme + * CSS whose tokens they mirror) but {@code ghostty_config_load_file} needs a + * real path, so each (theme, size) pair is materialised once per process + * into a private temp directory. + * + *

The font size rides in the config rather than in {@code + * ghostty_surface_config_s.font_size} deliberately: a theme toggle calls + * {@code ghostty_surface_update_config}, which re-derives a running + * surface's configuration from the app config and would drop a per-surface + * override, silently resetting the user's terminal size mid-session. Going + * through the config file instead means a size change applies live to + * running surfaces over the very same path.

*/ final class TerminalThemes { - private static final Map EXTRACTED = new EnumMap<>(UiTheme.class); + private static final Logger LOG = System.getLogger(TerminalThemes.class.getName()); + + /** Supported terminal font sizes; the settings slider exposes the same band. */ + static final double MIN_FONT_SIZE = 10.0; + static final double MAX_FONT_SIZE = 18.0; + + private static final Map EXTRACTED = new HashMap<>(); + + /** Cache key: one materialised config file per theme and rounded font size. */ + private record Key(UiTheme theme, int fontSize) { + } private TerminalThemes() { } - static synchronized Path configFileFor(UiTheme theme) { - return EXTRACTED.computeIfAbsent(theme, TerminalThemes::extract); + /** + * The config file for {@code theme} at {@code fontSize} points, clamped + * to {@link #MIN_FONT_SIZE}..{@link #MAX_FONT_SIZE} -- this is the point + * of application, and therefore the owner of the range (the codec + * deliberately stores whatever it was given). + * + *

Touches the filesystem on a cache miss. Safe to call on the FX + * thread only when the caller is warmed-by-construction: {@code + * MainWorkspace.createOpenSessionTab} calls this synchronously for the + * current (theme, terminal font size) pair, which {@code + * DrydockApplication} warms once at startup and {@code MainWorkspace} + * re-warms (through {@link #configFileForAsync}) on every theme toggle + * and terminal-size change, so a session-open call is a cache hit in + * the common case. That is not a guarantee, only the common case: a + * session opened fast enough to outrun a just-issued warm still falls + * back to doing the extraction inline, on the FX thread. A caller that + * cannot make the warmed-by-construction argument (a live slider drag, + * in particular) must go through {@link #configFileForAsync} + * instead.

+ */ + static synchronized Path configFileFor(UiTheme theme, double fontSize) { + int rounded = roundedFontSize(fontSize); + return EXTRACTED.computeIfAbsent(new Key(theme, rounded), TerminalThemes::extract); + } + + /** Already-extracted config file for {@code (theme, fontSize)}, with no filesystem access at all. */ + private static synchronized Optional cachedConfigFileFor(UiTheme theme, double fontSize) { + return Optional.ofNullable(EXTRACTED.get(new Key(theme, roundedFontSize(fontSize)))); + } + + private static int roundedFontSize(double fontSize) { + return (int) Math.round(Math.clamp(fontSize, MIN_FONT_SIZE, MAX_FONT_SIZE)); + } + + /** + * As {@link #configFileFor}, but safe to call from the FX thread + * unconditionally: a cache hit resolves {@code onReady} synchronously + * with no I/O, and a cache miss extracts on a virtual thread and hands + * the result back via {@link Platform#runLater}. A failure is logged and + * {@code onReady} is simply never called, leaving whatever config is + * currently applied in place. + */ + static void configFileForAsync(UiTheme theme, double fontSize, Consumer onReady) { + Optional cached = cachedConfigFileFor(theme, fontSize); + if (cached.isPresent()) { + onReady.accept(cached.get()); + return; + } + Thread.ofVirtual().start(() -> { + Path file; + try { + file = configFileFor(theme, fontSize); + } catch (UncheckedIOException e) { + LOG.log(Level.WARNING, "Could not extract the terminal theme config for " + theme + + " at size " + fontSize + "; keeping the current config", e); + return; + } + Platform.runLater(() -> onReady.accept(file)); + }); } - private static Path extract(UiTheme theme) { - String resource = theme == UiTheme.LIGHT ? "terminal-light.conf" : "terminal-dark.conf"; + private static Path extract(Key key) { + String resource = key.theme() == UiTheme.LIGHT ? "terminal-light.conf" : "terminal-dark.conf"; try (InputStream stream = TerminalThemes.class.getResourceAsStream("/app/drydock/ui/" + resource)) { if (stream == null) { throw new IllegalStateException("Missing bundled terminal theme resource: " + resource); } + String body = new String(stream.readAllBytes(), StandardCharsets.UTF_8); Path dir = Files.createTempDirectory("drydock-terminal-theme"); dir.toFile().deleteOnExit(); Path file = dir.resolve(resource); - Files.copy(stream, file, StandardCopyOption.REPLACE_EXISTING); + Files.writeString(file, body + System.lineSeparator() + + "font-size = " + key.fontSize() + System.lineSeparator(), StandardCharsets.UTF_8); file.toFile().deleteOnExit(); return file; } catch (IOException e) { diff --git a/app/src/main/java/app/drydock/ui/ThemeManager.java b/app/src/main/java/app/drydock/ui/ThemeManager.java index b54c0ce..d821e08 100644 --- a/app/src/main/java/app/drydock/ui/ThemeManager.java +++ b/app/src/main/java/app/drydock/ui/ThemeManager.java @@ -11,10 +11,11 @@ /** * Runtime theming (design handoff "Target stack"): the scene always carries - * {@code app.css} (structure, no colors) plus exactly one of {@code - * theme-dark.css} / {@code theme-light.css} (color tokens only). Toggling - * swaps the token sheet in place; persistence of the choice is delegated to - * the {@code onThemeChanged} callback so this class stays free of any state + * a (possibly font-scaled, see {@link UiFontScale}) {@code app.css} + * (structure, no colors) plus exactly one of {@code theme-dark.css} / + * {@code theme-light.css} (color tokens only). Toggling swaps the token + * sheet in place; persistence of the choice is delegated to the {@code + * onThemeChanged} callback so this class stays free of any state * dependency. * *

Also owns one-time registration of the bundled JetBrains Mono faces @@ -38,11 +39,17 @@ public final class ThemeManager { private final Scene scene; private final Consumer onThemeChanged; private UiTheme theme; + private double uiFontSize; + private double pendingUiFontSize; + private String appliedStylesheetUrl; - public ThemeManager(Scene scene, UiTheme initialTheme, Consumer onThemeChanged) { + public ThemeManager(Scene scene, UiTheme initialTheme, double initialUiFontSize, + Consumer onThemeChanged) { this.scene = scene; this.onThemeChanged = onThemeChanged; this.theme = initialTheme; + this.uiFontSize = Math.clamp(initialUiFontSize, UiFontScale.MIN_FONT_SIZE, UiFontScale.MAX_FONT_SIZE); + this.pendingUiFontSize = this.uiFontSize; loadBundledFonts(); apply(); } @@ -51,16 +58,90 @@ public UiTheme theme() { return theme; } + public double uiFontSize() { + return uiFontSize; + } + public void toggle() { - theme = theme.other(); + setTheme(theme.other()); + } + + /** Sets the theme absolutely (the settings modal's radio); {@link #toggle} delegates here. */ + public void setTheme(UiTheme newTheme) { + if (newTheme == theme) { + return; + } + theme = newTheme; apply(); onThemeChanged.accept(theme); } + /** + * Applies an interface font size by swapping in a stylesheet whose + * font sizes are scaled (see {@link UiFontScale}); clamped here because + * this is the point of application. Persisting the choice is the + * caller's job, exactly as with the theme. + * + *

Called on every tick of the settings modal's slider while the user + * drags, so it must never block the FX thread: the lookup goes through + * {@link UiFontScale#stylesheetForAsync}, which resolves synchronously + * on a cache hit and off-thread on a miss. {@code pendingUiFontSize} + * records the latest request and is re-checked when the async result + * lands, so a stale callback from a superseded drag position can never + * clobber a newer one -- the same pattern {@code MainWorkspace}'s + * {@code applyTerminalConfig} uses for the terminal config.

+ * + *

{@link #uiFontSize} therefore lags a request by one FX event on a + * cache miss, and callers must not read it back to learn what the user + * asked for -- persistence works from the value the user chose (see + * {@link SizeSetting}). What it does guarantee is that every size it + * holds has already been resolved to a stylesheet, hence cached, which + * is what {@link #apply} relies on.

+ */ + public void setUiFontSize(double newUiFontSize) { + double clamped = Math.clamp(newUiFontSize, UiFontScale.MIN_FONT_SIZE, UiFontScale.MAX_FONT_SIZE); + if (clamped == pendingUiFontSize) { + return; + } + pendingUiFontSize = clamped; + UiFontScale.stylesheetForAsync(clamped, url -> { + if (clamped != pendingUiFontSize) { + return; + } + uiFontSize = clamped; + if (!url.equals(appliedStylesheetUrl)) { + applyStylesheets(url); + } + }); + } + + /** + * Synchronous re-application, used by the constructor (before the stage + * is shown, where blocking is required, not merely tolerated: the scaled + * sheet has to be in place already, or the first layout would happen at + * the wrong size and visibly re-flow) and by {@link #setTheme} (a cache + * hit guaranteed: {@link #uiFontSize} only ever holds a size whose + * stylesheet has already been resolved -- see {@link #setUiFontSize} -- + * so this never touches disk). + */ private void apply() { - scene.getStylesheets().setAll( - resource("app.css"), - resource(theme.stylesheet())); + applyStylesheets(UiFontScale.stylesheetFor(uiFontSize)); + } + + /** + * Swaps in {@code fontSheetUrl} plus the current theme's token sheet, + * unconditionally -- {@link #setTheme} relies on that to always pick up + * the new theme resource, even when the font stylesheet URL is + * unchanged. {@link #setUiFontSize} carries its own "unchanged since + * last application" check before calling this (see its callback), since + * a slider drag re-running a full-scene CSS reapply on essentially + * every tick -- even though most ticks resolve to the same + * 0.5px-quantised stylesheet, see {@link UiFontScale#stylesheetFor} -- + * would otherwise be wasted work. + */ + private void applyStylesheets(String fontSheetUrl) { + appliedStylesheetUrl = fontSheetUrl; + scene.getStylesheets().setAll(fontSheetUrl, resource(theme.stylesheet())); } private static String resource(String name) { diff --git a/app/src/main/java/app/drydock/ui/TitleBar.java b/app/src/main/java/app/drydock/ui/TitleBar.java index b4d960b..89821fa 100644 --- a/app/src/main/java/app/drydock/ui/TitleBar.java +++ b/app/src/main/java/app/drydock/ui/TitleBar.java @@ -32,7 +32,7 @@ final class TitleBar extends StackPane { private double dragOffsetX; private double dragOffsetY; - TitleBar(Stage stage, String title, Runnable onHelp, Runnable onThemeToggle, + TitleBar(Stage stage, String title, Runnable onHelp, Runnable onSettings, Runnable onThemeToggle, Runnable onSidebarToggle) { getStyleClass().add("title-bar"); @@ -52,8 +52,10 @@ final class TitleBar extends StackPane { Button helpButton = iconButton("?", "Keyboard shortcuts (?)"); helpButton.setOnAction(e -> onHelp.run()); + Button settingsButton = iconButton("⚙", "Settings (⌘,)"); + settingsButton.setOnAction(e -> onSettings.run()); themeButton.setOnAction(e -> onThemeToggle.run()); - HBox right = new HBox(4, helpButton, themeButton); + HBox right = new HBox(4, helpButton, settingsButton, themeButton); right.setAlignment(Pos.CENTER_RIGHT); HBox sides = new HBox(lights, spacer(), right); diff --git a/app/src/main/java/app/drydock/ui/UiFontScale.java b/app/src/main/java/app/drydock/ui/UiFontScale.java new file mode 100644 index 0000000..0cc808a --- /dev/null +++ b/app/src/main/java/app/drydock/ui/UiFontScale.java @@ -0,0 +1,200 @@ +package app.drydock.ui; + +import app.drydock.domain.WorkspaceUiState; +import javafx.application.Platform; + +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.lang.System.Logger; +import java.lang.System.Logger.Level; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.function.Consumer; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Produces a copy of {@code app.css} with every {@code -fx-font-size} scaled + * by the user's interface size, materialised as a temp file whose URL the + * scene uses in place of the bundled sheet (the same extract-once-per-process + * pattern {@link TerminalThemes} uses for ghostty configs). + * + *

Two tempting alternatives are wrong. Rewriting the declarations as + * {@code em} compounds, because JavaFX resolves font-relative sizes against + * the font inherited from the nearest styleable ancestor, not against {@code + * .root}: a 12px rule containing an 11px rule would multiply both factors. + * An inline {@code -fx-font-size} on the scene root does override the {@code + * .root} rule, but never reaches combo popups, context menus, or tooltips -- + * they are separate scene graphs, which is exactly why app.css styles them + * with their own top-level selectors. A stylesheet reaches them; an inline + * style does not.

+ */ +final class UiFontScale { + + private static final Logger LOG = System.getLogger(UiFontScale.class.getName()); + + /** Supported interface font sizes; the settings slider exposes the same band. */ + static final double MIN_FONT_SIZE = 11.0; + static final double MAX_FONT_SIZE = 16.0; + + private static final String BASE_RESOURCE = "/app/drydock/ui/app.css"; + + /** + * Matches a {@code -fx-font-size} declaration's px literal (first + * alternative) or a standalone CSS comment, passed through untouched so + * text that merely looks like a declaration is never rewritten (second + * alternative). The declaration alternative is tried first and allows + * whitespace and comments between the colon and the value -- {@code + * -fx-font-size: /* TODO bump *}{@code / 12px;} must still be recognised + * as one declaration, or the comment would otherwise be consumed by the + * standalone-comment alternative first, leaving the px literal beyond it + * silently unscaled. Group 1 (property, separator, and any intervening + * comments, preserved verbatim) and group 2 (the number) are set when + * the declaration alternative matches; both are {@code null} when the + * standalone-comment alternative matched instead. + */ + private static final Pattern FONT_SIZE = Pattern.compile( + "(-fx-font-size\\s*:\\s*(?:/\\*.*?\\*/\\s*)*)(\\d+(?:\\.\\d+)?)px|/\\*.*?\\*/", + Pattern.DOTALL); + + private static final Map GENERATED = new HashMap<>(); + + private UiFontScale() { + } + + /** The bundled, unscaled sheet's URL. */ + static String baseStylesheetUrl() { + return UiFontScale.class.getResource(BASE_RESOURCE).toExternalForm(); + } + + /** + * The stylesheet URL for {@code fontSize}, clamped to {@link + * #MIN_FONT_SIZE}..{@link #MAX_FONT_SIZE} -- this is the point of + * application and therefore owns the range. The default size returns the + * bundled resource untouched; other sizes are generated once and cached. + * + *

Generation failures degrade to the bundled unscaled sheet: failures + * are logged, and the fallback is cached so no persisted interface size + * can cause a launch failure. The unhandled exception case is a missing + * bundled resource (a broken build), which surfaces at startup in any + * case. {@code ThemeManager}'s constructor calls this synchronously, so + * any size that could fail to resolve is resolved here rather than + * blocking subsequent launches.

+ * + *

Touches the filesystem on a cache miss, so this must only be called + * from the FX thread when the caller already knows the size is cached or + * default (e.g. {@code ThemeManager}'s constructor, which needs the sheet + * in place synchronously before the stage is shown). Any FX-thread caller + * that cannot make that guarantee -- a live slider drag, in particular -- + * must go through {@link #stylesheetForAsync} instead.

+ */ + static synchronized String stylesheetFor(double fontSize) { + double clamped = Math.clamp(fontSize, MIN_FONT_SIZE, MAX_FONT_SIZE); + int key = keyFor(clamped); + if (key == keyFor(WorkspaceUiState.DEFAULT_UI_FONT_SIZE)) { + return baseStylesheetUrl(); + } + return GENERATED.computeIfAbsent(key, k -> generateOrBase(k / 2.0)); + } + + /** + * The fallback is cached alongside real results deliberately: it keeps + * every size resolvable from the cache once it has been asked for, so + * neither {@link #cachedStylesheetFor} nor an FX-thread re-application of + * an already-applied size can be surprised into retrying failed I/O + * inline. A failure that was transient therefore sticks for the rest of + * the process -- an acceptable trade for never blocking or throwing on + * the FX thread. + */ + private static String generateOrBase(double fontSize) { + try { + return generate(fontSize); + } catch (RuntimeException e) { + LOG.log(Level.WARNING, "Could not generate the scaled stylesheet for size " + fontSize + + "; falling back to the unscaled stylesheet", e); + return baseStylesheetUrl(); + } + } + + /** Already-cached or default stylesheet for {@code fontSize}, with no filesystem access at all. */ + private static synchronized Optional cachedStylesheetFor(double fontSize) { + double clamped = Math.clamp(fontSize, MIN_FONT_SIZE, MAX_FONT_SIZE); + int key = keyFor(clamped); + if (key == keyFor(WorkspaceUiState.DEFAULT_UI_FONT_SIZE)) { + return Optional.of(baseStylesheetUrl()); + } + return Optional.ofNullable(GENERATED.get(key)); + } + + private static int keyFor(double fontSize) { + return (int) Math.round(fontSize * 2); // 0.5px resolution, matching the slider + } + + /** + * As {@link #stylesheetFor}, but safe to call from the FX thread + * unconditionally: a cache hit (or the default size) resolves {@code + * onReady} synchronously with no I/O, and a cache miss generates on a + * virtual thread and hands the result back via {@link Platform#runLater}. + * {@link #stylesheetFor} degrades a generation failure to the unscaled + * sheet rather than throwing, so a failure shows up as an unscaled + * interface, never as an error dialog over a purely cosmetic change. The + * unhandled case is a missing bundled resource (a broken build), which + * fails at startup in any case, so {@code onReady} receives a valid + * stylesheet in all normal scenarios. + */ + static void stylesheetForAsync(double fontSize, Consumer onReady) { + Optional cached = cachedStylesheetFor(fontSize); + if (cached.isPresent()) { + onReady.accept(cached.get()); + return; + } + Thread.ofVirtual().start(() -> { + String url = stylesheetFor(fontSize); + Platform.runLater(() -> onReady.accept(url)); + }); + } + + private static String generate(double fontSize) { + try (InputStream stream = UiFontScale.class.getResourceAsStream(BASE_RESOURCE)) { + if (stream == null) { + throw new IllegalStateException("Missing bundled stylesheet: " + BASE_RESOURCE); + } + String css = new String(stream.readAllBytes(), StandardCharsets.UTF_8); + String scaled = scaleCss(css, fontSize / WorkspaceUiState.DEFAULT_UI_FONT_SIZE); + Path dir = Files.createTempDirectory("drydock-ui-scale"); + dir.toFile().deleteOnExit(); + Path file = dir.resolve("app.css"); + Files.writeString(file, scaled, StandardCharsets.UTF_8); + file.toFile().deleteOnExit(); + return file.toUri().toURL().toExternalForm(); + } catch (IOException e) { + throw new UncheckedIOException("Could not generate the scaled stylesheet", e); + } + } + + /** + * Multiplies every {@code -fx-font-size} px literal by {@code factor}, + * rounded to two decimals, leaving the rest of the text byte-identical. + */ + static String scaleCss(String css, double factor) { + Matcher matcher = FONT_SIZE.matcher(css); + StringBuilder out = new StringBuilder(css.length()); + while (matcher.find()) { + if (matcher.group(1) == null) { + // A comment matched; pass it through untouched. + matcher.appendReplacement(out, Matcher.quoteReplacement(matcher.group())); + continue; + } + double scaled = Double.parseDouble(matcher.group(2)) * factor; + double rounded = Math.round(scaled * 100.0) / 100.0; + matcher.appendReplacement(out, Matcher.quoteReplacement(matcher.group(1) + rounded + "px")); + } + matcher.appendTail(out); + return out.toString(); + } +} diff --git a/app/src/main/resources/app/drydock/ui/app.css b/app/src/main/resources/app/drydock/ui/app.css index 316cde0..c01117d 100644 --- a/app/src/main/resources/app/drydock/ui/app.css +++ b/app/src/main/resources/app/drydock/ui/app.css @@ -2035,3 +2035,57 @@ -fx-font-size: 9.5px; -fx-padding: 0 6 0 6; } + +/* --- Settings modal ------------------------------------------------- */ +.settings-section-title { + -fx-font-size: 11px; + -fx-text-fill: -drydock-text-faint; + -fx-padding: 6px 0 0 0; +} + +.settings-row-label { + -fx-font-size: 12.5px; + -fx-text-fill: -drydock-text; +} + +.settings-value { + -fx-font-size: 12px; + -fx-text-fill: -drydock-text-faint; +} + +.settings-hint { + -fx-font-size: 11px; + -fx-text-fill: -drydock-text-faint; + -fx-padding: 0 0 0 132px; +} + +/* Stock RadioButton/Slider render against modena's light-default look, + * which app.css never overrides globally (unlike CheckBox -- see + * .result-check above, the established precedent this follows): the radio + * caption is near-black on the dark theme and the slider track is light + * gray in both themes. Token-driven overrides for the two controls this + * modal is the only user of. */ +.settings-radio { + -fx-text-fill: -drydock-text; +} +.settings-radio .radio { + -fx-background-color: -drydock-input-bg; + -fx-border-color: -drydock-border-strong; + -fx-border-radius: 1em; + -fx-background-radius: 1em; +} +.settings-radio:selected .radio { + -fx-border-color: -drydock-accent; +} +.settings-radio:selected .dot { + -fx-background-color: -drydock-accent; +} +.settings-slider .track { + -fx-background-color: -drydock-border-strong; + -fx-background-insets: 0; + -fx-pref-height: 4px; +} +.settings-slider .thumb { + -fx-background-color: -drydock-accent; + -fx-background-radius: 1em; +} diff --git a/app/src/test/java/app/drydock/app/RepositoryManagerTest.java b/app/src/test/java/app/drydock/app/RepositoryManagerTest.java index 3c0d4dd..9a20ef7 100644 --- a/app/src/test/java/app/drydock/app/RepositoryManagerTest.java +++ b/app/src/test/java/app/drydock/app/RepositoryManagerTest.java @@ -4,6 +4,7 @@ import app.drydock.domain.Repository; import app.drydock.domain.RepositoryId; import app.drydock.domain.SshRemote; +import app.drydock.domain.UiTheme; import app.drydock.git.GitExecutableLocator; import app.drydock.git.GitStatusService; import app.drydock.git.NotAGitRepositoryException; @@ -164,6 +165,26 @@ void removeRepositoryIsANoOpForAnUnknownId() throws Exception { assertEquals(savesBefore, stateRepository.saveCount(), "an unknown id must not trigger a redundant save"); } + @Test + void updateUiFontSizePersistsWithoutDisturbingTheTheme() { + manager.updateTheme(UiTheme.LIGHT); + + manager.updateUiFontSize(15.0); + + assertEquals(15.0, manager.state().ui().uiFontSize()); + assertEquals(UiTheme.LIGHT, manager.state().ui().theme()); + } + + @Test + void updateTerminalFontSizePersistsWithoutDisturbingTheInterfaceSize() { + manager.updateUiFontSize(15.0); + + manager.updateTerminalFontSize(11.0); + + assertEquals(11.0, manager.state().ui().terminalFontSize()); + assertEquals(15.0, manager.state().ui().uiFontSize()); + } + private static final class InMemoryStateRepository implements ApplicationStateRepository { // volatile: saves arrive on the state store's background writer thread. private volatile ApplicationState state = ApplicationState.empty(); diff --git a/app/src/test/java/app/drydock/config/UserConfigTest.java b/app/src/test/java/app/drydock/config/UserConfigTest.java index ce72e40..02be0fa 100644 --- a/app/src/test/java/app/drydock/config/UserConfigTest.java +++ b/app/src/test/java/app/drydock/config/UserConfigTest.java @@ -1,13 +1,23 @@ package app.drydock.config; +import app.drydock.state.json.JsonParser; +import app.drydock.state.json.JsonValue; +import app.drydock.state.json.JsonValue.JsonNumber; +import app.drydock.state.json.JsonValue.JsonObject; +import app.drydock.state.json.JsonValue.JsonString; + import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.api.parallel.ResourceLock; +import org.junit.jupiter.api.parallel.Resources; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertTrue; class UserConfigTest { @@ -60,9 +70,153 @@ void loadIgnoresANonStringWorktreesDirectory(@TempDir Path dir) throws Exception } @Test + // No parallelism is configured today (see the note above the saveAsync + // tests below), but this reads the developer's real ~/.drydock/config.json + // via "user.home" -- the same global the saveAsync tests below repoint -- + // so it stays behind the same lock rather than depending on that staying + // true. + @ResourceLock(Resources.SYSTEM_PROPERTIES) void loadAsyncReturnsTheSameResultAsLoadOffTheCallingThread() throws Exception { UserConfig config = UserConfig.loadAsync().get(); assertEquals(UserConfig.load(), config); } + + @Test + void savedConfigLoadsBackUnchanged(@TempDir Path tempDir) throws Exception { + Path configFile = tempDir.resolve("config.json"); + + UserConfig.save(new UserConfig(Optional.of(Path.of("/tmp/worktrees"))), configFile); + + assertEquals(Optional.of(Path.of("/tmp/worktrees")), UserConfig.load(configFile).worktreesDirectory()); + } + + @Test + void saveCreatesTheParentDirectory(@TempDir Path tempDir) throws Exception { + Path configFile = tempDir.resolve("nested").resolve(".drydock").resolve("config.json"); + + UserConfig.save(new UserConfig(Optional.of(Path.of("/tmp/worktrees"))), configFile); + + assertTrue(Files.exists(configFile)); + } + + @Test + void saveOverwritesAMalformedFileAndLeavesNoTempBehind(@TempDir Path tempDir) throws Exception { + Path configFile = tempDir.resolve("config.json"); + Files.writeString(configFile, "{ this is not json"); + + UserConfig.save(new UserConfig(Optional.of(Path.of("/tmp/worktrees"))), configFile); + + assertEquals(Optional.of(Path.of("/tmp/worktrees")), UserConfig.load(configFile).worktreesDirectory()); + try (var entries = Files.list(tempDir)) { + assertEquals(List.of("config.json"), + entries.map(p -> p.getFileName().toString()).sorted().toList()); + } + } + + @Test + void savePreservesMembersItDoesNotKnowAbout(@TempDir Path tempDir) throws Exception { + // The file is human-editable and may grow keys this build predates; + // rewriting it must not silently delete the user's other settings. + // A plain substring check would still pass if the value were + // mangled or the key duplicated, so parse the result and check the + // actual structure instead. + Path configFile = tempDir.resolve("config.json"); + Files.writeString(configFile, "{\"worktreesDirectory\":\"/old\",\"somethingElse\":42}"); + + UserConfig.save(new UserConfig(Optional.of(Path.of("/tmp/worktrees"))), configFile); + + String written = Files.readString(configFile); + JsonValue parsed = JsonParser.parse(written); + JsonObject root = assertInstanceOf(JsonObject.class, parsed, written); + assertEquals(new JsonNumber("42"), root.get("somethingElse"), written); + assertEquals(new JsonString(Path.of("/tmp/worktrees").toString()), root.get("worktreesDirectory"), written); + assertEquals(1, written.split("worktreesDirectory", -1).length - 1, + "worktreesDirectory must appear exactly once: " + written); + } + + @Test + void savingAnEmptyConfigClearsTheDirectory(@TempDir Path tempDir) throws Exception { + Path configFile = tempDir.resolve("config.json"); + UserConfig.save(new UserConfig(Optional.of(Path.of("/tmp/worktrees"))), configFile); + + UserConfig.save(UserConfig.empty(), configFile); + + assertEquals(Optional.empty(), UserConfig.load(configFile).worktreesDirectory()); + } + + // ---- saveAsync / flushPendingSaves ---- + // + // saveAsync always writes to UserConfig.defaultConfigFile(), which is + // derived from the "user.home" system property, so these tests point + // that property at a @TempDir for their duration. PENDING_SAVE and + // SAVE_EXECUTOR are static -- shared by the whole test JVM -- so every + // test below flushes in a finally block before restoring the property + // and letting its @TempDir be deleted; otherwise a write queued by one + // test could still be in flight (or land after) when the next test's + // directory no longer exists. + + @Test + @ResourceLock(Resources.SYSTEM_PROPERTIES) + void racingSaveAsyncCallsLeaveTheLastValueOnDiskAfterFlush(@TempDir Path tempDir) throws Exception { + String originalUserHome = System.getProperty("user.home"); + System.setProperty("user.home", tempDir.toString()); + try { + UserConfig.saveAsync(new UserConfig(Optional.of(Path.of("/tmp/worktrees-A")))); + UserConfig.saveAsync(new UserConfig(Optional.of(Path.of("/tmp/worktrees-B")))); + UserConfig.flushPendingSaves(); + + assertEquals(Optional.of(Path.of("/tmp/worktrees-B")), + UserConfig.load(UserConfig.defaultConfigFile()).worktreesDirectory()); + } finally { + UserConfig.flushPendingSaves(); + System.setProperty("user.home", originalUserHome); + } + } + + @Test + @ResourceLock(Resources.SYSTEM_PROPERTIES) + void loadAsyncObservesAPreviouslyQueuedSave(@TempDir Path tempDir) throws Exception { + // loadAsync must be ordered against SAVE_EXECUTOR, not race it on an + // independent thread: commit an edit, then immediately reload (as + // closing and reopening the settings modal does) and confirm the + // load never observes the pre-save value. + String originalUserHome = System.getProperty("user.home"); + System.setProperty("user.home", tempDir.toString()); + try { + UserConfig.saveAsync(new UserConfig(Optional.of(Path.of("/tmp/worktrees-fresh")))); + + UserConfig loaded = UserConfig.loadAsync().get(); + + assertEquals(Optional.of(Path.of("/tmp/worktrees-fresh")), loaded.worktreesDirectory()); + } finally { + UserConfig.flushPendingSaves(); + System.setProperty("user.home", originalUserHome); + } + } + + @Test + @ResourceLock(Resources.SYSTEM_PROPERTIES) + void flushPendingSavesWaitsForEveryQueuedSaveBeforeReturning(@TempDir Path tempDir) throws Exception { + String originalUserHome = System.getProperty("user.home"); + System.setProperty("user.home", tempDir.toString()); + try { + int calls = 50; + for (int i = 0; i < calls; i++) { + UserConfig.saveAsync(new UserConfig(Optional.of(Path.of("/tmp/worktrees-" + i)))); + } + UserConfig.flushPendingSaves(); + + // If flushPendingSaves returned before every queued save had + // actually finished writing, a straggler could still overwrite + // the file after this point with something other than the + // last-issued value -- so the file's content read right here, + // with no waiting or retrying, must already be final. + assertEquals(Optional.of(Path.of("/tmp/worktrees-" + (calls - 1))), + UserConfig.load(UserConfig.defaultConfigFile()).worktreesDirectory()); + } finally { + UserConfig.flushPendingSaves(); + System.setProperty("user.home", originalUserHome); + } + } } diff --git a/app/src/test/java/app/drydock/domain/WorkspaceUiStateTest.java b/app/src/test/java/app/drydock/domain/WorkspaceUiStateTest.java new file mode 100644 index 0000000..1c22a14 --- /dev/null +++ b/app/src/test/java/app/drydock/domain/WorkspaceUiStateTest.java @@ -0,0 +1,43 @@ +package app.drydock.domain; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * The font-size fields default to the design's 13px base and copy + * independently of every other field. + */ +class WorkspaceUiStateTest { + + @Test + void emptyUsesTheThirteenPixelDesignBase() { + WorkspaceUiState empty = WorkspaceUiState.empty(); + + assertEquals(13.0, empty.uiFontSize()); + assertEquals(13.0, empty.terminalFontSize()); + } + + @Test + void withUiFontSizeLeavesEveryOtherFieldAlone() { + WorkspaceUiState updated = WorkspaceUiState.empty() + .withSidebarWidth(300) + .withTheme(UiTheme.LIGHT) + .withUiFontSize(15.5); + + assertEquals(15.5, updated.uiFontSize()); + assertEquals(300, updated.sidebarWidth()); + assertEquals(UiTheme.LIGHT, updated.theme()); + assertEquals(13.0, updated.terminalFontSize()); + } + + @Test + void withTerminalFontSizeLeavesTheInterfaceSizeAlone() { + WorkspaceUiState updated = WorkspaceUiState.empty() + .withUiFontSize(16) + .withTerminalFontSize(11); + + assertEquals(16, updated.uiFontSize()); + assertEquals(11, updated.terminalFontSize()); + } +} diff --git a/app/src/test/java/app/drydock/state/ApplicationStateCodecTest.java b/app/src/test/java/app/drydock/state/ApplicationStateCodecTest.java index b8e15ee..2b94a87 100644 --- a/app/src/test/java/app/drydock/state/ApplicationStateCodecTest.java +++ b/app/src/test/java/app/drydock/state/ApplicationStateCodecTest.java @@ -13,6 +13,7 @@ import app.drydock.domain.WorkspaceUiState; import app.drydock.state.json.JsonParser; import app.drydock.state.json.JsonValue; +import app.drydock.state.json.JsonWriter; import app.drydock.state.json.JsonValue.JsonArray; import app.drydock.state.json.JsonValue.JsonObject; import app.drydock.state.json.JsonValue.JsonString; @@ -351,4 +352,66 @@ void sessionWithInvalidStatusThrowsStateDecodeException() { assertThrows(StateDecodeException.class, () -> ApplicationStateCodec.fromJson(JsonParser.parse(sessionDocument(session)))); } + + @Test + void fontSizesRoundTrip() { + ApplicationState state = new ApplicationState(List.of(), List.of(), + WorkspaceUiState.empty().withUiFontSize(15).withTerminalFontSize(11)); + + ApplicationState decoded = ApplicationStateCodec.fromJson( + JsonParser.parse(JsonWriter.write(ApplicationStateCodec.toJson(state)))); + + assertEquals(15, decoded.ui().uiFontSize()); + assertEquals(11, decoded.ui().terminalFontSize()); + } + + @Test + void absentFontSizesDecodeToTheDefaults() { + // A state file written before this feature existed: the ui object + // has no font-size members at all. + JsonValue json = JsonParser.parse(""" + {"schemaVersion":%d,"repositories":[],"sessions":[], + "ui":{"selectedRepositoryId":null,"sidebarWidth":288.0, + "expandedRepositoryIds":[],"theme":"DARK"}} + """.formatted(ApplicationStateCodec.SCHEMA_VERSION)); + + WorkspaceUiState ui = ApplicationStateCodec.fromJson(json).ui(); + + assertEquals(13.0, ui.uiFontSize()); + assertEquals(13.0, ui.terminalFontSize()); + } + + @Test + void malformedFontSizesDecodeToTheDefaultsWithoutFailingTheWholeState() { + JsonValue json = JsonParser.parse(""" + {"schemaVersion":%d,"repositories":[],"sessions":[], + "ui":{"selectedRepositoryId":null,"sidebarWidth":288.0, + "expandedRepositoryIds":[],"theme":"DARK", + "uiFontSize":"enormous","terminalFontSize":null}} + """.formatted(ApplicationStateCodec.SCHEMA_VERSION)); + + WorkspaceUiState ui = ApplicationStateCodec.fromJson(json).ui(); + + assertEquals(13.0, ui.uiFontSize()); + assertEquals(13.0, ui.terminalFontSize()); + } + + @Test + void outOfRangeFontSizeSurvivesTheDecodeUnchanged() { + // The codec does not clamp -- range ownership belongs to the point + // of application (ThemeManager / TerminalThemes), exactly like + // sidebarWidth, which the SplitPane clamps at use. A hand-edited + // value is honoured as far as it can be, never silently rewritten. + JsonValue json = JsonParser.parse(""" + {"schemaVersion":%d,"repositories":[],"sessions":[], + "ui":{"selectedRepositoryId":null,"sidebarWidth":288.0, + "expandedRepositoryIds":[],"theme":"DARK", + "uiFontSize":900.0,"terminalFontSize":0.0}} + """.formatted(ApplicationStateCodec.SCHEMA_VERSION)); + + WorkspaceUiState ui = ApplicationStateCodec.fromJson(json).ui(); + + assertEquals(900.0, ui.uiFontSize()); + assertEquals(0.0, ui.terminalFontSize()); + } } diff --git a/app/src/test/java/app/drydock/state/JsonApplicationStateRepositoryTest.java b/app/src/test/java/app/drydock/state/JsonApplicationStateRepositoryTest.java index c1f7598..77d38c2 100644 --- a/app/src/test/java/app/drydock/state/JsonApplicationStateRepositoryTest.java +++ b/app/src/test/java/app/drydock/state/JsonApplicationStateRepositoryTest.java @@ -59,7 +59,8 @@ void saveThenLoadRoundTripsRepositoriesAndUiState() throws IOException { Path repoRoot = Files.createDirectory(tempDir.resolve("repo-root")); Repository repo = sampleRepository(repoRoot); WorkspaceUiState ui = new WorkspaceUiState( - Optional.of(repo.id()), 321.0, Set.of(repo.id()), app.drydock.domain.UiTheme.LIGHT); + Optional.of(repo.id()), 321.0, Set.of(repo.id()), app.drydock.domain.UiTheme.LIGHT, + WorkspaceUiState.DEFAULT_UI_FONT_SIZE, WorkspaceUiState.DEFAULT_TERMINAL_FONT_SIZE); ApplicationState state = new ApplicationState(List.of(repo), List.of(), ui); JsonApplicationStateRepository repository = new JsonApplicationStateRepository(stateFile()); diff --git a/app/src/test/java/app/drydock/ui/SettingsModalTest.java b/app/src/test/java/app/drydock/ui/SettingsModalTest.java new file mode 100644 index 0000000..0a0062d --- /dev/null +++ b/app/src/test/java/app/drydock/ui/SettingsModalTest.java @@ -0,0 +1,19 @@ +package app.drydock.ui; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** The slider's readout: whole sizes lose the decimal, halves keep it. */ +class SettingsModalTest { + + @Test + void formatsWholeSizesWithoutADecimal() { + assertEquals("13 px", SettingsModal.format(13.0)); + } + + @Test + void formatsHalfSizesWithOneDecimal() { + assertEquals("13.5 px", SettingsModal.format(13.5)); + } +} diff --git a/app/src/test/java/app/drydock/ui/SizeSettingTest.java b/app/src/test/java/app/drydock/ui/SizeSettingTest.java new file mode 100644 index 0000000..5e4fe26 --- /dev/null +++ b/app/src/test/java/app/drydock/ui/SizeSettingTest.java @@ -0,0 +1,125 @@ +package app.drydock.ui; + +import org.junit.jupiter.api.Test; + +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Deque; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Exercises the apply/persist sequencing that the settings sliders drive. No + * JavaFX toolkit is involved: {@link SizeSetting} exists precisely so this + * logic can be driven directly (the convention in this module -- see {@code + * RemoteRepositoryModalTest} -- is to test pure helpers only). + */ +class SizeSettingTest { + + /** + * Stands in for {@code ThemeManager}: applying a size not yet cached + * completes only when the queued FX callback runs, so the applied size + * lags the request by one turn -- the situation every assertion below + * cares about. + */ + private static final class DeferredApply { + private final Deque queued = new ArrayDeque<>(); + private double appliedSize; + + DeferredApply(double initial) { + this.appliedSize = initial; + } + + void request(double size) { + queued.add(() -> appliedSize = size); + } + + void runQueued() { + while (!queued.isEmpty()) { + queued.remove().run(); + } + } + } + + /** + * The regression this type was extracted for: an arrow key produces a + * single tick with no drag in progress, so the persist runs in the same + * turn as the apply, while the applied size is still the previous one. + * Persisting the applied size here stored one step behind, invisibly + * until the next launch. + */ + @Test + void commitInTheSameTurnAsTheApplyStillPersistsTheChosenSize() { + DeferredApply applied = new DeferredApply(14.0); + List persisted = new ArrayList<>(); + SizeSetting setting = new SizeSetting(() -> applied.appliedSize, applied::request, persisted::add); + + setting.changed(15.0, false); + + assertEquals(List.of(15.0), persisted); + assertEquals(14.0, applied.appliedSize, "the apply must still be pending, or the test proves nothing"); + applied.runQueued(); + assertEquals(15.0, applied.appliedSize); + } + + /** Repeated keyboard steps each persist their own size, never the one before. */ + @Test + void everyDiscreteStepPersistsItsOwnSize() { + DeferredApply applied = new DeferredApply(14.0); + List persisted = new ArrayList<>(); + SizeSetting setting = new SizeSetting(() -> applied.appliedSize, applied::request, persisted::add); + + setting.changed(14.5, false); + setting.changed(15.0, false); + setting.changed(15.5, false); + + assertEquals(List.of(14.5, 15.0, 15.5), persisted); + } + + /** A drag applies per tick and persists exactly once, at the released value. */ + @Test + void dragAppliesEveryTickAndPersistsOnlyOnRelease() { + DeferredApply applied = new DeferredApply(14.0); + List persisted = new ArrayList<>(); + SizeSetting setting = new SizeSetting(() -> applied.appliedSize, applied::request, persisted::add); + + setting.changed(14.5, true); + setting.changed(15.0, true); + setting.changed(15.5, true); + assertTrue(persisted.isEmpty(), "no disk traffic mid-drag"); + assertEquals(3, applied.queued.size(), "every tick still applies live"); + + setting.dragEnded(15.5); + + assertEquals(List.of(15.5), persisted); + } + + /** + * A drag whose live applies have not landed yet still persists where the + * slider came to rest, not the size the UI happens to be showing. + */ + @Test + void dragEndedPersistsTheReleasedSizeNotTheAppliedOne() { + DeferredApply applied = new DeferredApply(14.0); + List persisted = new ArrayList<>(); + SizeSetting setting = new SizeSetting(() -> applied.appliedSize, applied::request, persisted::add); + + setting.changed(16.0, true); + setting.dragEnded(16.0); + + assertEquals(List.of(16.0), persisted); + assertEquals(14.0, applied.appliedSize); + } + + /** {@code current()} is the one legitimate read of applied state: the slider's opening value. */ + @Test + void currentReadsTheAppliedSize() { + DeferredApply applied = new DeferredApply(12.5); + SizeSetting setting = new SizeSetting(() -> applied.appliedSize, applied::request, size -> { + }); + + assertEquals(12.5, setting.current()); + } +} diff --git a/app/src/test/java/app/drydock/ui/TerminalThemesTest.java b/app/src/test/java/app/drydock/ui/TerminalThemesTest.java new file mode 100644 index 0000000..e8080f0 --- /dev/null +++ b/app/src/test/java/app/drydock/ui/TerminalThemesTest.java @@ -0,0 +1,72 @@ +package app.drydock.ui; + +import app.drydock.domain.UiTheme; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The generated ghostty config carries the user's terminal font size, and + * each (theme, size) pair gets its own file so switching back and forth + * never re-extracts or cross-contaminates. + */ +class TerminalThemesTest { + + @Test + void writesTheRequestedFontSizeIntoTheConfig() throws IOException { + Path config = TerminalThemes.configFileFor(UiTheme.DARK, 15.0); + + String text = Files.readString(config, StandardCharsets.UTF_8); + + assertTrue(text.contains("font-size = 15"), text); + // The bundled theme content must survive alongside it. + assertTrue(text.contains("background = #161514"), text); + } + + @Test + void clampsAnOutOfRangeSizeToTheSupportedBand() throws IOException { + Path tooLarge = TerminalThemes.configFileFor(UiTheme.DARK, 900.0); + Path tooSmall = TerminalThemes.configFileFor(UiTheme.DARK, 0.0); + + assertTrue(Files.readString(tooLarge, StandardCharsets.UTF_8).contains("font-size = 18")); + assertTrue(Files.readString(tooSmall, StandardCharsets.UTF_8).contains("font-size = 10")); + } + + @Test + void cachesPerThemeAndSize() { + Path darkThirteen = TerminalThemes.configFileFor(UiTheme.DARK, 13.0); + + assertEquals(darkThirteen, TerminalThemes.configFileFor(UiTheme.DARK, 13.0)); + assertNotEquals(darkThirteen, TerminalThemes.configFileFor(UiTheme.DARK, 14.0)); + assertNotEquals(darkThirteen, TerminalThemes.configFileFor(UiTheme.LIGHT, 13.0)); + } + + @Test + void asyncResolvesAnAlreadyCachedPairSynchronouslyOnTheCallingThread() { + // Warm the cache via the synchronous path first (as a theme toggle + // ordinarily would), then confirm the async entry point recognises + // the cache hit and never leaves the calling thread -- this is what + // lets MainWorkspace.applyTerminalTheme call it unconditionally from + // the settings modal's font-size slider. + Path expected = TerminalThemes.configFileFor(UiTheme.DARK, 12.0); + Thread callingThread = Thread.currentThread(); + AtomicReference result = new AtomicReference<>(); + AtomicReference calledFrom = new AtomicReference<>(); + + TerminalThemes.configFileForAsync(UiTheme.DARK, 12.0, file -> { + result.set(file); + calledFrom.set(Thread.currentThread()); + }); + + assertEquals(expected, result.get()); + assertEquals(callingThread, calledFrom.get()); + } +} diff --git a/app/src/test/java/app/drydock/ui/UiFontScaleTest.java b/app/src/test/java/app/drydock/ui/UiFontScaleTest.java new file mode 100644 index 0000000..48ab50b --- /dev/null +++ b/app/src/test/java/app/drydock/ui/UiFontScaleTest.java @@ -0,0 +1,145 @@ +package app.drydock.ui; + +import org.junit.jupiter.api.Test; + +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Scaling multiplies every {@code -fx-font-size} px literal by the factor + * and touches nothing else. + * + *

The nested-rule test is the important one. The rejected alternative -- + * rewriting the declarations as {@code em} -- looks correct line by line but + * compounds, because JavaFX resolves font-relative sizes against the + * inherited font rather than against {@code .root}: {@code .code-area} + * (12px) containing {@code .lineno} (11px) would land at 10.15px instead of + * 13.54px at factor 16/13. Absolute scaling keeps the ratio exact.

+ */ +class UiFontScaleTest { + + @Test + void scalesEveryFontSizeDeclaration() { + String scaled = UiFontScale.scaleCss(".root { -fx-font-size: 13px; }", 16.0 / 13.0); + + assertTrue(scaled.contains("-fx-font-size: 16.0px"), scaled); + } + + @Test + void nestedRulesKeepTheirRatioInsteadOfCompounding() { + String css = """ + .code-area { -fx-font-size: 12px; } + .code-area .lineno { -fx-font-size: 11px; } + """; + + String scaled = UiFontScale.scaleCss(css, 16.0 / 13.0); + + // 12 * 16/13 = 14.769..., 11 * 16/13 = 13.538... + assertTrue(scaled.contains("-fx-font-size: 14.77px"), scaled); + assertTrue(scaled.contains("-fx-font-size: 13.54px"), scaled); + } + + @Test + void preservesFractionalSourceSizes() { + String scaled = UiFontScale.scaleCss(".pill { -fx-font-size: 12.5px; }", 2.0); + + assertTrue(scaled.contains("-fx-font-size: 25.0px"), scaled); + } + + @Test + void leavesEverythingThatIsNotAFontSizeAlone() { + String css = """ + /* -fx-font-size: 99px in a comment must not move */ + .filter-field { + -fx-min-height: 32px; + -fx-max-height: 32px; + -fx-background-color: #161514; + -fx-font-family: "System"; + -fx-padding: 0.5em; + } + """; + + String scaled = UiFontScale.scaleCss(css, 2.0); + + assertTrue(scaled.contains("-fx-min-height: 32px"), scaled); + assertTrue(scaled.contains("-fx-max-height: 32px"), scaled); + assertTrue(scaled.contains("-fx-background-color: #161514"), scaled); + assertTrue(scaled.contains("-fx-padding: 0.5em"), scaled); + assertTrue(scaled.contains("99px in a comment"), scaled); + } + + @Test + void scalesADeclarationWithACommentBetweenTheColonAndTheValue() { + // A comment sitting between the colon and the value must not split + // the declaration into an unrecognised property and a bare number: + // the whole thing is still one declaration and the px literal must + // still be scaled, with the comment text preserved verbatim. + String scaled = UiFontScale.scaleCss( + ".pill { -fx-font-size: /* TODO bump */ 12px; }", 2.0); + + assertTrue(scaled.contains("-fx-font-size: /* TODO bump */ 24.0px"), scaled); + } + + @Test + void toleratesSpacingVariantsInTheSourceDeclaration() { + String scaled = UiFontScale.scaleCss(".a{-fx-font-size:10px}", 2.0); + + assertTrue(scaled.contains("20.0px"), scaled); + assertFalse(scaled.contains("10px"), scaled); + } + + @Test + void identityFactorReturnsTheOriginalResourceUrl() { + // The unscaled case must not write a temp file at all: it is both the + // default and the hot path at startup. + assertEquals(UiFontScale.baseStylesheetUrl(), UiFontScale.stylesheetFor(13.0)); + } + + @Test + void scaledSizeProducesAUsableStylesheetUrl() { + String url = UiFontScale.stylesheetFor(16.0); + + assertTrue(url.startsWith("file:"), url); + assertFalse(url.equals(UiFontScale.baseStylesheetUrl()), url); + } + + @Test + void asyncResolvesTheDefaultSizeSynchronouslyOnTheCallingThread() { + // The whole point of the async entry point is that an FX-thread + // caller can call it unconditionally; the default-size fast path + // must never hop to another thread to prove that. + Thread callingThread = Thread.currentThread(); + AtomicReference result = new AtomicReference<>(); + AtomicReference calledFrom = new AtomicReference<>(); + + UiFontScale.stylesheetForAsync(13.0, url -> { + result.set(url); + calledFrom.set(Thread.currentThread()); + }); + + assertEquals(UiFontScale.baseStylesheetUrl(), result.get()); + assertEquals(callingThread, calledFrom.get()); + } + + @Test + void asyncResolvesAnAlreadyCachedSizeSynchronouslyOnTheCallingThread() { + // Warm the cache via the synchronous path first (as ThemeManager's + // constructor does at startup), then confirm the async entry point + // recognises the cache hit and never leaves the calling thread. + String expected = UiFontScale.stylesheetFor(14.5); + Thread callingThread = Thread.currentThread(); + AtomicReference result = new AtomicReference<>(); + AtomicReference calledFrom = new AtomicReference<>(); + + UiFontScale.stylesheetForAsync(14.5, url -> { + result.set(url); + calledFrom.set(Thread.currentThread()); + }); + + assertEquals(expected, result.get()); + assertEquals(callingThread, calledFrom.get()); + } +} diff --git a/docs/manual-terminal-checklist.md b/docs/manual-terminal-checklist.md index ad792e9..e977e5c 100644 --- a/docs/manual-terminal-checklist.md +++ b/docs/manual-terminal-checklist.md @@ -84,3 +84,102 @@ Javadoc for a second, related finding (`ghostty_surface_text` is paste-only semantics, and gets wrapped in bracketed-paste markers once the shell enables bracketed paste -- ordinary typed characters must go through `ghostty_surface_key`/`sendCharKey` instead). + +## Settings — opening and closing the modal + +1. Look at the title bar. The gear button renders between the `?` shortcuts + button and the theme toggle — if it is missing or in the wrong slot, the + layout wiring in `TitleBar`/`AppShell` has regressed. +2. Click the gear button. Settings opens — this is the only way to reach it + without the keyboard, so it must work on its own, not just as a backup + for ⌘,. +3. Close it, then press ⌘, instead. Settings opens the same way — both + entry points must land on the same modal, not two different ones. +4. With Settings open, press ⌘, again. Nothing stacks and nothing flickers; + a second modal on top of the first would mean the ⌘, handler stopped + checking `modalLayer().isShowingModal()` before opening. +5. Close Settings, open New worktree, then press ⌘,. New worktree is left + untouched and Settings does not appear over it — ⌘, must stay inert + while any other modal owns the screen, or it could silently replace a + form the user is mid-way through filling in. +6. Open Settings again and press Esc. It closes — Esc must reach the modal + layer's generic close handling, not just the terminal. +7. Reopen it and click the × in the header. It closes. +8. Reopen it and click **Done**. It closes. All three (Esc, ×, Done) are + equivalent exits — there is no OK/Cancel distinction to get wrong. +9. Press `?` to open the shortcuts overlay and find the **Settings** row — + it must read `⌘,`, confirming the shortcut is documented where a user + would actually look for it. + +## Settings — unchanged at the default size + +1. Quit and relaunch with no settings changed (interface size at its 13.0 + default). Compare the app's overall look to before this feature existed + — every row, button, and font should look exactly the same. The interface + slider's default position deliberately returns the bundled, unscaled + stylesheet with no generated copy in play, so this is the one setting + combination most likely to silently break something structural if it + didn't. + +## Settings — legibility in both themes + +1. Open Settings in Dark. Read the **Dark**/**Light** radio captions and + drag both sliders. The captions must be clearly readable text (not + near-black on the dark background) and the slider tracks/thumbs must be + visibly distinct from the modal's backdrop — stock JavaFX controls + default to a light-mode look that app.css does not otherwise override, + so this is the one place in the app that regresses if that gap is ever + reopened. +2. Toggle to Light (closing Settings first, since the theme cannot change + while it's open — see the next item) and reopen Settings. Repeat the + same check: captions and slider tracks must be legible against the + light backdrop too. +3. With Settings open in either theme, press ⌘⇧L. Nothing happens — the + theme is locked while the modal is showing, specifically so the radio + (which reads the theme once, at construction) can never drift from + reality with no way to click it back. + +## Settings — worktrees directory round trip + +1. Open Settings. The **Directory** field starts disabled showing + "Loading…", then resolves to either empty (no directory configured yet) + or the previously saved path — never left stuck at "Loading…", which + would mean `UserConfig.loadAsync`'s future never completed. +2. Click **Browse…** and pick a directory. The field fills in with the + chosen absolute path and both the field and the button briefly disable + while the save is in flight, then re-enable. +3. Close Settings, then open **New worktree** for any repository. The + proposed worktree path is nested under the directory chosen in step 2, + not the old `~/dev/wt` default — proving `WorktreeNaming` actually reads + the saved value back, not just that Settings wrote something. +4. Quit the app (or just inspect the file) and check + `~/.drydock/config.json`. It contains a `worktreesDirectory` key with + the chosen path — this setting has no automated coverage at all, so this + manual step is the only check that the full write-then-read path works + end to end. + +## Settings — terminal font size + +1. Open a Claude session so a ghostty surface is live. +2. Open Settings (⌘,) and drag **Terminal size**. It snaps to whole pixels + only (no ".5" readout) — `TerminalThemes` rounds to an int internally, so + a half-pixel readout would show a number the terminal cannot actually + render. Drag it to 18. +3. The running terminal's text grows immediately — not only new sessions. +4. Toggle the theme (⌘⇧L). The terminal re-themes and **keeps** size 18. + (A regression here means the size went back to a per-surface override, + which `ghostty_surface_update_config` discards.) +5. Quit and relaunch. The terminal opens at 18. + +## Settings — interface font size + +1. Open Settings and sweep **Interface size** across 11 → 16. +2. At both extremes check for clipping in: the title bar and traffic + lights, the sidebar filter field, the icon buttons, a combo-box popup + (New worktree ▸ Fork from), a right-click context menu, and a tooltip. + The popups and menus must scale with everything else — they are separate + scene graphs, and an implementation that only styled the main scene would + leave them at 13px. +3. Fixed-height controls (filter field 32px, icon buttons 30px, title bar + 44px) do not grow; confirm their text still fits at 16. +4. Quit and relaunch. The size is restored with no visible re-layout flash. diff --git a/docs/superpowers/plans/2026-07-25-settings-ui.md b/docs/superpowers/plans/2026-07-25-settings-ui.md new file mode 100644 index 0000000..acd1375 --- /dev/null +++ b/docs/superpowers/plans/2026-07-25-settings-ui.md @@ -0,0 +1,1786 @@ +# Settings UI Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a Settings modal — theme, interface font size, terminal font size, worktrees directory — reachable from a new gear button in the title bar and from `⌘,`. + +**Architecture:** Cosmetic settings (theme, both font sizes) live in `WorkspaceUiState` and are written through `RepositoryManager`, the single holder of the state store. The worktrees directory lives in the human-editable `~/.drydock/config.json` behind `UserConfig`, which gains atomic write support. Interface font size works by generating a scaled copy of `app.css` at runtime (`UiFontScale`) rather than rewriting the sheet; terminal font size works by injecting `font-size = N` into the generated ghostty `.conf`, which the existing theme-reapply path already pushes to running surfaces. + +**Tech Stack:** Java 26, JavaFX 26, Gradle (Kotlin DSL), JUnit 5, libghostty via FFM. + +**Spec:** `docs/superpowers/specs/2026-07-25-settings-ui-design.md` + +## Global Constraints + +- Never run blocking work (file I/O, process spawns) on the JavaFX Application Thread; use a virtual thread or an owning executor and hop back with `Platform.runLater`. Every user-triggered async operation shows progress immediately, and **every** completion path — success, error, early return — clears it. +- Persistent state has one writer: managers submit transform functions to `ApplicationStateStore` via `RepositoryManager`. Never load-then-save. +- Cosmetic UI fields decode leniently: a malformed value falls back to a default and never fails the whole state decode. +- A service that writes files from a background thread exposes a flush, called from `DrydockApplication.stop()`. +- Range ownership is at the point of application, not in the codec: `uiFontSize` 11.0–16.0 clamped by `ThemeManager`, `terminalFontSize` 10.0–18.0 clamped by `TerminalThemes`. +- Defaults: `uiFontSize` 13.0, `terminalFontSize` 13.0. `app.css`'s `.root` base font size is **13px** — the divisor for every scale factor. +- Build/test: `./gradlew :app:test` runs the suite; `./gradlew :app:run` launches the app. +- Java style in this repo: package-private UI classes unless a wider scope is needed; Javadoc on every new class and non-obvious method explaining *why*, not *what*. + +--- + +### Task 1: Verify libghostty honours `font-size` in a config file + +The whole terminal-font-size design rests on this and it cannot be confirmed from this checkout — `third_party/ghostty` is an unpopulated submodule. Do this before writing any terminal code. + +**Files:** +- Modify (temporarily, reverted in Step 4): `app/src/main/resources/app/drydock/ui/terminal-dark.conf` + +- [ ] **Step 1: Add a deliberately huge font size to the dark terminal config** + +Append this line to `app/src/main/resources/app/drydock/ui/terminal-dark.conf`: + +``` +font-size = 24 +``` + +- [ ] **Step 2: Run the app and open a session** + +Run: `./gradlew :app:run` + +Add a repository if none is present, then start a Claude session or open a terminal tab so a ghostty surface appears. + +- [ ] **Step 3: Record the outcome** + +Expected: terminal text is obviously much larger than the surrounding UI. + +- If it IS larger → the assumption holds. Continue to Step 4 and then Task 2. +- If it is NOT larger → **stop**. Terminal font size is cut from this round per the spec's "Deliberately out of scope" rule. Report this, remove the terminal-size slider from Task 8's modal, skip Tasks 5 and 6, and drop `terminalFontSize` from Tasks 2 and 3. Do not fall back to the per-surface `font_size` struct write — the spec documents why that route is a trap. + +- [ ] **Step 4: Revert the probe** + +```bash +git checkout app/src/main/resources/app/drydock/ui/terminal-dark.conf +git status # expect: clean +``` + +No commit — this task produces knowledge, not code. + +--- + +### Task 2: Add the two font-size fields to `WorkspaceUiState` + +**Files:** +- Modify: `app/src/main/java/app/drydock/domain/WorkspaceUiState.java` + +**Interfaces:** +- Produces: `WorkspaceUiState(Optional selectedRepositoryId, double sidebarWidth, Set expandedRepositoryIds, UiTheme theme, double uiFontSize, double terminalFontSize)`; constants `DEFAULT_UI_FONT_SIZE = 13.0`, `DEFAULT_TERMINAL_FONT_SIZE = 13.0`; methods `withUiFontSize(double)`, `withTerminalFontSize(double)`. + +- [ ] **Step 1: Write the failing test** + +Create `app/src/test/java/app/drydock/domain/WorkspaceUiStateTest.java`: + +```java +package app.drydock.domain; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * The font-size fields default to the design's 13px base and copy + * independently of every other field. + */ +class WorkspaceUiStateTest { + + @Test + void emptyUsesTheThirteenPixelDesignBase() { + WorkspaceUiState empty = WorkspaceUiState.empty(); + + assertEquals(13.0, empty.uiFontSize()); + assertEquals(13.0, empty.terminalFontSize()); + } + + @Test + void withUiFontSizeLeavesEveryOtherFieldAlone() { + WorkspaceUiState updated = WorkspaceUiState.empty() + .withSidebarWidth(300) + .withTheme(UiTheme.LIGHT) + .withUiFontSize(15.5); + + assertEquals(15.5, updated.uiFontSize()); + assertEquals(300, updated.sidebarWidth()); + assertEquals(UiTheme.LIGHT, updated.theme()); + assertEquals(13.0, updated.terminalFontSize()); + } + + @Test + void withTerminalFontSizeLeavesTheInterfaceSizeAlone() { + WorkspaceUiState updated = WorkspaceUiState.empty() + .withUiFontSize(16) + .withTerminalFontSize(11); + + assertEquals(16, updated.uiFontSize()); + assertEquals(11, updated.terminalFontSize()); + } +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `./gradlew :app:test --tests 'app.drydock.domain.WorkspaceUiStateTest'` +Expected: compilation failure — `cannot find symbol: method uiFontSize()`. + +- [ ] **Step 3: Add the fields** + +In `WorkspaceUiState.java`, add the two components to the record header (after `theme`), add the constants, and add the two `with…` methods. Every existing `with…` method must be updated to pass the two new components through. The result: + +```java +public record WorkspaceUiState( + Optional selectedRepositoryId, + double sidebarWidth, + Set expandedRepositoryIds, + UiTheme theme, + double uiFontSize, + double terminalFontSize +) { + /** Design default 288px (handoff README section 2), clamped 220-520 at the SplitPane. */ + public static final double DEFAULT_SIDEBAR_WIDTH = 288.0; + + /** + * The app.css {@code .root} base size, and therefore the divisor every + * interface-scale factor is computed against (see {@code UiFontScale}). + */ + public static final double DEFAULT_UI_FONT_SIZE = 13.0; + + /** Matches the interface default so terminals start visually consistent with the UI. */ + public static final double DEFAULT_TERMINAL_FONT_SIZE = 13.0; + + public WorkspaceUiState { + Objects.requireNonNull(selectedRepositoryId, "selectedRepositoryId"); + expandedRepositoryIds = Set.copyOf(Objects.requireNonNull(expandedRepositoryIds, "expandedRepositoryIds")); + Objects.requireNonNull(theme, "theme"); + } + + public static WorkspaceUiState empty() { + return new WorkspaceUiState(Optional.empty(), DEFAULT_SIDEBAR_WIDTH, Set.of(), UiTheme.DARK, + DEFAULT_UI_FONT_SIZE, DEFAULT_TERMINAL_FONT_SIZE); + } + + public WorkspaceUiState withSelectedRepositoryId(Optional newSelectedRepositoryId) { + return new WorkspaceUiState(newSelectedRepositoryId, sidebarWidth, expandedRepositoryIds, theme, + uiFontSize, terminalFontSize); + } + + public WorkspaceUiState withSidebarWidth(double newSidebarWidth) { + return new WorkspaceUiState(selectedRepositoryId, newSidebarWidth, expandedRepositoryIds, theme, + uiFontSize, terminalFontSize); + } + + public WorkspaceUiState withExpandedRepositoryIds(Set newExpandedRepositoryIds) { + return new WorkspaceUiState(selectedRepositoryId, sidebarWidth, newExpandedRepositoryIds, theme, + uiFontSize, terminalFontSize); + } + + public WorkspaceUiState withTheme(UiTheme newTheme) { + return new WorkspaceUiState(selectedRepositoryId, sidebarWidth, expandedRepositoryIds, newTheme, + uiFontSize, terminalFontSize); + } + + public WorkspaceUiState withUiFontSize(double newUiFontSize) { + return new WorkspaceUiState(selectedRepositoryId, sidebarWidth, expandedRepositoryIds, theme, + newUiFontSize, terminalFontSize); + } + + public WorkspaceUiState withTerminalFontSize(double newTerminalFontSize) { + return new WorkspaceUiState(selectedRepositoryId, sidebarWidth, expandedRepositoryIds, theme, + uiFontSize, newTerminalFontSize); + } +} +``` + +Keep the existing class Javadoc and add a sentence noting the two new cosmetic fields. + +- [ ] **Step 4: Fix the compilation fallout** + +Run: `./gradlew :app:compileJava :app:compileTestJava` + +Any direct `new WorkspaceUiState(...)` call sites (in tests and `ApplicationStateCodec`) now need two more arguments. Pass `WorkspaceUiState.DEFAULT_UI_FONT_SIZE` / `WorkspaceUiState.DEFAULT_TERMINAL_FONT_SIZE` at every site — Task 3 replaces the codec's. + +- [ ] **Step 5: Run the tests** + +Run: `./gradlew :app:test` +Expected: PASS, including the three new tests. + +- [ ] **Step 6: Commit** + +```bash +git add app/src/main/java/app/drydock/domain/WorkspaceUiState.java \ + app/src/test/java/app/drydock/domain/WorkspaceUiStateTest.java +git add -u +git commit -m "Add interface and terminal font size to the workspace UI state" +``` + +--- + +### Task 3: Persist the font sizes through the codec + +**Files:** +- Modify: `app/src/main/java/app/drydock/state/ApplicationStateCodec.java` (`uiToJson` ~line 172, `uiFromJson` ~line 291) +- Test: `app/src/test/java/app/drydock/state/ApplicationStateCodecTest.java` + +**Interfaces:** +- Consumes: `WorkspaceUiState.uiFontSize()`, `.terminalFontSize()`, `DEFAULT_UI_FONT_SIZE`, `DEFAULT_TERMINAL_FONT_SIZE` (Task 2). +- Produces: JSON members `uiFontSize` / `terminalFontSize` inside the `ui` object. + +- [ ] **Step 1: Write the failing tests** + +Append to `ApplicationStateCodecTest`: + +```java + @Test + void fontSizesRoundTrip() { + ApplicationState state = new ApplicationState(List.of(), List.of(), + WorkspaceUiState.empty().withUiFontSize(15).withTerminalFontSize(11)); + + ApplicationState decoded = ApplicationStateCodec.fromJson( + JsonParser.parse(JsonWriter.write(ApplicationStateCodec.toJson(state)))); + + assertEquals(15, decoded.ui().uiFontSize()); + assertEquals(11, decoded.ui().terminalFontSize()); + } + + @Test + void absentFontSizesDecodeToTheDefaults() { + // A state file written before this feature existed: the ui object + // has no font-size members at all. + JsonValue json = JsonParser.parse(""" + {"schemaVersion":%d,"repositories":[],"sessions":[], + "ui":{"selectedRepositoryId":null,"sidebarWidth":288.0, + "expandedRepositoryIds":[],"theme":"DARK"}} + """.formatted(ApplicationStateCodec.SCHEMA_VERSION)); + + WorkspaceUiState ui = ApplicationStateCodec.fromJson(json).ui(); + + assertEquals(13.0, ui.uiFontSize()); + assertEquals(13.0, ui.terminalFontSize()); + } + + @Test + void malformedFontSizesDecodeToTheDefaultsWithoutFailingTheWholeState() { + JsonValue json = JsonParser.parse(""" + {"schemaVersion":%d,"repositories":[],"sessions":[], + "ui":{"selectedRepositoryId":null,"sidebarWidth":288.0, + "expandedRepositoryIds":[],"theme":"DARK", + "uiFontSize":"enormous","terminalFontSize":null}} + """.formatted(ApplicationStateCodec.SCHEMA_VERSION)); + + WorkspaceUiState ui = ApplicationStateCodec.fromJson(json).ui(); + + assertEquals(13.0, ui.uiFontSize()); + assertEquals(13.0, ui.terminalFontSize()); + } + + @Test + void outOfRangeFontSizeSurvivesTheDecodeUnchanged() { + // The codec does not clamp -- range ownership belongs to the point + // of application (ThemeManager / TerminalThemes), exactly like + // sidebarWidth, which the SplitPane clamps at use. A hand-edited + // value is honoured as far as it can be, never silently rewritten. + JsonValue json = JsonParser.parse(""" + {"schemaVersion":%d,"repositories":[],"sessions":[], + "ui":{"selectedRepositoryId":null,"sidebarWidth":288.0, + "expandedRepositoryIds":[],"theme":"DARK", + "uiFontSize":900.0,"terminalFontSize":0.0}} + """.formatted(ApplicationStateCodec.SCHEMA_VERSION)); + + WorkspaceUiState ui = ApplicationStateCodec.fromJson(json).ui(); + + assertEquals(900.0, ui.uiFontSize()); + assertEquals(0.0, ui.terminalFontSize()); + } +``` + +Add `import app.drydock.state.json.JsonWriter;` to the test's imports. If `SCHEMA_VERSION` is not accessible from the test, inline the current literal value instead of the `%d` placeholder. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `./gradlew :app:test --tests 'app.drydock.state.ApplicationStateCodecTest'` +Expected: FAIL — round-trip returns 13.0 instead of 15/11 (the fields are not encoded yet). + +- [ ] **Step 3: Encode the fields** + +In `uiToJson`, after the `theme` line: + +```java + obj.put("uiFontSize", JsonNumber.of(ui.uiFontSize())); + obj.put("terminalFontSize", JsonNumber.of(ui.terminalFontSize())); +``` + +- [ ] **Step 4: Decode the fields leniently** + +In `uiFromJson`, before the `return`: + +```java + // Cosmetic, so lenient like sidebarWidth: absent or non-numeric + // falls back to the design default. Deliberately NOT clamped here + // -- ThemeManager and TerminalThemes own their ranges at the point + // of application, so a hand-edited value is never silently rewritten. + double uiFontSize = obj.get("uiFontSize") instanceof JsonNumber n + ? n.asDouble() + : WorkspaceUiState.DEFAULT_UI_FONT_SIZE; + double terminalFontSize = obj.get("terminalFontSize") instanceof JsonNumber n + ? n.asDouble() + : WorkspaceUiState.DEFAULT_TERMINAL_FONT_SIZE; +``` + +and extend the constructor call: + +```java + return new WorkspaceUiState(selected, sidebarWidth, expanded, theme, uiFontSize, terminalFontSize); +``` + +- [ ] **Step 5: Update the schema documentation comment** + +The class Javadoc around line 69 documents the `ui` object's shape. Add the two new members to that example so the documented schema matches what is written. + +- [ ] **Step 6: Run the tests** + +Run: `./gradlew :app:test` +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add app/src/main/java/app/drydock/state/ApplicationStateCodec.java \ + app/src/test/java/app/drydock/state/ApplicationStateCodecTest.java +git commit -m "Persist the font sizes in the workspace UI state" +``` + +--- + +### Task 4: Expose the font-size writes on `RepositoryManager` + +**Files:** +- Modify: `app/src/main/java/app/drydock/app/RepositoryManager.java` (next to `updateTheme`, ~line 190) + +**Interfaces:** +- Produces: `RepositoryManager.updateUiFontSize(double)`, `RepositoryManager.updateTerminalFontSize(double)` — the only sanctioned route from the UI to the persisted font sizes. + +- [ ] **Step 1: Write the failing test** + +Append to `app/src/test/java/app/drydock/app/RepositoryManagerTest.java`, matching how the existing tests in that file construct a manager (reuse the file's existing setup/fixture helpers rather than inventing new ones): + +```java + @Test + void updateUiFontSizePersistsWithoutDisturbingTheTheme() { + manager.updateTheme(UiTheme.LIGHT); + + manager.updateUiFontSize(15.0); + + assertEquals(15.0, manager.state().ui().uiFontSize()); + assertEquals(UiTheme.LIGHT, manager.state().ui().theme()); + } + + @Test + void updateTerminalFontSizePersistsWithoutDisturbingTheInterfaceSize() { + manager.updateUiFontSize(15.0); + + manager.updateTerminalFontSize(11.0); + + assertEquals(11.0, manager.state().ui().terminalFontSize()); + assertEquals(15.0, manager.state().ui().uiFontSize()); + } +``` + +If the existing tests name the manager field differently, use that name. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `./gradlew :app:test --tests 'app.drydock.app.RepositoryManagerTest'` +Expected: compilation failure — `cannot find symbol: method updateUiFontSize(double)`. + +- [ ] **Step 3: Add the two writers** + +After `updateTheme`: + +```java + /** + * Persists the interface font size immediately, like the theme: a + * discrete user action in the settings modal, cheap to save, and losing + * it to a crash would be a visible regression on the next launch. + */ + public void updateUiFontSize(double uiFontSize) { + stateStore.update(state -> state.withUi(state.ui().withUiFontSize(uiFontSize))); + } + + /** Persists the terminal font size immediately, for the same reason as {@link #updateUiFontSize}. */ + public void updateTerminalFontSize(double terminalFontSize) { + stateStore.update(state -> state.withUi(state.ui().withTerminalFontSize(terminalFontSize))); + } +``` + +Both are transforms against the single writer, so a concurrent sidebar-width write at shutdown cannot clobber them. + +- [ ] **Step 4: Run the tests** + +Run: `./gradlew :app:test` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add app/src/main/java/app/drydock/app/RepositoryManager.java \ + app/src/test/java/app/drydock/app/RepositoryManagerTest.java +git commit -m "Route font-size writes through the single state writer" +``` + +--- + +### Task 5: Teach `TerminalThemes` about font size + +**Files:** +- Modify: `app/src/main/java/app/drydock/ui/TerminalThemes.java` +- Test: `app/src/test/java/app/drydock/ui/TerminalThemesTest.java` (create) + +**Interfaces:** +- Produces: `static Path configFileFor(UiTheme theme, double fontSize)` — replaces the single-argument overload; caches per `(theme, size)`; clamps `fontSize` to 10.0–18.0. + +- [ ] **Step 1: Write the failing test** + +Create `app/src/test/java/app/drydock/ui/TerminalThemesTest.java`: + +```java +package app.drydock.ui; + +import app.drydock.domain.UiTheme; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The generated ghostty config carries the user's terminal font size, and + * each (theme, size) pair gets its own file so switching back and forth + * never re-extracts or cross-contaminates. + */ +class TerminalThemesTest { + + @Test + void writesTheRequestedFontSizeIntoTheConfig() throws IOException { + Path config = TerminalThemes.configFileFor(UiTheme.DARK, 15.0); + + String text = Files.readString(config, StandardCharsets.UTF_8); + + assertTrue(text.contains("font-size = 15"), text); + // The bundled theme content must survive alongside it. + assertTrue(text.contains("background = #161514"), text); + } + + @Test + void clampsAnOutOfRangeSizeToTheSupportedBand() throws IOException { + Path tooLarge = TerminalThemes.configFileFor(UiTheme.DARK, 900.0); + Path tooSmall = TerminalThemes.configFileFor(UiTheme.DARK, 0.0); + + assertTrue(Files.readString(tooLarge, StandardCharsets.UTF_8).contains("font-size = 18")); + assertTrue(Files.readString(tooSmall, StandardCharsets.UTF_8).contains("font-size = 10")); + } + + @Test + void cachesPerThemeAndSize() { + Path darkThirteen = TerminalThemes.configFileFor(UiTheme.DARK, 13.0); + + assertEquals(darkThirteen, TerminalThemes.configFileFor(UiTheme.DARK, 13.0)); + assertNotEquals(darkThirteen, TerminalThemes.configFileFor(UiTheme.DARK, 14.0)); + assertNotEquals(darkThirteen, TerminalThemes.configFileFor(UiTheme.LIGHT, 13.0)); + } +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `./gradlew :app:test --tests 'app.drydock.ui.TerminalThemesTest'` +Expected: compilation failure — `configFileFor(UiTheme, double)` does not exist. + +- [ ] **Step 3: Rewrite `TerminalThemes`** + +```java +package app.drydock.ui; + +import app.drydock.domain.UiTheme; + +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; + +/** + * Provides the on-disk ghostty config file matching each {@link UiTheme} and + * terminal font size. The theme bodies live as classpath resources ({@code + * terminal-dark.conf} / {@code terminal-light.conf}, kept next to the theme + * CSS whose tokens they mirror) but {@code ghostty_config_load_file} needs a + * real path, so each (theme, size) pair is materialised once per process + * into a private temp directory. + * + *

The font size rides in the config rather than in {@code + * ghostty_surface_config_s.font_size} deliberately: a theme toggle calls + * {@code ghostty_surface_update_config}, which re-derives a running + * surface's configuration from the app config and would drop a per-surface + * override, silently resetting the user's terminal size mid-session. Going + * through the config file instead means a size change applies live to + * running surfaces over the very same path.

+ */ +final class TerminalThemes { + + /** Supported terminal font sizes; the settings slider exposes the same band. */ + static final double MIN_FONT_SIZE = 10.0; + static final double MAX_FONT_SIZE = 18.0; + + private static final Map EXTRACTED = new HashMap<>(); + + /** Cache key: one materialised config file per theme and rounded font size. */ + private record Key(UiTheme theme, int fontSize) { + } + + private TerminalThemes() { + } + + /** + * The config file for {@code theme} at {@code fontSize} points, clamped + * to {@link #MIN_FONT_SIZE}..{@link #MAX_FONT_SIZE} -- this is the point + * of application, and therefore the owner of the range (the codec + * deliberately stores whatever it was given). + */ + static synchronized Path configFileFor(UiTheme theme, double fontSize) { + int rounded = (int) Math.round(Math.clamp(fontSize, MIN_FONT_SIZE, MAX_FONT_SIZE)); + return EXTRACTED.computeIfAbsent(new Key(theme, rounded), TerminalThemes::extract); + } + + private static Path extract(Key key) { + String resource = key.theme() == UiTheme.LIGHT ? "terminal-light.conf" : "terminal-dark.conf"; + try (InputStream stream = TerminalThemes.class.getResourceAsStream("/app/drydock/ui/" + resource)) { + if (stream == null) { + throw new IllegalStateException("Missing bundled terminal theme resource: " + resource); + } + String body = new String(stream.readAllBytes(), StandardCharsets.UTF_8); + Path dir = Files.createTempDirectory("drydock-terminal-theme"); + dir.toFile().deleteOnExit(); + Path file = dir.resolve(resource); + Files.writeString(file, body + System.lineSeparator() + + "font-size = " + key.fontSize() + System.lineSeparator(), StandardCharsets.UTF_8); + file.toFile().deleteOnExit(); + return file; + } catch (IOException e) { + throw new UncheckedIOException("Could not extract terminal theme config " + resource, e); + } + } +} +``` + +- [ ] **Step 4: Update the three call sites** + +`MainWorkspace.java:437`, `:1181`, and `:1206` call the one-argument form. Each needs the current terminal font size. Add a supplier field next to the existing `themeProvider`: + +```java + /** Where new and re-themed terminals read the persisted font size from. */ + private Supplier terminalFontSizeProvider = () -> WorkspaceUiState.DEFAULT_TERMINAL_FONT_SIZE; + + /** Wires where terminals read the configured font size from (settings modal). */ + public void setTerminalFontSizeProvider(Supplier provider) { + this.terminalFontSizeProvider = + provider == null ? () -> WorkspaceUiState.DEFAULT_TERMINAL_FONT_SIZE : provider; + } +``` + +Then at each call site pass `terminalFontSizeProvider.get()` as the second argument, e.g. line 437 becomes: + +```java + Path configFile = TerminalThemes.configFileFor(theme, terminalFontSizeProvider.get()); +``` + +Import `app.drydock.domain.WorkspaceUiState` if not already imported. + +- [ ] **Step 5: Document that the theme path now carries the size too** + +Do **not** add a separate `applyTerminalFontSize` method — it would be a +pure delegate to `applyTerminalTheme`, which the reviewer would rightly flag. +Instead extend `applyTerminalTheme`'s Javadoc so the shared purpose is +discoverable from the one method that does the work: + +```java + /** + * Re-applies the terminal config to every open terminal (called on the FX + * thread by the theme toggle and by the settings modal's font-size + * slider). ghostty re-reads the whole config file, so theme and font size + * travel together over this one path -- which is exactly why the size + * lives in the config rather than in the per-surface struct, where + * {@code ghostty_surface_update_config} would discard it. + */ +``` + +Task 10 calls `applyTerminalTheme(...)` directly after a size change. + +- [ ] **Step 6: Run the tests** + +Run: `./gradlew :app:test` +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add app/src/main/java/app/drydock/ui/TerminalThemes.java \ + app/src/main/java/app/drydock/ui/MainWorkspace.java \ + app/src/test/java/app/drydock/ui/TerminalThemesTest.java +git commit -m "Carry the terminal font size in the generated ghostty config" +``` + +--- + +### Task 6: `UiFontScale` — the runtime-scaled stylesheet + +This is where the rejected `em` approach would have failed silently, so the test is the substantive part of the task. + +**Files:** +- Create: `app/src/main/java/app/drydock/ui/UiFontScale.java` +- Test: `app/src/test/java/app/drydock/ui/UiFontScaleTest.java` + +**Interfaces:** +- Produces: `static String stylesheetFor(double fontSize)` returning a stylesheet URL (an external form, suitable for `Scene.getStylesheets()`); `static String scaleCss(String css, double factor)` — package-private, pure, the unit under test. + +- [ ] **Step 1: Write the failing test** + +Create `app/src/test/java/app/drydock/ui/UiFontScaleTest.java`: + +```java +package app.drydock.ui; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Scaling multiplies every {@code -fx-font-size} px literal by the factor + * and touches nothing else. + * + *

The nested-rule test is the important one. The rejected alternative -- + * rewriting the declarations as {@code em} -- looks correct line by line but + * compounds, because JavaFX resolves font-relative sizes against the + * inherited font rather than against {@code .root}: {@code .code-area} + * (12px) containing {@code .lineno} (11px) would land at 10.15px instead of + * 13.54px at factor 16/13. Absolute scaling keeps the ratio exact.

+ */ +class UiFontScaleTest { + + @Test + void scalesEveryFontSizeDeclaration() { + String scaled = UiFontScale.scaleCss(".root { -fx-font-size: 13px; }", 16.0 / 13.0); + + assertTrue(scaled.contains("-fx-font-size: 16.0px"), scaled); + } + + @Test + void nestedRulesKeepTheirRatioInsteadOfCompounding() { + String css = """ + .code-area { -fx-font-size: 12px; } + .code-area .lineno { -fx-font-size: 11px; } + """; + + String scaled = UiFontScale.scaleCss(css, 16.0 / 13.0); + + // 12 * 16/13 = 14.769..., 11 * 16/13 = 13.538... + assertTrue(scaled.contains("-fx-font-size: 14.77px"), scaled); + assertTrue(scaled.contains("-fx-font-size: 13.54px"), scaled); + } + + @Test + void preservesFractionalSourceSizes() { + String scaled = UiFontScale.scaleCss(".pill { -fx-font-size: 12.5px; }", 2.0); + + assertTrue(scaled.contains("-fx-font-size: 25.0px"), scaled); + } + + @Test + void leavesEverythingThatIsNotAFontSizeAlone() { + String css = """ + /* -fx-font-size: 99px in a comment must not move */ + .filter-field { + -fx-min-height: 32px; + -fx-max-height: 32px; + -fx-background-color: #161514; + -fx-font-family: "System"; + -fx-padding: 0.5em; + } + """; + + String scaled = UiFontScale.scaleCss(css, 2.0); + + assertTrue(scaled.contains("-fx-min-height: 32px"), scaled); + assertTrue(scaled.contains("-fx-max-height: 32px"), scaled); + assertTrue(scaled.contains("-fx-background-color: #161514"), scaled); + assertTrue(scaled.contains("-fx-padding: 0.5em"), scaled); + assertTrue(scaled.contains("99px in a comment"), scaled); + } + + @Test + void toleratesSpacingVariantsInTheSourceDeclaration() { + String scaled = UiFontScale.scaleCss(".a{-fx-font-size:10px}", 2.0); + + assertTrue(scaled.contains("20.0px"), scaled); + assertFalse(scaled.contains("10px"), scaled); + } + + @Test + void identityFactorReturnsTheOriginalResourceUrl() { + // The unscaled case must not write a temp file at all: it is both the + // default and the hot path at startup. + assertEquals(UiFontScale.baseStylesheetUrl(), UiFontScale.stylesheetFor(13.0)); + } + + @Test + void scaledSizeProducesAUsableStylesheetUrl() { + String url = UiFontScale.stylesheetFor(16.0); + + assertTrue(url.startsWith("file:"), url); + assertFalse(url.equals(UiFontScale.baseStylesheetUrl()), url); + } +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `./gradlew :app:test --tests 'app.drydock.ui.UiFontScaleTest'` +Expected: compilation failure — `UiFontScale` does not exist. + +- [ ] **Step 3: Implement `UiFontScale`** + +```java +package app.drydock.ui; + +import app.drydock.domain.WorkspaceUiState; + +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Produces a copy of {@code app.css} with every {@code -fx-font-size} scaled + * by the user's interface size, materialised as a temp file whose URL the + * scene uses in place of the bundled sheet (the same extract-once-per-process + * pattern {@link TerminalThemes} uses for ghostty configs). + * + *

Two tempting alternatives are wrong. Rewriting the declarations as + * {@code em} compounds, because JavaFX resolves font-relative sizes against + * the font inherited from the nearest styleable ancestor, not against {@code + * .root}: a 12px rule containing an 11px rule would multiply both factors. + * An inline {@code -fx-font-size} on the scene root does override the {@code + * .root} rule, but never reaches combo popups, context menus, or tooltips -- + * they are separate scene graphs, which is exactly why app.css styles them + * with their own top-level selectors. A stylesheet reaches them; an inline + * style does not.

+ */ +final class UiFontScale { + + /** Supported interface font sizes; the settings slider exposes the same band. */ + static final double MIN_FONT_SIZE = 11.0; + static final double MAX_FONT_SIZE = 16.0; + + private static final String BASE_RESOURCE = "/app/drydock/ui/app.css"; + + /** + * Matches a {@code -fx-font-size} declaration's px literal, whatever the + * surrounding spacing. Group 1 is the property and separator (preserved + * verbatim), group 2 the number. + */ + private static final Pattern FONT_SIZE = + Pattern.compile("(-fx-font-size\\s*:\\s*)(\\d+(?:\\.\\d+)?)px"); + + private static final Map GENERATED = new HashMap<>(); + + private UiFontScale() { + } + + /** The bundled, unscaled sheet's URL. */ + static String baseStylesheetUrl() { + return UiFontScale.class.getResource(BASE_RESOURCE).toExternalForm(); + } + + /** + * The stylesheet URL for {@code fontSize}, clamped to {@link + * #MIN_FONT_SIZE}..{@link #MAX_FONT_SIZE} -- this is the point of + * application and therefore owns the range. The default size returns the + * bundled resource untouched; other sizes are generated once and cached. + * + *

Touches the filesystem, so callers on the FX thread must only hit + * the cached or default path (see {@code ThemeManager}).

+ */ + static synchronized String stylesheetFor(double fontSize) { + double clamped = Math.clamp(fontSize, MIN_FONT_SIZE, MAX_FONT_SIZE); + int key = (int) Math.round(clamped * 2); // 0.5px resolution, matching the slider + if (key == (int) Math.round(WorkspaceUiState.DEFAULT_UI_FONT_SIZE * 2)) { + return baseStylesheetUrl(); + } + return GENERATED.computeIfAbsent(key, k -> generate(k / 2.0)); + } + + private static String generate(double fontSize) { + try (InputStream stream = UiFontScale.class.getResourceAsStream(BASE_RESOURCE)) { + if (stream == null) { + throw new IllegalStateException("Missing bundled stylesheet: " + BASE_RESOURCE); + } + String css = new String(stream.readAllBytes(), StandardCharsets.UTF_8); + String scaled = scaleCss(css, fontSize / WorkspaceUiState.DEFAULT_UI_FONT_SIZE); + Path dir = Files.createTempDirectory("drydock-ui-scale"); + dir.toFile().deleteOnExit(); + Path file = dir.resolve("app.css"); + Files.writeString(file, scaled, StandardCharsets.UTF_8); + file.toFile().deleteOnExit(); + return file.toUri().toURL().toExternalForm(); + } catch (IOException e) { + throw new UncheckedIOException("Could not generate the scaled stylesheet", e); + } + } + + /** + * Multiplies every {@code -fx-font-size} px literal by {@code factor}, + * rounded to two decimals, leaving the rest of the text byte-identical. + */ + static String scaleCss(String css, double factor) { + Matcher matcher = FONT_SIZE.matcher(css); + StringBuilder out = new StringBuilder(css.length()); + while (matcher.find()) { + double scaled = Double.parseDouble(matcher.group(2)) * factor; + double rounded = Math.round(scaled * 100.0) / 100.0; + matcher.appendReplacement(out, Matcher.quoteReplacement(matcher.group(1) + rounded + "px")); + } + matcher.appendTail(out); + return out.toString(); + } +} +``` + +- [ ] **Step 4: Run the tests** + +Run: `./gradlew :app:test --tests 'app.drydock.ui.UiFontScaleTest'` +Expected: PASS. If `leavesEverythingThatIsNotAFontSizeAlone` fails on the comment case, the regex has drifted — it must match only `-fx-font-size` followed by a colon. + +- [ ] **Step 5: Commit** + +```bash +git add app/src/main/java/app/drydock/ui/UiFontScale.java \ + app/src/test/java/app/drydock/ui/UiFontScaleTest.java +git commit -m "Generate a font-scaled copy of app.css at runtime" +``` + +--- + +### Task 7: Apply the interface size from `ThemeManager` + +**Files:** +- Modify: `app/src/main/java/app/drydock/ui/ThemeManager.java` +- Modify: `app/src/main/java/app/drydock/ui/AppShell.java` (constructor ~line 49, `toggleTheme` ~line 143) +- Modify: `app/src/main/java/app/drydock/DrydockApplication.java` (`new AppShell(...)` ~line 191) + +**Interfaces:** +- Consumes: `UiFontScale.stylesheetFor(double)`, `UiFontScale.MIN_FONT_SIZE`, `UiFontScale.MAX_FONT_SIZE` (Task 6). +- Produces: `ThemeManager.setTheme(UiTheme)`, `ThemeManager.setUiFontSize(double)`, `ThemeManager.uiFontSize()`; `AppShell.themeManager()` already exposes the manager. + +- [ ] **Step 1: Rewrite `ThemeManager`'s apply path** + +Replace the constructor, add the absolute setters, and route `apply()` through the scaled sheet: + +```java + private final Scene scene; + private final Consumer onThemeChanged; + private UiTheme theme; + private double uiFontSize; + + public ThemeManager(Scene scene, UiTheme initialTheme, double initialUiFontSize, + Consumer onThemeChanged) { + this.scene = scene; + this.onThemeChanged = onThemeChanged; + this.theme = initialTheme; + this.uiFontSize = Math.clamp(initialUiFontSize, UiFontScale.MIN_FONT_SIZE, UiFontScale.MAX_FONT_SIZE); + loadBundledFonts(); + apply(); + } + + public UiTheme theme() { + return theme; + } + + public double uiFontSize() { + return uiFontSize; + } + + public void toggle() { + setTheme(theme.other()); + } + + /** Sets the theme absolutely (the settings modal's radio); {@link #toggle} delegates here. */ + public void setTheme(UiTheme newTheme) { + if (newTheme == theme) { + return; + } + theme = newTheme; + apply(); + onThemeChanged.accept(theme); + } + + /** + * Applies an interface font size by swapping in a stylesheet whose + * font sizes are scaled (see {@link UiFontScale}); clamped here because + * this is the point of application. Persisting the choice is the + * caller's job, exactly as with the theme. + */ + public void setUiFontSize(double newUiFontSize) { + double clamped = Math.clamp(newUiFontSize, UiFontScale.MIN_FONT_SIZE, UiFontScale.MAX_FONT_SIZE); + if (clamped == uiFontSize) { + return; + } + uiFontSize = clamped; + apply(); + } + + private void apply() { + scene.getStylesheets().setAll( + UiFontScale.stylesheetFor(uiFontSize), + resource(theme.stylesheet())); + } +``` + +Note `apply()` no longer uses `resource("app.css")`; keep `resource(...)` for the theme sheet. + +Update the class Javadoc: the scene carries a (possibly font-scaled) `app.css` plus exactly one token sheet. + +- [ ] **Step 2: Thread the initial size through `AppShell`** + +In `AppShell`'s constructor signature, add `double initialUiFontSize` immediately after `initialTheme`, and pass it to the `ThemeManager` constructor: + +```java + themeManager = new ThemeManager(scene, initialTheme, initialUiFontSize, theme -> { + titleBar.showThemeGlyphFor(theme); + onThemeChanged.accept(theme); + }); +``` + +Applying it in the constructor means the scaled sheet is in place before the stage is shown, so there is no visible re-layout at startup. + +- [ ] **Step 3: Pass the persisted value at the call site** + +In `DrydockApplication` (~line 191), the `new AppShell(...)` call gains the new argument: + +```java + appShell = new AppShell(primaryStage, WINDOW_TITLE, sidebar, mainWorkspace, + repositoryManager.state().ui().sidebarWidth(), + repositoryManager.state().ui().theme(), + repositoryManager.state().ui().uiFontSize(), + theme -> { +``` + +- [ ] **Step 4: Verify it builds and the suite still passes** + +Run: `./gradlew :app:test` +Expected: PASS. + +- [ ] **Step 5: Verify by eye** + +Run: `./gradlew :app:run` + +Expected: the app looks exactly as it did before this task. The stored size is still the 13.0 default, and `UiFontScale.stylesheetFor(13.0)` returns the bundled resource untouched, so any visible change here means the scaling path is being taken when it should not be. Task 11's manual check exercises the scaled path once the slider exists. + +- [ ] **Step 6: Commit** + +```bash +git add app/src/main/java/app/drydock/ui/ThemeManager.java \ + app/src/main/java/app/drydock/ui/AppShell.java \ + app/src/main/java/app/drydock/DrydockApplication.java +git commit -m "Apply the interface font size through the theme manager" +``` + +--- + +### Task 8: `UserConfig` write support + +**Files:** +- Modify: `app/src/main/java/app/drydock/config/UserConfig.java` +- Test: `app/src/test/java/app/drydock/config/UserConfigTest.java` + +**Interfaces:** +- Produces: `static void save(UserConfig config, Path configFile) throws IOException`, `static CompletableFuture saveAsync(UserConfig config)`, `static void flushPendingSaves()`. + +- [ ] **Step 1: Write the failing tests** + +Append to `UserConfigTest` (it already has a `@TempDir`-style setup — reuse whatever the file uses): + +```java + @Test + void savedConfigLoadsBackUnchanged() throws Exception { + Path configFile = tempDir.resolve("config.json"); + + UserConfig.save(new UserConfig(Optional.of(Path.of("/tmp/worktrees"))), configFile); + + assertEquals(Optional.of(Path.of("/tmp/worktrees")), UserConfig.load(configFile).worktreesDirectory()); + } + + @Test + void saveCreatesTheParentDirectory() throws Exception { + Path configFile = tempDir.resolve("nested").resolve(".drydock").resolve("config.json"); + + UserConfig.save(new UserConfig(Optional.of(Path.of("/tmp/worktrees"))), configFile); + + assertTrue(Files.exists(configFile)); + } + + @Test + void saveOverwritesAMalformedFileAndLeavesNoTempBehind() throws Exception { + Path configFile = tempDir.resolve("config.json"); + Files.writeString(configFile, "{ this is not json"); + + UserConfig.save(new UserConfig(Optional.of(Path.of("/tmp/worktrees"))), configFile); + + assertEquals(Optional.of(Path.of("/tmp/worktrees")), UserConfig.load(configFile).worktreesDirectory()); + try (var entries = Files.list(tempDir)) { + assertEquals(List.of("config.json"), + entries.map(p -> p.getFileName().toString()).sorted().toList()); + } + } + + @Test + void savePreservesMembersItDoesNotKnowAbout() throws Exception { + // The file is human-editable and may grow keys this build predates; + // rewriting it must not silently delete the user's other settings. + Path configFile = tempDir.resolve("config.json"); + Files.writeString(configFile, "{\"worktreesDirectory\":\"/old\",\"somethingElse\":42}"); + + UserConfig.save(new UserConfig(Optional.of(Path.of("/tmp/worktrees"))), configFile); + + String written = Files.readString(configFile); + assertTrue(written.contains("somethingElse"), written); + assertTrue(written.contains("/tmp/worktrees"), written); + } + + @Test + void savingAnEmptyConfigClearsTheDirectory() throws Exception { + Path configFile = tempDir.resolve("config.json"); + UserConfig.save(new UserConfig(Optional.of(Path.of("/tmp/worktrees"))), configFile); + + UserConfig.save(UserConfig.empty(), configFile); + + assertEquals(Optional.empty(), UserConfig.load(configFile).worktreesDirectory()); + } +``` + +Add imports as needed (`java.util.List`, `java.nio.file.Files`, `org.junit.jupiter.api.Assertions.assertTrue`). + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `./gradlew :app:test --tests 'app.drydock.config.UserConfigTest'` +Expected: compilation failure — `cannot find symbol: method save(UserConfig, Path)`. + +- [ ] **Step 3: Implement `save`** + +Add to `UserConfig`, using the repository's own JSON writer: + +```java + /** + * Writes {@code config} to {@code configFile}: temp file in the same + * directory, then an atomic move, so a crash mid-write can never leave a + * truncated config where a valid one was. + * + *

Members this build does not know about are read back from the + * existing file and preserved -- the file is hand-editable, so silently + * dropping a key a newer build (or the user) put there would be data + * loss.

+ * + *

Unlike {@link #load()}, a failure here throws: a save is a + * user-visible action, and one that silently did nothing is worse than + * one that reports why.

+ */ + static void save(UserConfig config, Path configFile) throws IOException { + JsonObject root = JsonObject.empty(); + if (Files.exists(configFile)) { + try { + if (JsonParser.parse(Files.readString(configFile, StandardCharsets.UTF_8)) + instanceof JsonObject existing) { + root = existing; + } + } catch (IOException | JsonParseException e) { + LOG.log(Level.WARNING, "Existing config " + configFile + + " is unreadable or malformed; replacing it", e); + } + } + root.members().remove("worktreesDirectory"); + config.worktreesDirectory().ifPresent(dir -> + root.put("worktreesDirectory", new JsonString(dir.toString()))); + + Path parent = configFile.getParent(); + if (parent != null) { + Files.createDirectories(parent); + } + Path temp = Files.createTempFile(parent, "config", ".json.tmp"); + try { + Files.writeString(temp, JsonWriter.write(root), StandardCharsets.UTF_8); + Files.move(temp, configFile, StandardCopyOption.REPLACE_EXISTING, + StandardCopyOption.ATOMIC_MOVE); + } finally { + Files.deleteIfExists(temp); + } + } +``` + +Imports to add: `app.drydock.state.json.JsonWriter`, `java.nio.file.StandardCopyOption`. + +If `JsonObject.members()` returns an immutable map, build a fresh `JsonObject` and copy the existing members into it instead of removing in place — check `JsonValue.java:22-43` and follow whichever the record supports. + +- [ ] **Step 4: Implement `saveAsync` and `flushPendingSaves`** + +```java + private static final AtomicReference> PENDING_SAVE = + new AtomicReference<>(CompletableFuture.completedFuture(null)); + + /** + * As {@link #save}, off the caller's thread -- the settings modal calls + * this from the FX thread, where a synchronous write is forbidden. The + * returned future completes exceptionally on failure so the caller can + * surface it; it is never swallowed. + */ + public static CompletableFuture saveAsync(UserConfig config) { + CompletableFuture future = new CompletableFuture<>(); + PENDING_SAVE.set(future); + Thread.ofVirtual().start(() -> { + try { + save(config, defaultConfigFile()); + future.complete(null); + } catch (IOException | RuntimeException e) { + future.completeExceptionally(e); + } + }); + return future; + } + + /** + * Awaits any in-flight {@link #saveAsync} (AGENTS.md: a service writing + * files from a background thread exposes a flush, so shutdown and tests + * do not race a pending write). Bounded, because a wedged disk must not + * hang shutdown -- the atomic move means the worst case is a stale file, + * never a corrupt one. + */ + public static void flushPendingSaves() { + try { + PENDING_SAVE.get().get(10, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } catch (ExecutionException | TimeoutException e) { + // saveAsync's caller already reported the failure to the user. + } + } +``` + +Imports: `java.util.concurrent.atomic.AtomicReference`, `java.util.concurrent.ExecutionException`, `java.util.concurrent.TimeUnit`, `java.util.concurrent.TimeoutException`. + +- [ ] **Step 5: Call the flush at shutdown** + +In `DrydockApplication.stop()`, after the sidebar-width block: + +```java + closeQuietly("UserConfig saves", UserConfig::flushPendingSaves); +``` + +Import `app.drydock.config.UserConfig`. + +- [ ] **Step 6: Run the tests** + +Run: `./gradlew :app:test` +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add app/src/main/java/app/drydock/config/UserConfig.java \ + app/src/main/java/app/drydock/DrydockApplication.java \ + app/src/test/java/app/drydock/config/UserConfigTest.java +git commit -m "Let the user config be written, atomically and off the FX thread" +``` + +--- + +### Task 9: The settings modal + +**Files:** +- Create: `app/src/main/java/app/drydock/ui/SettingsModal.java` +- Modify: `app/src/main/resources/app/drydock/ui/app.css` (new settings rules) + +**Interfaces:** +- Consumes: `ThemeManager.setTheme/setUiFontSize/theme/uiFontSize` (Task 7), `RepositoryManager.updateUiFontSize/updateTerminalFontSize` (Task 4), `UserConfig.loadAsync/saveAsync` (Task 8), `UiFontScale.MIN_FONT_SIZE/MAX_FONT_SIZE` (Task 6), `TerminalThemes.MIN_FONT_SIZE/MAX_FONT_SIZE` (Task 5), `UiErrors.show(String title, Throwable failure)`. +- Produces: `public final class SettingsModal extends VBox` with `public SettingsModal(Settings settings, Runnable onClose)` and the `public interface SettingsModal.Settings` defined below. + +`SettingsModal` is **public**, matching `GitHubCloneModal` and `RemoteRepositoryModal` (`GitHubCloneModal.java:32`, `RemoteRepositoryModal.java:30`) — the modals constructed from `DrydockApplication`, which lives in a different package. `ShortcutsOverlay` is package-private only because nothing outside `app.drydock.ui` builds it. + +- [ ] **Step 1: Define the modal's collaborator interface and skeleton** + +Create `SettingsModal.java`. It takes one bundle of callbacks so it depends on no manager directly — the same shape the other modals use: + +```java +package app.drydock.ui; + +import app.drydock.domain.UiTheme; +import javafx.application.Platform; +import javafx.geometry.Pos; +import javafx.scene.control.Button; +import javafx.scene.control.Label; +import javafx.scene.control.RadioButton; +import javafx.scene.control.Slider; +import javafx.scene.control.TextField; +import javafx.scene.control.ToggleGroup; +import javafx.scene.layout.HBox; +import javafx.scene.layout.Priority; +import javafx.scene.layout.Region; +import javafx.scene.layout.VBox; +import javafx.stage.DirectoryChooser; + +import java.io.File; +import java.nio.file.Path; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.function.DoubleConsumer; + +/** + * The Settings modal, reached from the title-bar gear or ⌘,. Four settings, + * applied as they change (the macOS preferences convention -- there is no + * OK/Cancel; Done, Esc, and × all just close). + * + *

Holds no manager or store: everything it can change arrives as a + * callback in {@link Settings}, so persistence stays with the single state + * writer and this class stays a view.

+ */ +public final class SettingsModal extends VBox { + + /** Everything the modal reads and writes, supplied by the application wiring. */ + public interface Settings { + UiTheme theme(); + + void setTheme(UiTheme theme); + + double uiFontSize(); + + /** Applies the interface size live; persistence is the implementation's job. */ + void setUiFontSize(double size); + + double terminalFontSize(); + + /** Applies the terminal size to running surfaces live; persistence likewise. */ + void setTerminalFontSize(double size); + + CompletableFuture> loadWorktreesDirectory(); + + CompletableFuture saveWorktreesDirectory(Optional directory); + } +} +``` + +- [ ] **Step 2: Build the layout** + +Fill in the constructor, following `StartSessionModal`'s structure (header with title + `×`, body, footer): + +```java + private static final double MODAL_WIDTH = 520; + + public SettingsModal(Settings settings, Runnable onClose) { + getStyleClass().add("modal"); + setMaxWidth(MODAL_WIDTH); + setMaxHeight(Region.USE_PREF_SIZE); + setSpacing(12); + + Label title = new Label("Settings"); + title.getStyleClass().add("modal-title"); + Button close = new Button("×"); + close.getStyleClass().add("icon-button"); + close.setOnAction(e -> onClose.run()); + Region headerSpacer = new Region(); + HBox.setHgrow(headerSpacer, Priority.ALWAYS); + HBox header = new HBox(8, title, headerSpacer, close); + header.setAlignment(Pos.CENTER_LEFT); + + Button done = new Button("Done"); + done.getStyleClass().add("primary-button"); + done.setDefaultButton(true); + done.setOnAction(e -> onClose.run()); + Region footerSpacer = new Region(); + HBox.setHgrow(footerSpacer, Priority.ALWAYS); + HBox footer = new HBox(8, footerSpacer, done); + + getChildren().addAll(header, + sectionTitle("Appearance"), + themeRow(settings), + sizeRow("Interface size", UiFontScale.MIN_FONT_SIZE, UiFontScale.MAX_FONT_SIZE, + settings.uiFontSize(), settings::setUiFontSize), + sizeRow("Terminal size", TerminalThemes.MIN_FONT_SIZE, TerminalThemes.MAX_FONT_SIZE, + settings.terminalFontSize(), settings::setTerminalFontSize), + sectionTitle("Worktrees"), + worktreesRow(settings), + footer); + } + + private static Label sectionTitle(String text) { + Label label = new Label(text); + label.getStyleClass().add("settings-section-title"); + return label; + } +``` + +Use `primary-button` only if that style class already exists in app.css; otherwise use the plain default (grep app.css for the class the other modals' confirm buttons use and match it). + +- [ ] **Step 3: Implement the theme row** + +```java + private static Region themeRow(Settings settings) { + ToggleGroup group = new ToggleGroup(); + RadioButton dark = new RadioButton("Dark"); + RadioButton light = new RadioButton("Light"); + dark.setToggleGroup(group); + light.setToggleGroup(group); + (settings.theme() == UiTheme.LIGHT ? light : dark).setSelected(true); + + // Applied on selection, not on Done: the whole modal is apply-on-change. + group.selectedToggleProperty().addListener((obs, old, selected) -> + settings.setTheme(selected == light ? UiTheme.LIGHT : UiTheme.DARK)); + + return labelled("Theme", new HBox(12, dark, light)); + } + + /** A settings row: a fixed-width caption on the left, the control on the right. */ + private static Region labelled(String caption, Region control) { + Label label = new Label(caption); + label.getStyleClass().add("settings-row-label"); + label.setMinWidth(120); + HBox row = new HBox(12, label, control); + row.setAlignment(Pos.CENTER_LEFT); + HBox.setHgrow(control, Priority.ALWAYS); + return row; + } +``` + +- [ ] **Step 4: Implement the size rows** + +```java + /** + * A font-size slider. The value applies live while dragging so the effect + * is visible, but the persisting callback fires only when the drag ends -- + * a state write per pixel would be pointless disk traffic. + */ + private static Region sizeRow(String caption, double min, double max, double initial, + DoubleConsumer onChanged) { + Slider slider = new Slider(min, max, Math.clamp(initial, min, max)); + slider.setMajorTickUnit(1); + slider.setMinorTickCount(1); + slider.setSnapToTicks(true); + slider.setBlockIncrement(1); + + Label value = new Label(format(slider.getValue())); + value.getStyleClass().add("settings-value"); + value.setMinWidth(52); + + slider.valueProperty().addListener((obs, old, now) -> { + value.setText(format(now.doubleValue())); + onChanged.accept(now.doubleValue()); + }); + + HBox control = new HBox(10, slider, value); + control.setAlignment(Pos.CENTER_LEFT); + HBox.setHgrow(slider, Priority.ALWAYS); + return labelled(caption, control); + } + + /** "13 px" / "13.5 px" -- no trailing zero on whole sizes. */ + static String format(double size) { + return (size == Math.rint(size) + ? String.valueOf((int) size) + : String.valueOf(Math.round(size * 10) / 10.0)) + " px"; + } +``` + +The listener applies on every change, which for a snap-to-tick slider is once per whole step, not once per pixel — so live application and persistence coincide and no debounce is needed. `Settings`' implementations (Task 10) are what persist. + +- [ ] **Step 5: Implement the worktrees row** + +```java + private static Region worktreesRow(Settings settings) { + TextField field = new TextField(); + field.setPromptText("Loading…"); + field.setDisable(true); + Button browse = new Button("Browse…"); + browse.setDisable(true); + + Label hint = new Label("New worktrees are created here."); + hint.getStyleClass().add("settings-hint"); + + // Load off the FX thread (UserConfig reads the file); the controls + // stay disabled with a "Loading…" prompt until it lands, so the row + // never shows a stale or empty value as if it were the real one. + settings.loadWorktreesDirectory().whenComplete((directory, failure) -> Platform.runLater(() -> { + field.setDisable(false); + browse.setDisable(false); + field.setPromptText(System.getProperty("user.home") + "/dev/wt"); + if (failure == null) { + directory.ifPresent(dir -> field.setText(dir.toString())); + } else { + UiErrors.show("Could not read the settings file", failure); + } + })); + + Runnable commit = () -> { + String text = field.getText() == null ? "" : field.getText().strip(); + Optional directory = text.isEmpty() ? Optional.empty() : Optional.of(Path.of(text)); + field.setDisable(true); + browse.setDisable(true); + settings.saveWorktreesDirectory(directory).whenComplete((ignored, failure) -> + Platform.runLater(() -> { + // Every path re-enables: success, failure, and the + // early return inside saveWorktreesDirectory. + field.setDisable(false); + browse.setDisable(false); + if (failure != null) { + UiErrors.show("Could not save the worktrees directory", failure); + } + })); + }; + + field.setOnAction(e -> commit.run()); + field.focusedProperty().addListener((obs, had, has) -> { + if (!has) { + commit.run(); + } + }); + + browse.setOnAction(e -> { + DirectoryChooser chooser = new DirectoryChooser(); + chooser.setTitle("Choose the worktrees directory"); + File chosen = chooser.showDialog(getScene() == null ? null : getScene().getWindow()); + if (chosen != null) { + field.setText(chosen.getAbsolutePath()); + commit.run(); + } + }); + + HBox control = new HBox(8, field, browse); + control.setAlignment(Pos.CENTER_LEFT); + HBox.setHgrow(field, Priority.ALWAYS); + return new VBox(4, labelled("Directory", control), hint); + } +``` + +`worktreesRow` calls `getScene()`, so it is an **instance** method — `private Region worktreesRow(Settings settings)` — while `sectionTitle`, `labelled`, `sizeRow`, and `themeRow` stay `private static`. In Step 2's constructor the call is therefore `worktreesRow(settings)` (unqualified, on `this`), which is what the listing already shows. + +`UiErrors.show(String title, Throwable failure)` is package-private static in `app.drydock.ui` (`UiErrors.java:32`), so `SettingsModal` can call it directly with the two arguments shown. + +- [ ] **Step 6: Add the CSS** + +Append to `app/src/main/resources/app/drydock/ui/app.css`, matching the token names the neighbouring modal rules use (grep for `-drydock-muted` / `-drydock-text` to confirm): + +```css +/* --- Settings modal ------------------------------------------------- */ +.settings-section-title { + -fx-font-size: 11px; + -fx-text-fill: -drydock-muted; + -fx-padding: 6px 0 0 0; +} + +.settings-row-label { + -fx-font-size: 12.5px; + -fx-text-fill: -drydock-text; +} + +.settings-value { + -fx-font-size: 12px; + -fx-text-fill: -drydock-muted; +} + +.settings-hint { + -fx-font-size: 11px; + -fx-text-fill: -drydock-muted; + -fx-padding: 0 0 0 132px; +} +``` + +These px font sizes are correct: `UiFontScale` scales them along with the rest of the sheet. + +- [ ] **Step 7: Write the formatter test** + +Create `app/src/test/java/app/drydock/ui/SettingsModalTest.java`: + +```java +package app.drydock.ui; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** The slider's readout: whole sizes lose the decimal, halves keep it. */ +class SettingsModalTest { + + @Test + void formatsWholeSizesWithoutADecimal() { + assertEquals("13 px", SettingsModal.format(13.0)); + } + + @Test + void formatsHalfSizesWithOneDecimal() { + assertEquals("13.5 px", SettingsModal.format(13.5)); + } +} +``` + +- [ ] **Step 8: Run the tests** + +Run: `./gradlew :app:test` +Expected: PASS. The modal itself is not instantiated in tests (it needs a JavaFX toolkit); its logic lives in `format` and in the Task 10 wiring. + +- [ ] **Step 9: Commit** + +```bash +git add app/src/main/java/app/drydock/ui/SettingsModal.java \ + app/src/main/resources/app/drydock/ui/app.css \ + app/src/test/java/app/drydock/ui/SettingsModalTest.java +git commit -m "Add the settings modal" +``` + +--- + +### Task 10: Wire the modal to the gear button and ⌘, + +**Files:** +- Modify: `app/src/main/java/app/drydock/ui/TitleBar.java` +- Modify: `app/src/main/java/app/drydock/ui/AppShell.java` +- Modify: `app/src/main/java/app/drydock/ui/ShortcutsOverlay.java` (`SHORTCUTS`, ~line 21) +- Modify: `app/src/main/java/app/drydock/DrydockApplication.java` (`installGlobalShortcuts` ~line 523, wiring ~line 191) + +**Interfaces:** +- Consumes: `SettingsModal(Settings, Runnable)` (Task 9), `AppShell.modalLayer()`, `AppShell.themeManager()`, `MainWorkspace.applyTerminalTheme(UiTheme)` (Task 5). +- Produces: `AppShell.setOnShowSettings(Runnable)` and `AppShell.showSettings()`. + +- [ ] **Step 1: Add the gear button to the title bar** + +In `TitleBar`, add the callback parameter and the button. The constructor becomes: + +```java + TitleBar(Stage stage, String title, Runnable onHelp, Runnable onSettings, Runnable onThemeToggle, + Runnable onSidebarToggle) { +``` + +and the right-hand group: + +```java + Button helpButton = iconButton("?", "Keyboard shortcuts (?)"); + helpButton.setOnAction(e -> onHelp.run()); + Button settingsButton = iconButton("⚙", "Settings (⌘,)"); + settingsButton.setOnAction(e -> onSettings.run()); + themeButton.setOnAction(e -> onThemeToggle.run()); + HBox right = new HBox(4, helpButton, settingsButton, themeButton); +``` + +- [ ] **Step 2: Expose it on `AppShell`** + +```java + private Runnable onShowSettings = () -> { }; + + /** Wires the gear button and ⌘, to the settings modal (supplied by the application). */ + public void setOnShowSettings(Runnable onShowSettings) { + this.onShowSettings = onShowSettings == null ? () -> { } : onShowSettings; + } + + public void showSettings() { + onShowSettings.run(); + } +``` + +and pass `this::showSettings` as the new `TitleBar` argument in the constructor. + +- [ ] **Step 3: Add the shortcut row** + +In `ShortcutsOverlay.SHORTCUTS`, after the `Toggle theme` entry: + +```java + {"Settings", "⌘,"}, +``` + +- [ ] **Step 4: Handle ⌘, in the global shortcut filter** + +In `installGlobalShortcuts`, add a branch alongside the others: + +```java + } else if (cmd && event.getCode() == KeyCode.COMMA) { + // Inert while another modal is up: ⌘, must never replace an + // in-progress Start-session or New-worktree modal underneath + // the user. (The gear button is unreachable then anyway -- + // the backdrop covers the title bar.) + if (!appShell.modalLayer().isShowingModal()) { + appShell.showSettings(); + } + event.consume(); +``` + +- [ ] **Step 5: Wire the `Settings` implementation** + +In `DrydockApplication.start`, after the `mainWorkspace.setThemeProvider(...)` line, add the terminal-size provider and the settings wiring: + +```java + mainWorkspace.setTerminalFontSizeProvider( + () -> repositoryManager.state().ui().terminalFontSize()); + + appShell.setOnShowSettings(() -> appShell.modalLayer().show( + new SettingsModal(new SettingsModal.Settings() { + @Override + public UiTheme theme() { + return appShell.themeManager().theme(); + } + + @Override + public void setTheme(UiTheme theme) { + // Persists and re-themes terminals via AppShell's own + // onThemeChanged callback; no duplicate write here. + appShell.themeManager().setTheme(theme); + } + + @Override + public double uiFontSize() { + return appShell.themeManager().uiFontSize(); + } + + @Override + public void setUiFontSize(double size) { + appShell.themeManager().setUiFontSize(size); + repositoryManager.updateUiFontSize(size); + } + + @Override + public double terminalFontSize() { + return repositoryManager.state().ui().terminalFontSize(); + } + + @Override + public void setTerminalFontSize(double size) { + // Persist first: applyTerminalFontSize re-reads the + // size through the provider above. + repositoryManager.updateTerminalFontSize(size); + mainWorkspace.applyTerminalTheme(appShell.themeManager().theme()); + } + + @Override + public CompletableFuture> loadWorktreesDirectory() { + return UserConfig.loadAsync().thenApply(UserConfig::worktreesDirectory); + } + + @Override + public CompletableFuture saveWorktreesDirectory(Optional directory) { + return UserConfig.saveAsync(new UserConfig(directory)); + } + }, appShell.modalLayer()::close))); +``` + +Add `import app.drydock.ui.SettingsModal;` alongside the existing `GitHubCloneModal` / `RemoteRepositoryModal` imports at the top of `DrydockApplication` (both are public for exactly this reason, and Task 9 makes `SettingsModal` public to match). Also import `app.drydock.config.UserConfig`, `java.nio.file.Path`, `java.util.Optional`, and `java.util.concurrent.CompletableFuture` if they are not already present. + +- [ ] **Step 6: Run the tests and the app** + +Run: `./gradlew :app:test` +Expected: PASS. + +Run: `./gradlew :app:run` + +Verify by hand: +1. The gear appears between `?` and the theme glyph; clicking it opens Settings. +2. `⌘,` opens it; `⌘,` again while it is open does nothing (no stacked modal). +3. Esc, `×`, and Done all close it. +4. `?` shows `Settings ⌘,` in the shortcuts list. + +- [ ] **Step 7: Commit** + +```bash +git add app/src/main/java/app/drydock/ui/TitleBar.java \ + app/src/main/java/app/drydock/ui/AppShell.java \ + app/src/main/java/app/drydock/ui/ShortcutsOverlay.java \ + app/src/main/java/app/drydock/ui/SettingsModal.java \ + app/src/main/java/app/drydock/DrydockApplication.java +git commit -m "Open settings from the title-bar gear and command-comma" +``` + +--- + +### Task 11: End-to-end verification and the manual checklist + +**Files:** +- Modify: `docs/manual-terminal-checklist.md` + +- [ ] **Step 1: Add the checklist entries** + +Append to `docs/manual-terminal-checklist.md`: + +```markdown +## Settings — terminal font size + +1. Open a Claude session so a ghostty surface is live. +2. Open Settings (⌘,) and drag **Terminal size** to 18. +3. The running terminal's text grows immediately — not only new sessions. +4. Toggle the theme (⌘⇧L). The terminal re-themes and **keeps** size 18. + (A regression here means the size went back to a per-surface override, + which `ghostty_surface_update_config` discards.) +5. Quit and relaunch. The terminal opens at 18. + +## Settings — interface font size + +1. Open Settings and sweep **Interface size** across 11 → 16. +2. At both extremes check for clipping in: the title bar and traffic + lights, the sidebar filter field, the icon buttons, a combo-box popup + (New worktree ▸ Fork from), a right-click context menu, and a tooltip. + The popups and menus must scale with everything else — they are separate + scene graphs, and an implementation that only styled the main scene would + leave them at 13px. +3. Fixed-height controls (filter field 32px, icon buttons 30px, title bar + 44px) do not grow; confirm their text still fits at 16. +4. Quit and relaunch. The size is restored with no visible re-layout flash. +``` + +- [ ] **Step 2: Run the whole suite** + +Run: `./gradlew :app:test` +Expected: PASS. + +- [ ] **Step 3: Walk the checklist** + +Run: `./gradlew :app:run` and perform every step above. Record any clipping found at 16px; if a control genuinely breaks, narrow `UiFontScale.MAX_FONT_SIZE` and note it in the spec rather than shipping a broken maximum. + +- [ ] **Step 4: Verify persistence end to end** + +```bash +cat ~/.drydock/config.json +``` +Expected: contains `worktreesDirectory` if one was set in the modal. + +Find the state file (`ApplicationStateRepository.stateFile()`; it sits next to the config under `~/.drydock/`) and confirm its `ui` object now carries `uiFontSize` and `terminalFontSize`. + +- [ ] **Step 5: Commit** + +```bash +git add docs/manual-terminal-checklist.md +git commit -m "Add settings checks to the manual checklist" +``` + +--- + +## Notes for the implementer + +- **Task 1 is a gate, not a formality.** If libghostty ignores `font-size` in the config file, terminal font size is cut from this round. Do not substitute the per-surface `ghostty_surface_config_s.font_size` write — the spec explains at length why that route silently reverts on the next theme toggle. +- **Do not convert `app.css` to `em`.** It is the obvious-looking approach and it is wrong; `UiFontScaleTest.nestedRulesKeepTheirRatioInsteadOfCompounding` exists to catch anyone who tries. +- **Do not add clamping to the codec.** Ranges belong to `ThemeManager` and `TerminalThemes`. `ApplicationStateCodecTest.outOfRangeFontSizeSurvivesTheDecodeUnchanged` locks this in. diff --git a/docs/superpowers/specs/2026-07-25-settings-ui-design.md b/docs/superpowers/specs/2026-07-25-settings-ui-design.md new file mode 100644 index 0000000..989ae81 --- /dev/null +++ b/docs/superpowers/specs/2026-07-25-settings-ui-design.md @@ -0,0 +1,301 @@ +# Settings UI + +A small preferences panel, reachable from the title bar, exposing the four +settings that have (or can cheaply gain) real consumers: theme, interface +font size, terminal font size, and the worktrees directory. + +## Motivation + +Drydock has no settings surface. Theme is toggleable only from the title-bar +glyph, and `worktreesDirectory` — the one genuinely user-editable setting — +can only be changed by hand-editing `~/.drydock/config.json`, which is +undiscoverable. Font sizes are not configurable at all. + +## Entry point + +The app runs an undecorated stage with a custom `TitleBar`; there is no menu +bar and none is introduced. A `⚙` icon button is added to the title bar's +right-hand group, between the `?` (shortcuts) and theme-toggle buttons: + +``` +[● ● ●] [◧] Drydock [ ? ] [ ⚙ ] [ ☾ ] +``` + +- Tooltip: `Settings (⌘,)`. +- `AppShell` gains `showSettings()`, mirroring the existing + `showShortcutsOverlay()`. +- `DrydockApplication`'s scene-level `KEY_PRESSED` filter gains a + `cmd + COMMA` branch alongside the existing `⌘⇧L` / `⌘0` / `⌘F` branches. +- `ShortcutsOverlay.SHORTCUTS` gains a `{"Settings", "⌘,"}` row. + +## Settings and where they persist + +Two persistence stores already exist and the settings split cleanly across +them: + +| Setting | Home | Default | +| --- | --- | --- | +| Theme | `WorkspaceUiState.theme` (existing) | `DARK` | +| Interface font size | `WorkspaceUiState.uiFontSize` (new) | `13.0` | +| Terminal font size | `WorkspaceUiState.terminalFontSize` (new) | `13.0` | +| Worktrees directory | `UserConfig.worktreesDirectory` (existing) | `/dev/wt` | + +Cosmetic, app-owned UI state lives in `WorkspaceUiState` alongside `theme` +and `sidebarWidth`. The human-editable `~/.drydock/config.json` keeps the +worktrees directory, which already has a consumer (`NewWorktreeModal` / +`WorktreeNaming`). + +No single user action writes both stores, so there is no cross-store +consistency concern. + +### `WorkspaceUiState` fields + +`uiFontSize` and `terminalFontSize` are added as `double` components with +`with…` copy methods, following the record's existing shape. + +`ApplicationStateCodec` encodes them as JSON numbers and decodes them +**leniently**, consistent with how `sidebarWidth` / `theme` already decode: + +- member absent → default, +- member present but not a number → default. + +A malformed value is never a reason to fail the whole decode. The change is +purely additive to the JSON schema; documents written before this change +decode to the defaults. + +The codec deliberately does **not** clamp, matching `sidebarWidth` +(`ApplicationStateCodec.java:304-306` accepts any number; the SplitPane +clamps at use). Range ownership sits in exactly one place — the point of +application — so a hand-edited out-of-range value is honoured as far as it +can be rather than silently rewritten: + +- `uiFontSize`: clamped to 11.0 – 16.0 by `ThemeManager.setUiFontSize`, +- `terminalFontSize`: clamped to 10.0 – 18.0 by `TerminalThemes`. + +The sliders expose the same ranges, so in normal use the clamp never fires. + +### Who writes it + +`RepositoryManager` is the only holder of the `ApplicationStateStore` and +already owns the equivalent writes (`updateSidebarWidth` /`updateTheme`, +`RepositoryManager.java:181,190`). It gains `updateUiFontSize(double)` and +`updateTerminalFontSize(double)` in the same shape: a state-transform +submitted to the single writer, never a load-then-save. `SettingsModal` +receives these as callbacks from the `DrydockApplication` wiring, exactly as +the existing modals do; it holds no reference to the store. + +Because every ui write is a transform against the single writer, a +shutdown-time sidebar-width write cannot clobber a font size the modal wrote +(verified: `RepositoryManager.java:181,190`). + +### `UserConfig` write support + +`UserConfig` is read-only today. It gains: + +- `save(UserConfig, Path configFile)` — creates the parent directory when + absent, writes to a sibling temp file, then atomically moves it into + place, so a crash mid-write cannot leave a truncated config. +- `saveAsync(UserConfig)` — as `save`, on a virtual thread, returning a + `CompletableFuture`, because the caller is on the FX thread. + +- `flushPendingSaves()` — awaits any in-flight `saveAsync`, mirroring + `AnnotationStore.flushPendingSaves`, per AGENTS.md's rule that a service + writing files from a background thread must expose a flush so tests and + shutdown do not race pending writes. Called from `DrydockApplication.stop()` + alongside the existing flushes. + +Failure is surfaced to the caller (the future completes exceptionally); it +is not swallowed. This is deliberately asymmetric with `load()`, which +tolerates a missing or malformed file and falls back to `empty()` — a failed +*save* is a user-visible action that silently did nothing, so it must be +reported. + +## Interface font size + +All 142 `-fx-font-size` declarations live in a single file, `app.css`; there +are none in Java, none in the theme sheets, and no `-fx-font` shorthand or +`Font.font(…)`/`setFont(…)` call anywhere in `app/src`. + +**`app.css` is not rewritten.** Instead, `UiFontScale` (new, package-private +in `app.drydock.ui`) generates a scaled copy of the sheet at runtime: it +reads `app.css` from the classpath, multiplies every `-fx-font-size: px` +literal by `size / 13.0`, and writes the result to a temp file whose URL is +used in place of `app.css` in `scene.getStylesheets()`. Results are cached +per size, and the scale-1.0 case returns the original resource URL +untouched. This mirrors the extract-to-temp-file pattern `TerminalThemes` +already uses for ghostty configs. + +Two approaches were rejected, both of which look correct and are not: + +- **Converting the declarations to `em`.** JavaFX resolves font-relative + sizes against the font inherited from the nearest styleable ancestor, not + against `.root`, so nested rules would compound: `.code-area` (12px, + `app.css:1195`) containing `.lineno` (11px, `app.css:1206`) would yield + 13 × 0.923 × 0.846 = 10.15px instead of 11px. Every converted line looks + individually correct, so the error would not survive review — it would + survive *past* it. +- **An inline `-fx-font-size` on the scene root.** It does override the + `.root` rule, but combo popups, context menus and tooltips are separate + scene graphs that an inline style on this scene's root never reaches — + a fact `app.css:1304-1306` already documents. They would stay at 13px + while the rest of the UI scaled. A *stylesheet* does reach them, which is + precisely how app.css styles `.combo-box-popup .list-cell` and + `.menu-item .label` today. + +`ThemeManager` owns the swap, since it already owns +`scene.getStylesheets().setAll(app.css, theme.css)`: + +- `setUiFontSize(double)` — clamps to 11–16, generates/looks up the scaled + sheet, and re-applies both stylesheets. +- `setTheme(UiTheme)` — an absolute setter; only `toggle()` exists today + (`ThemeManager.java:52-56`) and the modal's radio needs to set a specific + value. `toggle()` is reimplemented in terms of it, and both keep firing + `onThemeChanged` so the title-bar glyph and persistence stay in sync. +- The constructor takes the persisted `uiFontSize` alongside `initialTheme`, + so the scaled sheet is in place before first layout rather than being + applied as a visible re-layout after startup. + +Generating the sheet touches the filesystem, so it runs off the FX thread on +first use for a given size and is applied via `Platform.runLater`; the +startup case is generated before the scene is shown. + +Scaling covers font sizes only. Stock JavaFX controls (modena) express +padding in `em`, so their padding scales with the root font size for free; +`app.css`'s own fixed `-fx-min-height`/`-fx-max-height` rules (24 of each, +e.g. `.filter-field` 32px, `.icon-button` 30px, `.title-bar` 44px) do not. +That mismatch, not the text itself, is what bounds the range: 11–16 is the +band to be confirmed by the manual check below, and the slider is capped +there. + +## Terminal font size + +The size goes into the generated ghostty config file, not the surface +struct. `TerminalThemes.configFileFor(UiTheme)` (`TerminalThemes.java:28-46`) +already extracts a per-theme `.conf` to a temp file; it gains the font size +as a second parameter, keys its cache on `(theme, size)`, and appends a +`font-size = ` line to the extracted config. + +Applying a change then reuses the path a theme toggle already takes: +`MainWorkspace.applyTerminalTheme` (`MainWorkspace.java:435-441`) loops the +open tabs, and `TerminalBridge` (`TerminalBridge.java:345-357`) calls +`ghostty_app_update_config` followed by `ghostty_surface_update_config` on +each surface (`GhosttySurface.java:276-285`). So the terminal size applies +**live to already-running sessions**, like the theme does. The `Applies to +new sessions` caption in the mockup is therefore dropped. + +The rejected alternative was the per-surface route: `ghostty_surface_config_s` +does declare an unset `font_size` float at offset 32 +(`GhosttyAppBinding.SURFACE_CONFIG_LAYOUT`), and writing it at +`ghostty_surface_new` looks like the natural seam. It is a trap — the very +next theme toggle calls `ghostty_surface_update_config`, which re-derives the +surface's config from the app config and drops the per-surface override, so +the user's terminal size would silently reset mid-session. It would also +have rippled a new component through `TerminalSpec`, `TerminalRuntime`, +`GhosttyApp`, `GhosttySurface.create`, `OpenSessionTab`, `SessionManager`, +three spike source sets, and `TerminalSpecTest`. The config-file route +changes one class. + +One assumption to verify before building on it: that libghostty honours +`font-size` in a config file loaded via `ghostty_config_load_file`. +`third_party/ghostty` is an unpopulated submodule, so it cannot be confirmed +from this checkout. **Verification step, first task in the plan:** append +`font-size = 20` to a `terminal-*.conf`, run the app, open a session, and +confirm the text is visibly larger. If it is not honoured, terminal font +size is cut from this round rather than reinstating the per-surface route. + +## The modal + +`SettingsModal` — new, package-private, in `app.drydock.ui`, structured like +`StartSessionModal`: a `VBox` with the `modal` style class, hosted in the +existing `ModalLayer`. That gives Esc-to-close, backdrop-click-to-close, and +automatic hiding of the native terminal view while the modal is up +(`ModalLayer.setOnShowingChanged`) with no new machinery. + +``` +┌─ Settings ──────────────────────────────── × ┐ +│ Appearance │ +│ Theme ( ● Dark ○ Light ) │ +│ Interface size 11 ──●───── 16 13 px │ +│ Terminal size 10 ────●─── 18 13 px │ +│ │ +│ Worktrees │ +│ Directory [ ~/dev/wt ] [Browse…] │ +│ New worktrees are created here │ +│ [ Done ] │ +└───────────────────────────────────────────────┘ +``` + +Behaviour: + +- **Apply on change**, no OK/Cancel — the macOS preferences convention. + `Done` (and Esc, and the `×`) simply close. +- **Theme** radio → `ThemeManager.setTheme`, which already routes through the + callback that updates the title-bar glyph and persists the choice. The + title-bar toggle and the modal therefore cannot drift: both read the + manager's current theme. +- **Interface size** slider → `ThemeManager.setUiFontSize` live while + dragging, so the effect is visible immediately; the state write is + debounced and happens on release, not per pixel. +- **Terminal size** slider → same shape: applied live to open surfaces via + the existing theme-reapply path, persisted on release. +- **Worktrees directory** → `UserConfig.loadAsync` when the modal opens, with + the field disabled and showing a "Loading…" prompt until it resolves; + `DirectoryChooser` behind `Browse…`; `saveAsync` on commit with the field + and Browse button disabled until it completes. Every completion path — + success, failure, and early return — re-enables the controls. A save + failure is reported through `UiErrors`. + +The modal holds no blocking I/O on the FX thread, per the repository's async +rules. + +### Opening it + +`⌘,` is handled in `DrydockApplication`'s existing scene key filter, and is +**inert while a modal is already showing** (`modalLayer.isShowingModal()`), +so it cannot replace an open Start-session or New-worktree modal underneath +the user. The gear button is unreachable in that state anyway, since the +backdrop covers the title bar. + +## Testing + +Following the repository's existing pure-logic test style +(`NewWorktreeStateTest`, `ApplicationStateCodecTest`, `UserConfigTest`): + +- `ApplicationStateCodecTest`: round-trip of `uiFontSize` / + `terminalFontSize`; absent members decode to defaults; non-numeric members + decode to defaults; an out-of-range number survives the decode unchanged + (clamping belongs to the consumer); none of these fail the overall decode. +- `UserConfigTest`: `save` → `load` round-trip; save over an existing + malformed file; save when `~/.drydock` does not yet exist; the temp file + is not left behind. +- `UiFontScaleTest` — the substantive new unit test, since this is where the + rejected `em` approach would have gone wrong silently. Against a fixture + stylesheet: every `-fx-font-size: Npx` is scaled by the factor and nothing + else in the text is touched (colors, `-fx-min-height`, `em` values, + comments, `-fx-font-family`); a nested rule pair scales + proportionally rather than compounding (12px/11px at factor 16/13 → + 14.77px/13.54px, ratio preserved); fractional sources like `12.5px` + survive; scale 1.0 is an identity that returns the original resource. +- A small pure helper for slider clamping / display formatting, tested + directly, keeping `SettingsModal` itself thin enough to need no UI test. +- `docs/manual-terminal-checklist.md` gains two steps: (a) with a session + open, change the terminal size and confirm the **running** surface resizes + and survives a subsequent theme toggle without reverting; (b) sweep the + interface size across 11–16 and confirm no clipping in the title bar, + filter field, icon buttons, combo popups, context menus, and tooltips — + the fixed-height rules and separate popup scene graphs being the two known + risks. + +## Deliberately out of scope + +- **Default base branch.** `NewWorktreeModal` already defaults the fork-from + field to the repository's checked-out branch, which is a better default + than any global value; a global override would fight it. +- **Skip delete confirmations.** Cheap to wire, but it removes the only + safety net on an irreversible, destructive action. +- **Per-repository settings.** `RepositorySettings` stays an empty record; + nothing in this round is repository-scoped. +- **Scaling padding, spacing, and fixed heights.** Only font sizes scale. + Making the fixed `-fx-min-height`/`-fx-max-height` rules scale too would + mean scaling the title bar and traffic lights, which are sized against + macOS conventions. The capped 11–16 range is the accepted trade.