From bb90255002beda369534554fba2e64229a26a3b8 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Mon, 28 Sep 2026 16:38:51 -0400 Subject: [PATCH] Add a button to open in android studio. --- build.zig | 13 +++ src/review/Server.zig | 63 ++++++++++++ src/review/studio.zig | 165 ++++++++++++++++++++++++++++++++ web/src/App.tsx | 11 +++ web/src/api.ts | 5 + web/src/components/DiffView.tsx | 37 +++++++ web/src/components/Icon.tsx | 1 + web/src/types.ts | 3 + 8 files changed, 298 insertions(+) create mode 100644 src/review/studio.zig diff --git a/build.zig b/build.zig index 3f17791..31dea02 100644 --- a/build.zig +++ b/build.zig @@ -257,6 +257,19 @@ pub fn build(b: *std.Build) void { }); test_step.dependOn(&b.addRunArtifact(store_tests).step); + // Only the path rule is tested: finding and running a launcher is the + // machine's business, and the rule is what keeps the page from opening a + // file outside the review. + const studio_tests = b.addTest(.{ + .root_module = b.createModule(.{ + .root_source_file = b.path("src/review/studio.zig"), + .target = target, + .optimize = optimize, + .link_libc = true, + }), + }); + test_step.dependOn(&b.addRunArtifact(studio_tests).step); + // The mute rules are their own root. Whether a tab is silent at this instant, // and what its row menu says about that, is arithmetic on a deadline — and // it is the half of notifications that can be wrong without anyone noticing, diff --git a/src/review/Server.zig b/src/review/Server.zig index 2708b0f..40c2c52 100644 --- a/src/review/Server.zig +++ b/src/review/Server.zig @@ -34,6 +34,7 @@ const Store = @import("Store.zig"); const assets = @import("assets.zig"); const git = @import("git.zig"); const model = @import("model.zig"); +const studio = @import("studio.zig"); const Server = @This(); @@ -687,6 +688,10 @@ fn handleApi( if (std.mem.eql(u8, path, "file")) { return self.handleFile(a, request, repo, query); } + if (std.mem.eql(u8, path, "studio/open")) { + if (method != .POST) return notFound(request); + return self.handleOpenInStudio(a, request, repo); + } if (std.mem.eql(u8, path, "comments")) { return switch (method) { .GET => writeJson(a, request, .ok, try store.list(a)), @@ -738,6 +743,10 @@ fn handleRepo( .drafts = store.countByStatus(.draft), .openComments = store.countByStatus(.submitted), .context = tab.ctx, + // Whether the file headers should offer to open a file in Android + // Studio. Asked on every load rather than once, so installing it while + // the app runs is enough for the next reload to pick it up. + .studio = studio.find(a, self.io) != null, }); } @@ -918,6 +927,60 @@ fn handleFile( }); } +/// Open one of the review's files in Android Studio, at a line when given one. +/// +/// The path is the diff's, relative to the work-tree root, and has to stay +/// inside it; see `studio.isInside`. A file the diff deletes, or one reviewed in +/// a commit that the work tree has since dropped, is a 404 rather than an IDE +/// opening onto nothing. +fn handleOpenInStudio( + self: *Server, + a: std.mem.Allocator, + request: *std.http.Server.Request, + repo: git.Repo, +) !void { + const raw = try readBody(a, request) orelse return writeError( + a, + request, + .bad_request, + "path is required", + ); + const body = std.json.parseFromSliceLeaky( + struct { path: []const u8 = "", line: ?u32 = null }, + a, + raw, + .{ .ignore_unknown_fields = true }, + ) catch return writeError(a, request, .bad_request, "expected {\"path\": \"...\", \"line\": n}"); + if (!studio.isInside(body.path)) return writeError( + a, + request, + .bad_request, + "path must be relative to the repository and stay inside it", + ); + + const file = try std.fs.path.join(a, &.{ repo.path, body.path }); + std.Io.Dir.accessAbsolute(self.io, file, .{}) catch return writeError( + a, + request, + .not_found, + "that file isn't in the work tree", + ); + const launcher = studio.find(a, self.io) orelse return writeError( + a, + request, + .not_found, + "Android Studio wasn't found — put its studio launcher on PATH", + ); + + studio.open(a, self.io, launcher, repo.path, file, body.line) catch return writeError( + a, + request, + .internal_server_error, + "Android Studio could not be started", + ); + return request.respond("", .{ .status = .no_content }); +} + fn handleCreateComment( self: *Server, a: std.mem.Allocator, diff --git a/src/review/studio.zig b/src/review/studio.zig new file mode 100644 index 0000000..6d637f0 --- /dev/null +++ b/src/review/studio.zig @@ -0,0 +1,165 @@ +//! Opening a file under review in Android Studio. +//! +//! Done through the IDE's own command-line launcher, `studio --line N `, +//! because that is the entry point that knows about windows. With Studio already +//! running, the launcher does not start a second copy: it hands the file to the +//! running instance and exits, and the instance opens it in the window of the +//! project that contains it — the checkout's own window, not whichever one was +//! focused last. With nothing running, the launcher *is* Studio starting up, and +//! it opens the project the file belongs to. +//! +//! That second case is why the launcher is not simply a child of the app. A +//! Studio started from here would land in playpen's own systemd scope, where it +//! running out of memory would take the app down with it — the failure +//! `scope.zig` exists to prevent for shells. So it goes into a scope of its own, +//! and is detached from the request that started it, which must not sit waiting +//! for an IDE to quit. + +const std = @import("std"); + +/// Launcher names worth finding on PATH: JetBrains Toolbox's shim when its +/// scripts directory is on PATH, what distro and snap packages install, and the +/// script a plain tarball ships with. +const path_names = [_][]const u8{ "studio", "android-studio", "studio.sh" }; + +/// Where launchers live when nothing put them on PATH, under `$HOME`. Toolbox +/// writes its shim here whether or not the directory is on PATH, which on a +/// desktop session started from a launcher it usually is not. +const home_paths = [_][]const u8{ + ".local/share/JetBrains/Toolbox/scripts/studio", + "android-studio/bin/studio", + "android-studio/bin/studio.sh", +}; + +/// Where a tarball, a snap or a Flatpak leaves it system-wide. +const system_paths = [_][]const u8{ + "/opt/android-studio/bin/studio", + "/opt/android-studio/bin/studio.sh", + "/usr/local/android-studio/bin/studio.sh", + "/snap/bin/android-studio", + "/var/lib/flatpak/exports/bin/com.google.AndroidStudio", +}; + +/// The Android Studio launcher on this machine, or null if there is none where +/// one is normally installed. Looked up afresh each time — a handful of +/// `access` calls — so installing Studio while the app runs is enough. +pub fn find(a: std.mem.Allocator, io: std.Io) ?[]const u8 { + if (std.c.getenv("PATH")) |raw| { + var dirs = std.mem.tokenizeScalar(u8, std.mem.span(raw), ':'); + while (dirs.next()) |dir| { + if (!std.fs.path.isAbsolute(dir)) continue; + for (path_names) |name| { + const candidate = std.fs.path.join(a, &.{ dir, name }) catch return null; + if (executable(io, candidate)) return candidate; + } + } + } + if (std.c.getenv("HOME")) |raw| { + const home = std.mem.span(raw); + if (std.fs.path.isAbsolute(home)) for (home_paths) |rel| { + const candidate = std.fs.path.join(a, &.{ home, rel }) catch return null; + if (executable(io, candidate)) return candidate; + }; + } + for (system_paths) |candidate| { + if (executable(io, candidate)) return candidate; + } + return null; +} + +fn executable(io: std.Io, path: []const u8) bool { + std.Io.Dir.accessAbsolute(io, path, .{ .execute = true }) catch return false; + return true; +} + +/// Whether `path` names something inside the directory it is relative to: not +/// absolute, and never stepping up out of it. Paths here come from the page, and +/// a request to show a file under review is no reason to hand an IDE any file +/// on disk. +pub fn isInside(path: []const u8) bool { + if (path.len == 0 or std.fs.path.isAbsolute(path)) return false; + if (std.mem.indexOfScalar(u8, path, 0) != null) return false; + var parts = std.mem.splitScalar(u8, path, '/'); + while (parts.next()) |part| { + if (std.mem.eql(u8, part, "..")) return false; + } + return true; +} + +/// Backgrounds the launcher and exits, so the request returns as soon as it has +/// started rather than when Studio quits. The launcher's own output goes nowhere: +/// it would otherwise hold the pipes `std.process.run` is reading open, and the +/// run would last as long as the IDE does. +/// +/// The scope is the same bargain `scope.zig` makes for shells, made with +/// `systemd-run` because this side of the app has no D-Bus connection to make it +/// with. Only tried when there is a session bus to ask; without one the launcher +/// runs in the app's own cgroup, as anything did before scopes. +const launch_script = + \\if [ -S "${XDG_RUNTIME_DIR:-/nonexistent}/bus" ] && command -v systemd-run >/dev/null 2>&1; then + \\ set -- systemd-run --user --scope --collect --quiet --slice=app.slice \ + \\ --description="Android Studio (from playpen)" -p OOMPolicy=continue -- "$@" + \\fi + \\"$@" /dev/null 2>&1 & +; + +/// Open `file`, an absolute path, in Android Studio at `line` (1-based) if one +/// is given. Returns once the launcher has been started, not once it has opened +/// anything: the launcher answers nothing back either way. +pub fn open( + a: std.mem.Allocator, + io: std.Io, + launcher: []const u8, + cwd: []const u8, + file: []const u8, + line: ?u32, +) !void { + var argv: std.ArrayListUnmanaged([]const u8) = .empty; + try argv.appendSlice(a, &.{ "sh", "-c", launch_script, "sh", launcher }); + if (line) |n| try argv.appendSlice(a, &.{ "--line", try std.fmt.allocPrint(a, "{d}", .{n}) }); + try argv.append(a, file); + + // The server's `Io` hands children an empty environment, which suits git + // and would leave the launcher unable to find a display to open a window + // on, a session bus for the scope, or even the running instance it is + // meant to hand the file to — that is found through `$HOME`. So this one + // gets the app's own. + var env = try processEnviron().createMap(a); + const result = std.process.run(a, io, .{ + .argv = argv.items, + .cwd = .{ .path = cwd }, + .environ_map = &env, + .stdout_limit = .limited(4 * 1024), + .stderr_limit = .limited(4 * 1024), + .timeout = .{ .duration = .{ .raw = .fromSeconds(5), .clock = .awake } }, + }) catch |err| { + std.log.warn("review: launching Android Studio: {s}", .{@errorName(err)}); + return error.LaunchFailed; + }; + switch (result.term) { + .exited => |code| if (code != 0) { + std.log.warn("review: launching Android Studio exited {d}: {s}", .{ + code, std.mem.trim(u8, result.stderr, " \t\r\n"), + }); + return error.LaunchFailed; + }, + else => return error.LaunchFailed, + } +} + +fn processEnviron() std.process.Environ { + var n: usize = 0; + while (std.c.environ[n] != null) : (n += 1) {} + return .{ .block = .{ .slice = std.c.environ[0..n :null] } }; +} + +test "isInside keeps paths within the work tree" { + try std.testing.expect(isInside("app/src/main/Foo.kt")); + try std.testing.expect(isInside("README.md")); + try std.testing.expect(isInside("a/..b/c")); + try std.testing.expect(!isInside("")); + try std.testing.expect(!isInside("/etc/passwd")); + try std.testing.expect(!isInside("../elsewhere")); + try std.testing.expect(!isInside("app/../../elsewhere")); + try std.testing.expect(!isInside("app/..")); +} diff --git a/web/src/App.tsx b/web/src/App.tsx index 496bee3..d0d7442 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -629,6 +629,16 @@ export default function App() { setLocate({ file, seq: ++locateSeq.current }); }, []); + // Success is Studio coming to the front, so only a failure needs saying here. + const openInStudio = useCallback( + (file: string, line?: number) => { + api + .openInStudio(file, line) + .catch((e) => flash(e instanceof Error ? e.message : String(e))); + }, + [flash], + ); + const draftCount = useMemo( () => comments.filter((c) => c.status === 'draft').length, [comments], @@ -963,6 +973,7 @@ export default function App() { reveal={reveal} onSetViewed={setFileViewed} onLocate={locateFile} + onOpenInStudio={repo?.studio ? openInStudio : undefined} onStartDraft={setDraft} onCancelDraft={cancelDraft} onSubmitDraft={submitDraft} diff --git a/web/src/api.ts b/web/src/api.ts index 03ca099..bdec90f 100644 --- a/web/src/api.ts +++ b/web/src/api.ts @@ -202,6 +202,11 @@ export const api = { reopen: (id: string) => postJSON(`${apiBase}/comments/${id}/reopen`).then(json), // Submits every draft in the review — the same set the rail shows. + // Hands the file to Android Studio's launcher, which opens it in the window of + // the project containing it. Resolves once the launcher has started, not once + // the file is on screen: nothing comes back from the IDE either way. + openInStudio: (path: string, line?: number) => + postJSON(`${apiBase}/studio/open`, { path, line }).then(json), submit: () => postJSON(`${apiBase}/review/submit`).then(json<{ submitted: number }>), // Deletes every comment, whatever its status. The reviewer's viewed marks are diff --git a/web/src/components/DiffView.tsx b/web/src/components/DiffView.tsx index 473f0d9..20accfa 100644 --- a/web/src/components/DiffView.tsx +++ b/web/src/components/DiffView.tsx @@ -66,6 +66,9 @@ interface Props { onSetViewed: (file: string, viewed: boolean, source?: string) => void; // Reveals the file in the left rail's file list. onLocate: (file: string) => void; + // Opens the file in Android Studio at `line`. Absent when there's no Studio to + // open it in, which hides the button. + onOpenInStudio?: (file: string, line?: number) => void; onStartDraft: (d: DraftTarget) => void; onCancelDraft: () => void; onSubmitDraft: (body: string) => Promise; @@ -435,6 +438,7 @@ export function DiffView({ reveal, onSetViewed, onLocate, + onOpenInStudio, onStartDraft, onCancelDraft, onSubmitDraft, @@ -484,6 +488,7 @@ export function DiffView({ bodyRef={bodyRef(path)} onSetViewed={onSetViewed} onLocate={onLocate} + onOpenInStudio={onOpenInStudio} onStartDraft={onStartDraft} onCancelDraft={onCancelDraft} onSubmitDraft={onSubmitDraft} @@ -495,6 +500,21 @@ export function DiffView({ ); } +// Where a file's first change sits in the work tree — the line to open it at. A +// deletion has no line of its own on the new side, so it lands on the line that +// now sits where the removed ones were. +function firstChangedLine(hunks: HunkData[]): number | undefined { + const hunk = hunks[0]; + if (!hunk) return undefined; + let next = hunk.newStart; + for (const change of hunk.changes) { + if (change.type === 'insert') return change.lineNumber; + if (change.type === 'delete') return Math.max(1, next); + next = change.newLineNumber + 1; + } + return Math.max(1, hunk.newStart); +} + function statusLabel(type: string): string { switch (type) { case 'add': @@ -542,6 +562,7 @@ const FileView = memo(function FileView({ bodyRef, onSetViewed, onLocate, + onOpenInStudio, onStartDraft, onCancelDraft, onSubmitDraft, @@ -567,6 +588,7 @@ const FileView = memo(function FileView({ bodyRef: (el: HTMLElement | null) => void; onSetViewed: Props['onSetViewed']; onLocate: Props['onLocate']; + onOpenInStudio: Props['onOpenInStudio']; onStartDraft: (d: DraftTarget) => void; onCancelDraft: () => void; onSubmitDraft: Props['onSubmitDraft']; @@ -763,6 +785,21 @@ const FileView = memo(function FileView({ > + {onOpenInStudio && file.type !== 'delete' && ( + + )} {file.type !== 'modify' && ( {statusLabel(file.type)} diff --git a/web/src/components/Icon.tsx b/web/src/components/Icon.tsx index 0fe530a..fc3478a 100644 --- a/web/src/components/Icon.tsx +++ b/web/src/components/Icon.tsx @@ -21,6 +21,7 @@ const PATHS = { 'M.513 1.513A1.75 1.75 0 0 1 1.75 1h3.5c.55 0 1.07.26 1.4.7l.9 1.2a.25.25 0 0 0 .2.1H13a1 1 0 0 1 1 1v.5H2.75a.75.75 0 0 0 0 1.5h11.978a1 1 0 0 1 .994 1.117L15 13.25A1.75 1.75 0 0 1 13.25 15H1.75A1.75 1.75 0 0 1 0 13.25V2.75c0-.464.184-.91.513-1.237Z', 'file-diff': 'M1 1.75C1 .784 1.784 0 2.75 0h7.586c.464 0 .909.184 1.237.513l2.914 2.914c.329.328.513.773.513 1.237v9.586A1.75 1.75 0 0 1 13.25 16H2.75A1.75 1.75 0 0 1 1 14.25Zm1.75-.25a.25.25 0 0 0-.25.25v12.5c0 .138.112.25.25.25h10.5a.25.25 0 0 0 .25-.25V4.664a.25.25 0 0 0-.073-.177l-2.914-2.914a.25.25 0 0 0-.177-.073ZM8 3.25a.75.75 0 0 1 .75.75v1.5h1.5a.75.75 0 0 1 0 1.5h-1.5v1.5a.75.75 0 0 1-1.5 0V7h-1.5a.75.75 0 0 1 0-1.5h1.5V4A.75.75 0 0 1 8 3.25Zm-3 8a.75.75 0 0 1 .75-.75h4.5a.75.75 0 0 1 0 1.5h-4.5a.75.75 0 0 1-.75-.75Z', + code: 'm11.28 3.22 4.25 4.25a.75.75 0 0 1 0 1.06l-4.25 4.25a.749.749 0 0 1-1.275-.326.749.749 0 0 1 .215-.734L13.94 8l-3.72-3.72a.749.749 0 0 1 .326-1.275.749.749 0 0 1 .734.215Zm-6.56 0a.751.751 0 0 1 1.042.018.751.751 0 0 1 .018 1.042L2.06 8l3.72 3.72a.749.749 0 0 1-.326 1.275.749.749 0 0 1-.734-.215L.47 8.53a.75.75 0 0 1 0-1.06Z', copy: 'M0 6.75C0 5.784.784 5 1.75 5h1.5a.75.75 0 0 1 0 1.5h-1.5a.25.25 0 0 0-.25.25v7.5c0 .138.112.25.25.25h7.5a.25.25 0 0 0 .25-.25v-1.5a.75.75 0 0 1 1.5 0v1.5A1.75 1.75 0 0 1 9.25 16h-7.5A1.75 1.75 0 0 1 0 14.25ZM5 1.75C5 .784 5.784 0 6.75 0h7.5C15.216 0 16 .784 16 1.75v7.5A1.75 1.75 0 0 1 14.25 11h-7.5A1.75 1.75 0 0 1 5 9.25Zm1.75-.25a.25.25 0 0 0-.25.25v7.5c0 .138.112.25.25.25h7.5a.25.25 0 0 0 .25-.25v-7.5a.25.25 0 0 0-.25-.25Z', unfold: 'm8.177.677 2.896 2.896a.25.25 0 0 1-.177.427H8.75v1.25a.75.75 0 0 1-1.5 0V4H5.104a.25.25 0 0 1-.177-.427L7.823.677a.25.25 0 0 1 .354 0ZM7.25 10.75a.75.75 0 0 1 1.5 0V12h2.146a.25.25 0 0 1 .177.427l-2.896 2.896a.25.25 0 0 1-.354 0l-2.896-2.896A.25.25 0 0 1 5.104 12H7.25v-1.25Zm-5-2a.75.75 0 0 0 0-1.5h-.5a.75.75 0 0 0 0 1.5h.5ZM6 8a.75.75 0 0 1-.75.75h-.5a.75.75 0 0 1 0-1.5h.5A.75.75 0 0 1 6 8Zm2.25.75a.75.75 0 0 0 0-1.5h-.5a.75.75 0 0 0 0 1.5h.5ZM12 8a.75.75 0 0 1-.75.75h-.5a.75.75 0 0 1 0-1.5h.5A.75.75 0 0 1 12 8Zm2.25.75a.75.75 0 0 0 0-1.5h-.5a.75.75 0 0 0 0 1.5h.5Z', diff --git a/web/src/types.ts b/web/src/types.ts index 4a9ee0a..2ac2cbb 100644 --- a/web/src/types.ts +++ b/web/src/types.ts @@ -96,6 +96,9 @@ export interface RepoState extends RepoInfo { // page publishes one. The browser is the source of truth for what's on screen; // this copy is what lets an agent review, and anchor comments to, the same diff. context?: DiffContext | null; + // Whether this machine has an Android Studio for the file headers to open + // files in (see api.openInStudio). + studio: boolean; } export interface DiffFile {